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

Components representing inter-boundary communication need to declare the direction of data flow #930

Open
14 tasks
Tracked by #808
aj-stein-gsa opened this issue Nov 22, 2024 · 8 comments · Fixed by #968 or #994
Open
14 tasks
Tracked by #808

Comments

@aj-stein-gsa
Copy link
Contributor

aj-stein-gsa commented Nov 22, 2024

Constraint Task

There must be at least one direction property, with no more than one incoming and no more than one outgoing direction.

Intended Outcome

Check that different kinds of network components for different leveraged authorizations and interconnection scenarios (see #808 (comment)) declare minimally required network traffic directions.

Syntax Type

This is required core OSCAL syntax.

Allowed Values

There are no relevant allowed values.

Metapath(s) to Content

<!-- Metapath context target -->
"//component[ (@type='service'  and not(./prop[@name='leveraged-authorization-uuid']) and ./prop[@name='implementation-point' and @value='external']) or (@type='interconnection') or (@type='service' and ./prop[@name='implementation-point' and @value='internal'] and ./prop[@name='direction']) or (@type='software' and ./prop[@name='asset-type' and @value='cli'] and ./prop[@name='direction']) ]"

<!-- Constraint requirements:   There must be at least one direction property, with no more than one incoming and no more than one outgoing direction -->
count(./prop[@name='direction'])>=1
count(./prop[@name='direction' and @value='incoming']) <= 1
count(./prop[@name='direction' and @value='outgoing']) <= 1


<!-- Constraint requirements:
        If ./prop[@name='direction' and @value='incoming'] 
        If direction is incoming there must be at least one local ipv4 address, ipv6 address, or URI to an API.
-->
count( (./prop[@class='local' and @name=('ipv4-address', 'ipv6-address')]) or (./link[@rel='uri']) ) >=1

<!-- Constraint requirements:
        If ./prop[@name='direction' and @value='incoming'] 
        If direction is incoming there must be at least one local protocol entry.
-->
count(./protocol[@name='local']/port-range/@start)

<!-- Constraint requirements:
        If ./prop[@name='direction' and @value='outgoing']
        If direction is outgoing there must be at least one remote ipv4 address, ipv6 address or URI to an API.
-->
count( (./prop[@class='remote' and @name=('ipv4-address', 'ipv6-address')]) or (./link[@rel='uri']) ) >=1
<!-- Constraint requirements:
       If ./prop[@name='direction' and @value='outgoing']
        If direction is outgoing there must be at least one remote ports entry. -->
count(./protocol[@name='remote']/port-range/@start) >= 1

Purpose of the OSCAL Content

To identify network traffic patterns for reviewers to know if it is properly risk-managed and conforms with FedRAMP requirements and best practices.

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

This is part of #807 and #808.

@aj-stein-gsa aj-stein-gsa moved this from 🆕 New to 🔖 Ready in FedRAMP Automation Nov 22, 2024
@DimitriZhurkin DimitriZhurkin self-assigned this Nov 26, 2024
@brian-ruf brian-ruf changed the title Network components need to declare the direction of network traffic Components representing inter-boundary communication need to declare the direction of network traffic Nov 26, 2024
@brian-ruf
Copy link
Contributor

brian-ruf commented Dec 2, 2024

@aj-stein-gsa / @DimitriZhurkin - it would be best to pause any constraint work dealing with /component/protocol until I finish my analysis of the "Ports, Protocols, and Services" section.
I am seeing some evidence that I need to rethink how this is represented for "interconnection" components.

In particular, I am expecting to make the following changes:

  • eliminate the use of "local" and "remote" in the protocol/@name attribute. That should be used for the IANA registered service name. (i.e. "http", "https", "smtp", etc.)
  • Move the "remote" protocol entry from the "interconnection" component to the "system" component that represents the far end of the interconnection.
  • sever the relationship between the "direction" property and the expectation of a protocol assembly as "direction" is about data flow, and was not intended to reflect which end initiates the connection.

@brian-ruf
Copy link
Contributor

brian-ruf commented Dec 3, 2024

These are still OK

  • count(./prop[@name='direction'])>=1
  • count(./prop[@name='direction' and @value='incoming']) <= 1
  • count(./prop[@name='direction' and @value='outgoing']) <= 1

These incorrectly conflate data direction with connection direction and should not be created or should be dropped if already created:

  • If direction is incoming there must be at least one local ipv4 address, ipv6 address, or URI to an API.
    count( (./prop[@Class='local' and @name=('ipv4-address', 'ipv6-address')]) or (./link[@rel='uri']) ) >=1

  • If direction is incoming there must be at least one local protocol entry.
    count(./protocol[@name='local']/port-range/@start)

-If direction is outgoing there must be at least one remote ipv4 address, ipv6 address or URI to an API.
count( (./prop[@Class='remote' and @name=('ipv4-address', 'ipv6-address')]) or (./link[@rel='uri']) ) >=1

  • If direction is outgoing there must be at least one remote ports entry.
    count(./protocol[@name='remote']/port-range/@start) >= 1

These incorrect items do not need to be replaced. This task can be closed when the first three count constraints are addressed.

@brian-ruf brian-ruf changed the title Components representing inter-boundary communication need to declare the direction of network traffic Components representing inter-boundary communication need to declare the direction of data flow Dec 3, 2024
@Gabeblis Gabeblis linked a pull request Dec 3, 2024 that will close this issue
7 tasks
@Gabeblis Gabeblis moved this from 🏗 In progress to 👀 In review in FedRAMP Automation Dec 3, 2024
DimitriZhurkin added a commit to DimitriZhurkin/fedramp-automation that referenced this issue Dec 5, 2024
@Gabeblis Gabeblis linked a pull request Dec 6, 2024 that will close this issue
7 tasks
@Gabeblis Gabeblis moved this from 👀 In review to 🚢 Ready to Ship in FedRAMP Automation Dec 6, 2024
@brian-ruf
Copy link
Contributor

We need to pause this and discuss it. There are two items that put any further work on this at risk:

  • the concept I proposed about bundling the data flow direction with the information type so that it is clear what information is crossing into the boundary and what is crossing out of the boundary.
  • a head's up from @david-waltermire that the PMO is considering revisions to boundary guidance that is likely to impact how we model and enforce this content in OSCAL.

@brian-ruf
Copy link
Contributor

brian-ruf commented Dec 10, 2024

Per conversations over the past two days there are actually three things happening with this issue.

Item 1: Potential changes to the FedRAMP requirements for Table 7-1 are "in the works" related to the pending boundary guidance; however, there is no clear ETA nor definition for this yet.

Decision on Item 1: We have agreed to continue working on OSCAL modeling, documentation, and constraints based on existing guidance. We will circle back to any changes once they are more clearly defined with a clearer ETA. We intend to insert OSCAL-rework into that boundary guidance release process.

Item 2: @DimitriZhurkin had started work on this and implemented the first constraint of four. I then realized that I had incorrectly used the "direction" property in the metapath. If we continue forward with this approach (vs Item #3 below) we need to rework the first constraint to use this revised metapath:

context="//component[
   (@type=('service', 'software') and not(./prop[@name='leveraged-authorization-uuid']) and  ./prop[@name='implementation-point' and @value='external'])
or
   (@type='interconnection')
or 
   (@type=('service', 'software') and ./prop[@name='implementation-point' and @value='internal'] and ./prop[@name='communicates-externally' and @value='yes' and @ns='http://fedramp.gov/ns/oscal'])
]"

Item 3: The current "direction" property in concert with the FedRAMP "information-type" prop/extension does not give a complete picture. It becomes ambiguous when the direction is both incoming and outgoing with more than one information type specified. We need clarity around which information is crossing the boundary on its way into the system vs crossing the boundary on its way out. As a result I proposed the use of @class on the "information-type" prop/extensions to indicate direction of boundary crossing. There has been no decision made on this proposal yet.


Regardless, the other constraints can still be worked, but with the revised target pasted in #2 above.

wandmagic added a commit to wandmagic/fedramp-automation that referenced this issue Dec 11, 2024
commit f010473
Author: wandmagic <[email protected]>
Date:   Tue Dec 10 15:08:00 2024 -0500

    re-introduce implemented-requirements constraints (GSA#981)

    * re-introduce implemented-requirements constraints

    * add doc available check for health url

    * fix spacing

    * Update src/validations/constraints/fedramp-external-constraints.xml

    Co-authored-by: Gabeblis <[email protected]>

    * Update src/validations/constraints/fedramp-external-constraints.xml

    Co-authored-by: Gabeblis <[email protected]>

    ---------

    Co-authored-by: Gabeblis <[email protected]>

commit c0ad00e
Author: Gabeblis <[email protected]>
Date:   Mon Dec 9 17:17:47 2024 -0500

    Adjust link for all profiles (GSA#979)

commit 8561600
Author: Gabeblis <[email protected]>
Date:   Mon Dec 9 11:27:48 2024 -0500

    Add Components To `information-type-800-60-v2r1` Allowed Values (GSA#973)

    * Add Leveraged Authorizations and External, Interconnected, and Unauthorized Systems components to information-type allowed values

    * Adjust constraint target

commit 788b67e
Author: Gabeblis <[email protected]>
Date:   Mon Dec 9 09:32:35 2024 -0500

    Fix constraint targets (GSA#974)

commit 9d7946c
Author: A.J. Stein <[email protected]>
Date:   Fri Dec 6 17:10:04 2024 -0500

    [chore] Update container image to cli v2.4.0 (GSA#971)

commit b2c9712
Author: Gabeblis <[email protected]>
Date:   Fri Dec 6 15:26:04 2024 -0500

    Add `used-by-link-references-component` constraint (GSA#972)

    * Add 'used-by-link-references-component' constraint

    * Fix message

    Co-authored-by: Kylie Hunter <[email protected]>

    * fix message

    Co-authored-by: DimitriZhurkin <[email protected]>

    ---------

    Co-authored-by: Kylie Hunter <[email protected]>
    Co-authored-by: DimitriZhurkin <[email protected]>

commit 3dac668
Author: Gabeblis <[email protected]>
Date:   Fri Dec 6 13:43:16 2024 -0500

    Add `component-has-used-by-link` constraint (GSA#970)

    * Add constraint 'protocol-has-used-by-link'

    * Fix message

    * Change constraint id

    * Fix message (last time)

    * Update src/validations/constraints/content/ssp-component-has-used-by-link-INVALID.xml

    Co-authored-by: A.J. Stein <[email protected]>

    ---------

    Co-authored-by: A.J. Stein <[email protected]>

commit c3db2b2
Author: DimitriZhurkin <[email protected]>
Date:   Thu Dec 5 13:07:39 2024 -0700

    Add inter-boundary-component-has-direction constraint (GSA#930) (GSA#968)

commit 5d6710f
Author: Gabeblis <[email protected]>
Date:   Thu Dec 5 13:32:28 2024 -0500

    Fix dev-constraint.js bug (GSA#967)

commit a7f9022
Author: Gabeblis <[email protected]>
Date:   Thu Dec 5 13:23:21 2024 -0500

    Add exists() to tests and remove duplicate constraint and fix system-implementation context (GSA#966)

    Remove duplicate constraint and fix system-implementation context

commit 780b38a
Author: wandmagic <[email protected]>
Date:   Thu Dec 5 12:50:29 2024 -0500

    Hotfix/deprecate all valid (GSA#960)

    * deprecate ssp-all-valid

    * Update src/validations/constraints/content/ssp-has-network-architecture-diagram-link-href-target-VALID-1.xml

    Co-authored-by: A.J. Stein <[email protected]>

    * Update src/validations/constraints/content/ssp-has-authorization-boundary-diagram-link-href-target-VALID-1.xml

    Co-authored-by: A.J. Stein <[email protected]>

    * Update src/validations/constraints/content/ssp-has-data-flow-diagram-link-href-target-VALID-1.xml

    Co-authored-by: A.J. Stein <[email protected]>

    * Update src/validations/constraints/content/ssp-has-network-architecture-diagram-link-href-target-VALID-1.xml

    Co-authored-by: A.J. Stein <[email protected]>

    * Update fedramp-ssp-example.oscal.xml

    ---------

    Co-authored-by: A.J. Stein <[email protected]>

commit 2c0e4de
Author: Gabeblis <[email protected]>
Date:   Thu Dec 5 10:21:00 2024 -0500

    Change cia-has-selected test (GSA#965)

commit 9a8e155
Author: wandmagic <[email protected]>
Date:   Wed Dec 4 15:30:29 2024 -0500

    Update fedramp-ssp-example.oscal.xml (GSA#959)

commit 5f7ce81
Author: Gabeblis <[email protected]>
Date:   Tue Dec 3 23:38:31 2024 +0000

    change example ssp location

commit 56f399e
Author: Gabeblis <[email protected]>
Date:   Tue Dec 3 23:23:59 2024 +0000

    Edit content to make constraints pass

commit d521a22
Author: Gabeblis <[email protected]>
Date:   Tue Dec 3 19:12:01 2024 +0000

    Delete extra ssp

commit 8cfb601
Author: Gabeblis <[email protected]>
Date:   Tue Dec 3 17:39:38 2024 +0000

    Add example ssp to content file and edit constraint script to point yaml pass file to example ssp

commit ff8f812
Author: ~ . ~ <[email protected]>
Date:   Tue Dec 3 13:50:22 2024 -0500

    fix ssp to pass tests

commit 85ec424
Author: Gabeblis <[email protected]>
Date:   Tue Dec 3 17:17:18 2024 +0000

    Add example ssp to content file and edit constraint script to point yaml pass file to example ssp

commit 7312686
Author: Kylie Hunter <[email protected]>
Date:   Mon Nov 25 16:15:01 2024 -0700

    Add connection-security prop constraint for GSA#931

commit 6ccb539
Author: Gabeblis <[email protected]>
Date:   Tue Dec 3 16:39:47 2024 -0500

    Add `issue-893` Constraints (GSA#949)

    * Add component-has-non-provider-responsible-role and tests

    * Add constraints and tests

    * Edit message

commit dd3be5f
Author: wandmagic <[email protected]>
Date:   Tue Dec 3 16:39:32 2024 -0500

    remove rev4 constraints (GSA#954)

commit 113c4f5
Author: Gabeblis <[email protected]>
Date:   Tue Dec 3 15:42:43 2024 -0500

    Fix Bug Issue GSA#940 (GSA#951)

commit c6f8e8f
Author: wandmagic <[email protected]>
Date:   Tue Dec 3 13:08:35 2024 -0500

    implementation point constraint (GSA#936)

    * implementation point constraint

    * add help uri

    * improve constraint

    * add extra fail content

    * Update src/validations/constraints/content/ssp-all-VALID.xml

    Co-authored-by: DimitriZhurkin <[email protected]>

    * Update fedramp-external-constraints.xml

    Co-authored-by: Rene Tshiteya <[email protected]>

    * implementation point constraint

    * add help uri

    * improve constraint

    * add extra fail content

    * Update src/validations/constraints/content/ssp-all-VALID.xml

    Co-authored-by: DimitriZhurkin <[email protected]>

    * Update fedramp-external-constraints.xml

    Co-authored-by: Rene Tshiteya <[email protected]>

    * add needed props to all valid

    * rebase

    Co-Authored-By: A.J. Stein <[email protected]>

    * Update src/validations/constraints/fedramp-external-constraints.xml

    Co-authored-by: A.J. Stein <[email protected]>

    ---------

    Co-authored-by: DimitriZhurkin <[email protected]>
    Co-authored-by: Rene Tshiteya <[email protected]>
    Co-authored-by: A.J. Stein <[email protected]>

commit 1377478
Author: Gabeblis <[email protected]>
Date:   Tue Dec 3 08:57:37 2024 -0500

    Add `component-responsible-role-references-party` constraint (GSA#945)

    * Add constraint 'component-responsible-role-references-party' and tests

    * correct test

    * Rename constraint and adjust help-url

    * Edit message

    Co-authored-by: A.J. Stein <[email protected]>

    ---------

    Co-authored-by: A.J. Stein <[email protected]>

commit a8461fb
Author: ~ . ~ <[email protected]>
Date:   Mon Dec 2 11:09:13 2024 -0500

    pin server + update oscal-js version

commit b82c417
Author: Gabeblis <[email protected]>
Date:   Mon Dec 2 14:07:05 2024 -0500

    Add `leveraged-authorization-has-valid-impact-level` Constraint (GSA#913)

    * Add leveraged-authorization constraint

    * rename constraint

    * fix constraint test

    * correct constraint test

    * Change 'http' to 'https'

    * Add level

commit 1db5f97
Author: Gabeblis <[email protected]>
Date:   Mon Dec 2 13:13:17 2024 -0500

    Constraints/cleanup constraints file (GSA#946)

    * clean up fedramp-external-constraints.xml

    * fix

    * Add message to fully-operational-date-type
@aj-stein-gsa
Copy link
Contributor Author

Item 3: The current "direction" property in concert with the FedRAMP "information-type" prop/extension does not give a complete picture. It becomes ambiguous when the direction is both incoming and outgoing with more than one information type specified. We need clarity around which information is crossing the boundary on its way into the system vs crossing the boundary on its way out. As a result I proposed the use of @class on the "information-type" prop/extensions to indicate direction of boundary crossing. There has been no decision made on this proposal yet.

Regardless, the other constraints can still be worked, but with the revised target pasted in #2 above.

Per discussion today, I support moving forward with item 2 and Item 3. Item 2 adds the prop[@name="communicates-externally"] and require the class on information types with a class of ingoing, outgoing, or doubled up for bi-directional like so.

         <prop name="information-type" value="C.3.5.1" class="incoming" ns="http://fedramp.gov/ns/oscal"/>
         <prop name="information-type" value="C.3.5.8" class="outgoing" ns="http://fedramp.gov/ns/oscal"/>

@brian-ruf and @Rene2mt agreed that's a good way forward. So the next steps:

  1. (Short-term) We need to adjust the relevant acceptance criteria for the open issues not ready to ship.
  2. (Medium to long-term) We need to notify the community even though it is develop, we are signaling the change is coming down the pike. We will send an email to the mailing list (and consider a website update if we need to call it out) and quickly brief in the next Implementers Meeting. (At a subsequent meeting we will do a deep dive.)

@brian-ruf or @Rene2mt, do I need to update the issues for 1? Did I understand correctly that it is just #930?

@aj-stein-gsa aj-stein-gsa moved this from 🚢 Ready to Ship to 🏗 In progress in FedRAMP Automation Dec 12, 2024
DimitriZhurkin added a commit to DimitriZhurkin/fedramp-automation that referenced this issue Dec 12, 2024
DimitriZhurkin added a commit that referenced this issue Dec 13, 2024
* Revise inter-boundary-component-has-direction constraint (#930)

* replace http with https

* apply comments
wandmagic pushed a commit that referenced this issue Dec 13, 2024
#930) (#994)

* Revise inter-boundary-component-has-direction constraint (#930)

* replace http with https

* apply comments

* Add inter-boundary-component-direction-incoming-has-ipv-uri constraint
@brian-ruf
Copy link
Contributor

@DimitriZhurkin consistent with @aj-stein-gsa decision above, please drop/remove any constraints related to the "direction" prop. The other constraints remain viable based on the redefined metapath in my above comment that replaced the "direction" prop with the "communicates-externally" prop/extension.

@DimitriZhurkin
Copy link

@brian-ruf, as regards "other constraints remain viable," they all include direction.

Please redefine constraints that need to be worked on. In the meantime, I'll remove the direction constraints and will pause working on this issue.

@brian-ruf
Copy link
Contributor

@DimitriZhurkin as promised hours ago, here is the final update, with the whole issue re-summarized.

Drop these:

  • count(./prop[@name='direction'])>=1
  • count(./prop[@name='direction' and @value='incoming']) <= 1
  • count(./prop[@name='direction' and @value='outgoing']) <= 1

Create/Revise These As Follows

context="//component[@type='interconnection']"

  • "interconnection" components must have at least one remote IPv4 Address, IPv6 Address or URI

    • target="."
    • count( (./prop[@class='remote' and @name=('ipv4-address', 'ipv6-address')]) or (./link[@rel='uri']) ) >=1
  • "interconnection" components must have at least one local IPv4 Address, IPv6 Address or URI

    • target="."
    • count( (./prop[@class='local' and @name=('ipv4-address', 'ipv6-address')]) or (./link[@rel='uri']) ) >=1
  • For an "interconnection" component, at least one of the "used-by" linked components must have at least one protocol assembly.

    • target="."
    • count( //component[@uuid=//component[@type='interconnection']/link[@rel='used-by']/substring-after(@href, "#")]/protocol ) >=1

NOTE that the protocol construct had to move to one of the "used-by" components as there was no way within the protocol construct to indicate which end was "listening" on those ports if that construct remained within the "interconnection" component. (Core OSCAL states the @name attribute is supposed to contain the protocol name, and the title field is markup-line, which is not intended for machine readable values.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment