Implement Constraints for Diagram Link Validations That Are No Longer Blocked

[Gabeblis]

Constraint Task

Develop and integrate the following constraints which are now feasible due to the support introduced in OSCAL-CLI version 2.3:

• has-authorization-boundary-diagram-link-href-target
• has-network-architecture-diagram-link-href-target
• has-data-flow-diagram-link-href-target

Intended Outcome

The intended outcome is to write validation checks for diagram link targets. This ensures that the respective diagram links in the system-characteristics have a valid reference to a resource in the back-matter.

Syntax Type

This is required core OSCAL syntax.

Allowed Values

There are no relevant allowed values.

Metapath(s) to Content

/system-security-plan/system-characteristics/(authorization-boundary | data-flow | network-architecture)/diagram/link/@href

Purpose of the OSCAL Content

No response

Dependencies

No response

Acceptance Criteria

• All OSCAL adoption content affected by the change in this issue have been updated in accordance with the Documentation Standards.
  • Explanation is present and accurate
  • sample content is present and accurate
  • Metapath is present, accurate, and does not throw a syntax exception using oscal-cli metaschema metapath eval -e "expression".
• All constraints associated with the review task have been created
• The appropriate example OSCAL file is updated with content that demonstrates the FedRAMP-compliant OSCAL presentation.
• The constraint conforms to the FedRAMP Constraint Style Guide.
  • All automated and manual review items that identify non-conformance are addressed; or technical leads (David Waltermire; AJ Stein) have approved the PR and “override” the style guide requirement.
• Known good test content is created for unit testing.
• Known bad test content is created for unit testing.
• Unit testing is configured to run both known good and known bad test content examples.
• Passing and failing unit tests, and corresponding test vectors in the form of known valid and invalid OSCAL test files, are created or updated for each constraint.
• A Pull Request (PR) is submitted that fully addresses the goals section of the User Story in the issue.
• This issue is referenced in the PR.

Other information

No response

[brian-ruf]

A few thoughts here as I perform analysis on #811 :

• OSCAL allows for the diagram/@href to use a URI fragment, a relative URI, or an absolute URI. It appears we are only allowing the first option here. As long as that is a consistently enforced approach to all FedRAMP attachments, it is fine. I just want to make sure this is a conscious choice and not an oversight.
• It's not enough to check that the URI fragment points to a valid resource because rlink and base64 fields are both optional. After checking that the resource exists, we then need to:
  • check that a base64 field exits
  • if no base64 field exists we need to check for an rlink/@href flag, and then check that the href value points to a valid file.

[brian-ruf]

One additional point here:
The idea that the PMO only accepts certain formats for diagrams has come up from time to time.

At a minimum, we should define the list of diagram formats we know we can handle and WARN if we either can't detect the format or if we detect a format that isn't on our known-good list. We can still attempt to accept other formats, but can't guarantee their viability in our system and would at least want to discourage known-good formats.

As for what formats should be on the known-good list, I assume the UI for our platform will be browser-based, thus any image format that can be handled by a standard Edge or Chrome (Chromium) or Firefox-based browser should be acceptable, such as GIF, JPEG, PNG, SVG, AVIF, HEIF, WebP at a minimum (Although we may want to encourage/discourage certain ones in this list.). We'd have to more carefully consider types such as Ai, TIFF, EPS, Raw, BMP, WMF and EMF.

In addition, reviewers are required to use GSA laptops due to the sensitivity of the content. These always include the MS Office suite and an ability to view PDF documents. So we should also include PDF, DOC, DOCX, ODT, PPT, PPTX, VSD, VSDX (Word, PowerPoint and Visio. No Excel and no templates). More discussion is required for macro-enabled MS Office file types (DOCM, PPTM, VSDM).

[aj-stein-gsa]

A few thoughts here as I perform analysis on #811 :

• OSCAL allows for the diagram/@href to use a URI fragment, a relative URI, or an absolute URI. It appears we are only allowing the first option here. As long as that is a consistently enforced approach to all FedRAMP attachments, it is fine. I just want to make sure this is a conscious choice and not an oversight.

Do you mean link/@href (example)? Both cases are covered by the current PR in the different link checks in scope for this comment.

