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

Protocol assemblies can be defined in SSPs empty without port-ranges #1521

Closed
aj-stein-nist opened this issue Oct 20, 2022 · 6 comments · Fixed by #1674
Closed

Protocol assemblies can be defined in SSPs empty without port-ranges #1521

aj-stein-nist opened this issue Oct 20, 2022 · 6 comments · Fixed by #1674
Assignees
Labels
Milestone

Comments

@aj-stein-nist
Copy link
Contributor

aj-stein-nist commented Oct 20, 2022

Describe the bug

Per discussion with @david-waltermire-nist, @iMichaela, and @GaryGapinski with an external stakeholder, it was determined that vanilla NIST OSCAL (in 1.x.y releases) permit a protocol assembly to be defined implemented without a port-range within or any other semantically meaningful detail documented, but is optional. Per discussion with the team, it is advisable to add a warning constraint (or pending further feedback, a schema modification but this would be backwards-compatibility breaking) to inform developers and users the protocol definition will only logically be useful without port ranges.

Who is the bug affecting

Tool authors and security practitioners who want to properly document protocol usage in a SSP component accurately without error and missing meaningful information.

What is affected by this bug

Documentation, Metaschema, Modeling

How do we replicate this issue

  1. Create an OSCAL system-security-plan.
  2. Create a component in the SSP from 1.
  3. Create a <protocol /> within the component from 2.
  4. Run schema validation or constraints through the Java-based oscal-cli tool. Observe no warning or other form of output indicating that a protocol without a port-range is missing meaningful port information.

Expected behavior (i.e. solution)

A constraint is implemented to warn the developers or users that the protocol is missing meaningful port range information.

Other comments

No response

@aj-stein-nist
Copy link
Contributor Author

  • Add a constraint to error when the ports are missing (do not enforce this in JSON and XML Schema).
    • Potentially review with community if such a constraint violation is a warning or a full-blown error.
  • Potentially update documentation to ensure it is clear there should be ports for a protocol (not schema enforced for backwards compat)
  • Add an issue to fix this in V2 when it will not break backwards compat

Potentially we can fix this in 1.1 (not yet sprint assigned, just an estimate of the milestone per discussion in issue triage).

@aj-stein-nist aj-stein-nist added this to the OSCAL 1.1.0 milestone Nov 1, 2022
@GaryGapinski
Copy link

Since neither start nor end are required for port-range at this time,

  • Will both be required? Reason for asking is many protocols use a single port, thus start or end could serve to represent a single port. Or not, in which case a separate attribute (e.g., port) could be used. Otherwise start and end should both be required.
  • The documentation does not strongly indicate that start and end delimit a contiguous range.
  • Should not end be constrained to be greater than or equal to start?
  • If multiple port-range elements are employed, should not there be a constraint that the multiple contiguous ranges do not overlap?

@Compton-US Compton-US self-assigned this Feb 2, 2023
@Compton-US
Copy link
Contributor

Compton-US commented Feb 27, 2023

This is what I'm looking at for the fix. I tried to base on the other tests I saw in OSCAL, so if someone could double check me for obvious test syntax problems.

The plan:

  • Change this file: src/metaschema/oscal_implementation-common_metaschema.xml:279ish
  • Add the following constraints as warnings for the port-range assembly. (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.
<!-- Added Contraints as Warnings -->
<constraint>
      <expect level="WARNING" id="port-range-start-and-end-not-specified" target="." test="exists(@start) and exists(@end)">
            <message>If a protocol is defined, it should include a start and end port range. To define a single port, the start and end should be the same value.</message>
      </expect>
      <expect level="WARNING" id="port-range-start-specified-with-no-end" target="." test="exists(@start) and not(exists(@end))">
            <message>A start port exists, but an end point does not. To define a single port, the start and end should be the same value.</message>
      </expect>
      <expect level="WARNING" id="port-range-end-specified-with-no-start" target="." test="not(exists(@start)) and exists(@end)">
            <message>An end point exists, but a start port does not. To define a single port, the start and end should be the same value.</message>
      </expect>
      <expect level="WARNING" id="port-range-end-date-is-before-start-date" target="." test="@start &le; @end">
            <message>The port range specified has an end port that is less than the start port.</message>
      </expect>
</constraint>

If this looks good I'll work it up in a PR to develop.

@Compton-US Compton-US moved this from Todo to In Progress in NIST OSCAL Work Board Feb 27, 2023
@Compton-US Compton-US moved this from In Progress to Under Review in NIST OSCAL Work Board Feb 27, 2023
@iMichaela
Copy link
Contributor

@Compton-NIST and @GaryGapinski - I think Gary brings very good points, that both start and end should be both mandatory, with the end value > start value. Any reason/scenario we are trying to accommodate with the warnings instead of constraints?

@aj-stein-nist
Copy link
Contributor Author

If this looks good I'll work it up in a PR to develop.

Looks good so far, but just draft the PR and we will review it there.

@Compton-US
Copy link
Contributor

Any reason/scenario we are trying to accommodate with the warnings instead of constraints?

I set it at the warning level with the expectation that this would not break existing implementations of OSCAL by unexpectedly failing validations. We can always escalate the level of the constraint in a breaking release.

Compton-US pushed a commit to Compton-US/OSCAL that referenced this issue Feb 28, 2023
aj-stein-nist pushed a commit that referenced this issue Mar 1, 2023
* Require (Warn) port start and end when protocol is specified. #1521

* Fix less than/equal entity in test. #1521
@aj-stein-nist aj-stein-nist linked a pull request Mar 1, 2023 that will close this issue
9 tasks
@github-project-automation github-project-automation bot moved this from Under Review to Done in NIST OSCAL Work Board Mar 1, 2023
aj-stein-nist pushed a commit to aj-stein-nist/OSCAL-forked that referenced this issue Jun 29, 2023
…gov#1674)

* Require (Warn) port start and end when protocol is specified. usnistgov#1521

* Fix less than/equal entity in test. usnistgov#1521
aj-stein-nist pushed a commit to aj-stein-nist/OSCAL-forked that referenced this issue Jul 10, 2023
…gov#1674)

* Require (Warn) port start and end when protocol is specified. usnistgov#1521

* Fix less than/equal entity in test. usnistgov#1521
aj-stein-nist pushed a commit to galtm/OSCAL that referenced this issue Sep 28, 2023
…gov#1674)

* Require (Warn) port start and end when protocol is specified. usnistgov#1521

* Fix less than/equal entity in test. usnistgov#1521
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants