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

Fix: #1954. No normative text in security considerations. #1972

Merged
merged 3 commits into from
Feb 18, 2022

Conversation

ioggstream
Copy link
Contributor

This PR

signatures.
As explained in {{sec-limitations}}, integrity fields rely
on transport layer integrity protection mechanisms,
while {{algorithms}} provides further guidance on algorithm's choice.
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks.

Seeing it like this, is there even a need to say anything here? The referenced sections apply to the usage of digest and don't appear to add any further insight or considerations when used with Signature.

@jricher WDYT about just deleting the two sentences and not adding anything more?

Copy link
Contributor

Choose a reason for hiding this comment

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

I actually like the cross-reference here, I would keep it this way.

Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting. If someone is using Digests with signature, they should really apply all the recommondations and apply all the considerations of the document. There seems to me no need to call out any particular section as unique to Signatures, unless there is something to say about why. Is that the case here?

The rest of this section does achieve the goal of signature-specific considerations. It's just these that stick out to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@LPardue {{algorithms}} actually references a signature-related normative text, so I think it should stay. The TLS part in 2022 is probably redundant for a careful reader.

Can we agree on removing the transport layer text and leave the {{algorithms}} reference? cc: @jricher

Copy link
Contributor

Choose a reason for hiding this comment

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

I still think they can both stay, but at this stage I'll leave it up to your discretion if you think it is clearer without them.

Copy link
Contributor

Choose a reason for hiding this comment

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

So reading the text in {{algorithms}} the relevant part seems to be about saying signatures is a potential adversarial setting.

But we already have the Algorithm Agility security consideration section, which now contains nice text about considerations for choosing secure or insecure algorithms.

So instead I would say

"Signatures are likely to be deemed an adversarial setting when applying integrity digests; see {{algorithm agility}}."

And still drop the part about transport integrity.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

deal!

signatures.
As explained in {{sec-limitations}}, integrity fields rely
on transport layer integrity protection mechanisms,
while {{algorithms}} provides further guidance on algorithm's choice.
Copy link
Contributor

Choose a reason for hiding this comment

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

I actually like the cross-reference here, I would keep it this way.

@ioggstream ioggstream merged commit f3fe8bf into main Feb 18, 2022
@ioggstream ioggstream deleted the ioggstream-1954 branch February 18, 2022 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

No normative statements in Security Considerations
3 participants