https://github.com/Gabeblis/fedramp-automation/blob/5e8b5d902cf53756be3392aba5f5413024661344/src/validations/constraints/fedramp-external-constraints.xml#L79

• It's not enough to check that the URI fragment points to a valid resource because rlink and base64 fields are both optional. After checking that the resource exists, we then need to:

  • check that a base64 field exits
  • if no base64 field exists we need to check for an rlink/@href flag, and then check that the href value points to a valid file.

Totally agreed, but FYI those are separately handled in a separate constraint to highlight such an error on the resource itself.

fedramp-automation/src/validations/constraints/fedramp-external-constraints.xml

Lines 126 to 134 in 821ec62

	<expect id="resource-has-base64-or-rlink" target="resource" test="count(./rlink) >= 1 or count(./base64) >= 1" level="WARNING">
	<formal-name>Resource Has Base64 Or Rlink</formal-name>
	<prop namespace="https://docs.oasis-open.org/sarif/sarif/v2.1.0" name="help-url" value="https://automate.fedramp.gov/documentation/general-concepts/oscal-citations-and-attachments/#including-multiple-rlink-and-base64-fields"/>
	<message>Every supporting artifact found in a citation MUST have at least one base64 or rlink element.</message>
	</expect>
	<expect id="resource-has-title" target="resource" test="title" level="WARNING">
	<formal-name>Resource Has Title</formal-name>
	<prop namespace="https://docs.oasis-open.org/sarif/sarif/v2.1.0" name="help-url" value="https://automate.fedramp.gov/documentation/general-concepts/oscal-citations-and-attachments/#citation-and-attachment-details"/>
	<message>Every supporting artifact found in a citation SHOULD have a title.</message>

[aj-stein-gsa]

I would think maybe doc-available() for non-base64 is helpful in the URL but that goes a little beyond the scope of the simple completeness realm for something we do not load up until post-processing in a package (maybe?) and there are some concerns with flakiness there, but that is why I didn't push much harder on that in review. I more than willing to hear counterpoints on that too!

[aj-stein-gsa]

One additional point here: The idea that the PMO only accepts certain formats for diagrams has come up from time to time.

Yep, thus closed attachment check PRs and slated work in #674. We should discuss! 😄

[brian-ruf]

Totally agreed, but FYI those are separately handled in a separate constraint to highlight such an error on the resource itself.

@aj-stein-gsa I'm glad to see this exists, and understand you are saying that if we are checking all resources for attachments, we don't need to duplicate that check on one-off checks.

I like that approach, but see a few challenges we need to address.

First Concern

The constraint you cited appears to require that EVERY resource must either have a base64 or rlink/@href. Some legitimately will have neither, such as when the resource is a true citation using the resource/citation/link/@href assembly. And I believe it is fair to have a required citation that is not published on the Internet. So while rare, it is possible to have a valid resource/citation that does not have a link.

At a minimum, we should revise that constraint to ONE of the following:

• test="count(./rlink) >= 1 or count(./base64) >= 1 or count(./citation/link) >= 1"; or
• test="count(./rlink) >= 1 or count(./base64) >= 1 or count(./citation) >= 1"

Second Concern

This needs to go one step further if possible. The goal is to ensure an attachment is available with the PMO reviews the SSP.
There can be 0 or 1 base64 fields and 0 or more rlink fields. Among that collection, at least one must be viable to the PMO upon receipt.

Can constraints test whether href values are reachable? If not, we may need to defer this for now, per our agreed prioritization in the constraint strategy.

In any case, this has the additional concern that a link is valid within a CSP's intranet when they run the constraint, yet invalid to FedRAMP reviewers post delivery. href values (that are full URIs, not fragments) are the only OSCAL content that has this challenge.

[brian-ruf]

I would think maybe doc-available() for non-base64 is helpful in the URL but that goes a little beyond the scope of the simple completeness realm for something we do not load up until post-processing in a package (maybe?) and there are some concerns with flakiness there, but that is why I didn't push much harder on that in review. I more than willing to hear counterpoints on that too!

I wrote the above "Second Concern" before I read this comment. Sounds like we are saying similar - that this may go beyond completeness checks and have to be deferred.

[aj-stein-gsa]

Totally agreed, but FYI those are separately handled in a separate constraint to highlight such an error on the resource itself.

