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: #1190. Remove algorithm specific rules for content identifiers. #1314

Merged
merged 4 commits into from Feb 19, 2021
Merged
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 4 additions & 4 deletions draft-ietf-httpbis-message-signatures.md
Expand Up @@ -377,8 +377,8 @@ The following sections describe each of these steps in detail.
5. The signer creates an ordered list of content identifiers representing the message content and signature metadata to be covered by the signature, and assigns this list as the signature's Covered Content.
* Each identifier MUST be one of those defined in Section 2.
* This list MUST NOT be empty, as this would result in creating a signature over the empty string.
* If the signature's Algorithm name does not start with rsa, hmac, or ecdsa, signers SHOULD include `*created` and `*request-target` in the list.
* If the signature's Algorithm starts with rsa, hmac, or ecdsa, signers SHOULD include `date` and `*request-target` in the list.
* Signers SHOULD include `*created` and `*request-target` in the list.
jricher marked this conversation as resolved.
Show resolved Hide resolved
* Signers SHOULD include `date` and `*request-target` in the list.
ioggstream marked this conversation as resolved.
Show resolved Hide resolved
* Further guidance on what to include in this list and in what order is out of scope for this document. However, the list order is significant and once established for a given signature it MUST be preserved for that signature.


Expand Down Expand Up @@ -413,9 +413,9 @@ The following table presents a non-normative example of metadata values that a s

The Signature Input is a US-ASCII string containing the content that will be signed. To create it, the signer concatenates together entries for each identifier in the signature's Covered Content in the order it occurs in the list, with each entry separated by a newline `"\n"`. An identifier's entry is a US-ASCII string consisting of the lowercased identifier followed with a colon `":"`, a space `" "`, and the identifier's canonicalized value (described below).

ioggstream marked this conversation as resolved.
Show resolved Hide resolved
If Covered Content contains `*created` and the signature's Creation Time is undefined or the signature's Algorithm name starts with `rsa`, `hmac`, or `ecdsa` an implementation MUST produce an error.

Choose a reason for hiding this comment

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

Should not remove this entirely. Implementations should still produce an error if the corresponding header for a signature parameter is not found.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is the suggestion that this constraint be applied regardless of algorithm name?

I completely agree that this "algorithm name starts with" language is hugely problematic.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@llorllale @jricher right! Both of you are correct! Fixing...

Choose a reason for hiding this comment

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

@jricher

Is the suggestion that this constraint be applied regardless of algorithm name?

Yes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

PTAL

If Covered Content contains `*created` and the signature's Creation Time is undefined an implementation MUST produce an error.

If Covered Content contains `*expires` and the signature does not have an Expiration Time or the signature's Algorithm name starts with `rsa`, `hmac`, or `ecdsa` an implementation MUST produce an error.

Choose a reason for hiding this comment

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

Same here - should error if not found

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thx @llorllale, I fixed it below: does it work for you?

If Covered Content contains `*expires` and the signature does not have an Expiration Time an implementation MUST produce an error.

If Covered Content contains an identifier for a header field that is not present or malformed in the message, the implementation MUST produce an error.

Expand Down