Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Local version of SP800-53rev5.1.1 that contains the correct labels #540

Merged

Conversation

david-waltermire
Copy link
Member

@david-waltermire david-waltermire commented Dec 18, 2023

Committer Notes

After reviewing the SP 800-53 rev5 catalog changes in oscal-content release 1.2.1, this catalog should not have been released as-is.

Speaking from a FedRAMP perspective, I agree with much of what @wendellpiez has been commenting on in the related PR for a number of reasons.

  1. These adjustments are not backwards compatible. This will cause breakage for FedRAMP users.
    a. The "label" prop is discontinued. This will break consumers expecting a label. They need to be restored.
    b. The alt-identifier was used semantically to identify legacy labels in rev 4, which were mapped to new objects in rev5. Using them for labels breaks the expected semantics. Although the change was discussed in issue 224, no reason has been given why it was necessary to make this change to make the labels more consistent. The original approach needs to be restored.
  2. Issue 224 was requesting feedback in 24 hours (due by COB 12/8), which was not a sufficient amount of time for community review. The FedRAMP community meets roughly bi-weekly, so there hasn't been an opportunity to discuss this yet in a high-bandwidth forum. We are also in the holiday season, which is disrupting some of the normal meeting cycles and diverting attention. To maximize attention and review, this change should have been discussed with the community after the holidays.
  3. Combining the identifier changes with the other fixes is not a good practice. It makes it difficult to tell what was related to an identifier change vs a fix. In the future, such changes should be split into two PRs to facilitate adequate review. @wendellpiez requested this, but his request was not addressed.
  4. The change in release 1.2.1 is described as a patch release, but the adjustments clearly break backwards compatibility and add new features such as zero padded identifiers. This isn't appropriate for a patch release, which should be backwards compatible with 1.2.0.

Unless the labels are restored, I don't see how FedRAMP can use the catalog as-is. Instead, we will use a local version of the SP800-53 catalog to allow for better change control. FedRAMP will socialize the catalog in this PR with the FedRAMP stakeholder community in early January to get consensus on the approach. FedRAMP is interested in contributing this back to NIST once some consensus is reached on the approach.

This PR reverts the labels back, but still incorporates the other fixes. A new class="sp800-53-zero-padded" label was added to include the padded identifier.

While the class names can be debated, this approach provides a means to support the prior non-padded versions, while also supporting the padded versions. Using this approach, the existing labels remain stable, while adding a new label for the padded version. This also allows both the padded and non-padded versions to be used for display based on the same content, which is not possible under the current release.

This PR also resolves issue #232 by fixing the media types used in the FedRAMP baselines.

All Submissions:

By submitting a pull request, you are agreeing to provide this contribution under the CC0 1.0 Universal public domain dedication.

@rpalmer-gsa
Copy link

Concur,

  1. Further collaboration with NIST is needed to ensure FedRAMP can identify, plan for, and manage downstream impacts to FedRAMP OSCAL and the FedRAMP stakeholder community.
  2. Further collaboration and discussion is needed with the FedRAMP stakeholder community to refine FedRAMP's approach for change control and versioning.

@david-waltermire david-waltermire force-pushed the sp800-53rev5-local-version branch 3 times, most recently from 74c10b0 to 7579214 Compare December 19, 2023 00:04
@david-waltermire
Copy link
Member Author

david-waltermire commented Jan 4, 2024

This diff illustrates the changes to the NIST_SP-800-53_rev5_catalog.xml in this PR.

These changes are based on the v1.2.0 version (raw) released by NIST.

@david-waltermire
Copy link
Member Author

This PR has been updated to sync with the changes from usnistgov/oscal-content#238. The local catalog is now in sync with the latest in oscal-content develop.

Copy link
Member

@Rene2mt Rene2mt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a comment / question about which prop name and class we want to use going forward for the zero padded control ID labels

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Local catalog is good. However, noticed that the FedRAMP local catalog uses the label prop with class for SP800-53:

<prop name="label" class="sp800-53-zero-padded" value="AC-02(01)"/>

Whereas NIST very recently release the catalog using a different prop and class:

<prop name="alt-identifier" class="sp800-53" value="AC-02(01)"/>

Both approaches are valid but the subtle differences (in prop and class) may cause issues downstream. It might be better if both catalogs use the same prop & class (either NIST or FedRAMP updates their version of the catalog).

Copy link

@iMichaela iMichaela Feb 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Rene2mt - the OSCAL content latest release (v.1.3.0) which includes the NIST 800-53 using OSCAL 1.1.2 uses 4 times of labels:

  1. one to align the labels with the padded control IDs (replaces the prop/@name='alt-identifier') :
   <prop name="label" class="zero-padded" value="AC-02"/>
  1. and 3 were kept for label-related backwards compatibility:
      <prop name="label" value="AC-2"/>
      <prop name="label" class="sp800-53a" value="AC-02"/>
      <prop name="sort-id" value="ac-02"/>

@david-waltermire david-waltermire changed the base branch from master to develop March 7, 2024 12:45
@david-waltermire david-waltermire changed the base branch from develop to master March 7, 2024 12:50
@david-waltermire david-waltermire merged commit 8c9f8af into GSA:master Mar 7, 2024
7 checks passed
@david-waltermire david-waltermire deleted the sp800-53rev5-local-version branch March 7, 2024 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rev5 NIST 800-53 rev 5 scope: baselines
Projects
Archived in project
4 participants