@aj-stein-gsa I'm glad to see this exists, and understand you are saying that if we are checking all resources for attachments, we don't need to duplicate that check on one-off checks.

I like that approach, but see a few challenges we need to address.

First Concern

The constraint you cited appears to require that EVERY resource must either have a base64 or rlink/@href. Some legitimately will have neither, such as when the resource is a true citation using the resource/citation/link/@href assembly. And I believe it is fair to have a required citation that is not published on the Internet. So while rare, it is possible to have a valid resource/citation that does not have a link.

At a minimum, we should revise that constraint to ONE of the following:

• test="count(./rlink) >= 1 or count(./base64) >= 1 or count(./citation/link) >= 1"; or

• test="count(./rlink) >= 1 or count(./base64) >= 1 or count(./citation) >= 1"

I would say we can patch in a bug fix I can open up an issue for that and PR it.

Second Concern

This needs to go one step further if possible. The goal is to ensure an attachment is available with the PMO reviews the SSP.

There can be 0 or 1 base64 fields and 0 or more rlink fields. Among that collection, at least one must be viable to the PMO upon receipt.

Can constraints test whether href values are reachable? If not, we may need to defer this for now, per our agreed prioritization in the constraint strategy.

In any case, this has the additional concern that a link is valid within a CSP's intranet when they run the constraint, yet invalid to FedRAMP reviewers post delivery.

What you say resonates and I agree. In the PR attached to this issue, we can recommend doc-available() calls for the PR related for this issue. I have been light on such feedback in reviews thus far and expect rework on the boundary of constraint and extra-constraint check implementation due to the other topic you call out, I get into that below. So I still say we add it here, but rework will be easy to identify and incrementally enhance, I don't disagree: it's important.

Beyond that whether the content that is resolved from a base64 payload or rlink or link/@href is less clear. So using absolute URLs or relative URLs is probably to our benefit for these integrations that go just beyond constraint checks and will need to be overriden anyway. I would say mandating one or the other or neither right now is premature. (There's a lot of detail there and I can't say much because that is contingent on the platform pilot and details we have not hammered down. But this is not that different from the profile resolution concern and why I have dissenting opinion on what other communities propose upstream with NIST staff, but that goes way beyond the scope of this issue beyond the inevitable need of software, by FedRAMP or by others, for this extra constraint analysis even if an absolute or relative path is resolvable, but I'll leave that for another time.)

[aj-stein-gsa]

The constraint you cited appears to require that EVERY resource must either have a base64 or rlink/@href. Some legitimately will have neither, such as when the resource is a true citation using the resource/citation/link/@href assembly. And I believe it is fair to have a required citation that is not published on the Internet. So while rare, it is possible to have a valid resource/citation that does not have a link.

I also wanted to address this concern, which is a matter of style. Essentially, I have seen nothing FedRAMP produces use citations, and they seem particularly popular with NIST only for catalog and profile back-matter resource references. Even when they do use them, the citation/text is defined, and the sibling rlink is used, not the child citation/link. I think it stands to reason we should not be encouraging or using the latter either.

https://github.com/usnistgov/oscal-content/blob/941c978d14c57379fbf6f7fb388f675067d5bff7/nist.gov/SP800-53/rev5/xml/NIST_SP-800-53_rev5_catalog.xml

For digital authorization packages, we have already had a few discussions about this, but as a principle citation-based resources without a link we do not really need or want, and the use cases we see it in the wild are outside the scope of the packages themselves and if so they use the sibling rlink and base64 on purpose. That is preferable.

[brian-ruf]

For digital authorization packages, we have already had a few discussions about this, but as a principle citation-based resources without a link we do not really need or want, and the use cases we see it in the wild are outside the scope of the packages themselves and if so they use the sibling rlink and base64 on purpose. That is preferable.

I can agree with this. It's fair if we known of no use case for the latter. If one exists, someone can engage us to request we relax this to WARNING instead of ERROR.

[brian-ruf]

I just noticed in the constraint tracker that the authorization-boundary and data-flow diagrams were properly marked as ERROR, but network-architecture diagram was incorrectly marked as WARNING. This should also be ERROR. I changed it in the constraint tracker, but wanted to capture it here as well.

UPDATE: Looks like it was correctly implemented as ERROR in PR #865. No further action needed.