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

doc: clarify SCMP reality vs. ambition #4533

Merged
merged 2 commits into from
May 27, 2024

Conversation

matzf
Copy link
Contributor

@matzf matzf commented May 24, 2024

The SCMP "specification" document described support for SCMP as mandatory. Furthermore, it mandates the use of SPAO+DRKey authentication for SCMPs. This does not correspond to what is currently implemented. SCMP support has been treated as at least semi-optional e.g. in discussions around the dispatcher-less end host stack.

Regarding the authentication, there is no full implementation of this neither in router (support in "our" router is experimental/demo-level, no other router implementation supports it at all) nor in end hosts (currently, nothing at all). There is not sufficiently clear consensus around this to pretend that we'll realize this ambition shortly.

Update the documentation specs to reflect this reality. Move the SCMP authentication description into a design document.

Closes #4528

@matzf
Copy link
Contributor Author

matzf commented May 24, 2024

This change is Reviewable

@matzf matzf requested a review from jiceatscion May 24, 2024 08:20
Copy link
Contributor

@jiceatscion jiceatscion left a comment

Choose a reason for hiding this comment

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

Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @matzf)


doc/dev/design/scmp-authentication.rst line 37 at r1 (raw file):

.. _scmp-spao:

I have a bit of meta-comment on this document: it is hard to figure out under what circumstances SCMP authentication is required or not in relations with the fate of the proposal. It might help to have a paragraph that states something like (if I interpreted the doc correctly):

Should this proposal be adopted, the following rules apply:
* SCMP support is optional
* If SCMP is supported it must behave as described in this document.

As a result, the rest of this document uses the terms MUST and MUST NOT, as they apply to SCMP support being intended. If SCMP support is not intended then these rules do NOT become optional; instead the implementation simply MUST NOT send or receive SMP messages at all.

Once you said that, you could go on your merry way throughout the document and use only SHOULD when something is optional per the authenticated scmp protocol and MUST the rest of the time. This is in contrast with the current state where one wonders if "SHOULD" means "MUST-unless-scmp-is-not-supported", or if it means "NOT-MANDATORY-even-if-scmp-is-supported."

Only a suggestion.

Copy link
Contributor Author

@matzf matzf left a comment

Choose a reason for hiding this comment

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

Reviewable status: 3 of 4 files reviewed, all discussions resolved (waiting on @jiceatscion)


doc/dev/design/scmp-authentication.rst line 37 at r1 (raw file):

Previously, jiceatscion wrote…

I have a bit of meta-comment on this document: it is hard to figure out under what circumstances SCMP authentication is required or not in relations with the fate of the proposal. It might help to have a paragraph that states something like (if I interpreted the doc correctly):

Should this proposal be adopted, the following rules apply:
* SCMP support is optional
* If SCMP is supported it must behave as described in this document.

As a result, the rest of this document uses the terms MUST and MUST NOT, as they apply to SCMP support being intended. If SCMP support is not intended then these rules do NOT become optional; instead the implementation simply MUST NOT send or receive SMP messages at all.

Once you said that, you could go on your merry way throughout the document and use only SHOULD when something is optional per the authenticated scmp protocol and MUST the rest of the time. This is in contrast with the current state where one wonders if "SHOULD" means "MUST-unless-scmp-is-not-supported", or if it means "NOT-MANDATORY-even-if-scmp-is-supported."

Only a suggestion.

Done, added such an intro.
Regarding the SHOULDs; I think the intention was "you really should, but you're putting yourself at risk if you leave it out". This was probably not very useful and more open than necessary. I've changed this to MUST in two cases (the receiver MUST check the auth header and MUST drop the packet if its bad).

Copy link
Contributor

@jiceatscion jiceatscion left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @matzf)

matzf added 2 commits May 27, 2024 14:13
The SCMP "specification" document described support for SCMP as
mandatory. Furthermore, it mandates the use of SPAO+DRKey authentication
for SCMPs. This does not correspond to what is currently implemented.
SCMP support has been treated as at least semi-optional e.g. in
discussions around the dispatcher-less end host stack.

Regarding the authentication, there is no full implementation of this
neither in router (support in "our" router is experimental/demo-level,
no other router implementation supports it at all) nor in end hosts
(currently, nothing at all). There is not sufficiently clear consensus
around this to pretend that we'll realize this ambition shortly.

Update the documentation specs to reflect this reality.
Move the SCMP authentication description into a design document.
@matzf matzf force-pushed the doc-scmp-optional branch from c5526aa to 34eb4a0 Compare May 27, 2024 12:13
Copy link
Contributor Author

@matzf matzf left a comment

Choose a reason for hiding this comment

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

Reviewed 3 of 4 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @matzf)

@matzf matzf merged commit 9d52e2f into scionproto:master May 27, 2024
4 checks passed
@matzf matzf deleted the doc-scmp-optional branch May 27, 2024 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

doc: clarify SCMP reality vs. ambition
2 participants