Require (Warn) port start and end when protocol is specified.

[Compton-US]

Committer Notes

See #1521 for detail.

Add WARNING constraints for the port-range assembly when there are issues with the port start and end.
(Assuming this prevents breaking change, but if that's not true, let me know what level it should be.)

This covers:

• Port range start and end not specified.
• Port range start specified without an end.
• Port range end specified without a start.
• Port range end is less than the start.

All Submissions:

• Have you selected the correct base branch per Contributing guidance?
• Have you set "Allow edits and access to secrets by maintainers
  "?
• Have you checked to ensure there aren't other open Pull Requests for the same update/change?
• Have you squashed any non-relevant commits and commit messages? [instructions]
• Do all automated CI/CD checks pass?

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

Changes to Core Features:

• Have you added an explanation of what your changes do and why you'd like us to include them?
• Have you written new tests for your core changes, as applicable?
• Have you included examples of how to use your new feature(s)?
• Have you updated all OSCAL website and readme documentation affected by the changes you made? Changes to the OSCAL website can be made in the docs/content directory of your branch.

[wendellpiez]

So ... the remarks state the expectation is that the value be an integer greater than zero. The tests, however, only check for the existence of the attribute not its lexical form. As given, the warning will not be emitted for start='there'.

For this to align, we could test whether each value is an integer, as well as checking its value (to prohibit negatives), as well as @end >= @start.

As given the patch is an improvement over silence but does not express the rule fully, either.

[aj-stein-nist]

So ... the remarks state the expectation is that the value be an integer greater than zero. The tests, however, only check for the existence of the attribute not its lexical form. As given, the warning will not be emitted for start='there'.

For this to align, we could test whether each value is an integer, as well as checking its value (to prohibit negatives), as well as @end >= @start.

As given the patch is an improvement over silence but does not express the rule fully, either.

Wait, I want to make sure I understand based on the existing schema (in the same file but not modified in this PR): we do not check this "at a lower layer" in XSD with type enforcement and a nonNegativeInteger?

OSCAL/src/metaschema/oscal_implementation-common_metaschema.xml

Lines 290 to 296 in 41bc4d7

	<define-flag name="end" as-type="nonNegativeInteger">
	<formal-name>End</formal-name>
	<description>Indicates the ending port number in a port range</description>
	<remarks>
	<p>Should be a number within a permitted range</p>
	</remarks>
	</define-flag>

[wendellpiez]

No we do not. However, these could be expressed directly as constraints. Indeed that is the point. To check them in XSD requires XSD+Schematron, not implemented everywhere. To check them in unextended JSON Schema is not possible today TMK. Hence we decided to rely on the constraints layer, which presumably will have implementations on both XML and object-notation sides.

This is an awesome example of the use case for constraints. We have two flags each of which follows 'semantic' rules only partially explained in lexical terms. We also see a requirement that they be conditioned on one another (end must be after start).

No one has asked yet that multiple port ranges not be permitted to overlap. That will be a 'doozy' is I think the technical term.

[aj-stein-nist]

No we do not. However, these could be expressed directly as constraints. Indeed that is the point. To check them in XSD requires XSD+Schematron, not implemented everywhere. To check them in unextended JSON Schema is not possible today TMK. Hence we decided to rely on the constraints layer, which presumably will have implementations on both XML and object-notation sides.

This is an awesome example of the use case for constraints. We have two flags each of which follows 'semantic' rules only partially explained in lexical terms. We also see a requirement that they be conditioned on one another (end must be after start).

No one has asked yet that multiple port ranges not be permitted to overlap. That will be a 'doozy' is I think the technical term.

I checked the XML Schema validation and the non-negative integer checking and enforcement appears to do what we need to cover this at a separate layer, distinct from the constraints. I am happy to accept the PR after testing that. start="there" will fail as will other values like "0" or negative number gotchas we discussed like "--10000.00".

As for the port-range and overlap concerns, those are valid, but I am not sure that is something we understand the subject domain (from personal experience, that could be acceptable or non-acceptable depending on the protocols in question and/or the system, let alone the assessor). This is a good start, and we can always go further with considering or definitively answering these questions when community members bring them up.

[wendellpiez]

@aj-stein-nist @Compton-NIST nice job!

@aj-stein-nist before publication, enhancements such as this should probably be road-tested, to confirm the dev's testing (in this case Chris's). The unit tests I showed you yesterday could actually be adapted for that, or it would be the same idea.