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

Clean up signature references and text #2143

Merged
merged 15 commits into from
Jun 17, 2022
Merged

Clean up signature references and text #2143

merged 15 commits into from
Jun 17, 2022

Conversation

jricher
Copy link
Contributor

@jricher jricher commented Jun 1, 2022

Multiple editorial and example cleanups:

@@ -1790,6 +1793,20 @@ From here, the signing process proceeds as usual.

Upon verification, it is important that the verifier validate not only the signature but also the value of the Content-Digest field itself against the actual received content. Unless the verifier performs this step, it would be possible for an attacker to substitute the message content but leave the Content-Digest field value untouched. Since only the field value is covered by the signature directly, checking only the signature is not sufficient protection against such a substitution attack.

## Non-List Field Values {#security-non-list}

When an HTTP field occurs multiple times in a single message, these values need to be combined into a single one-line string value to be included in the HTTP signature base, as described in {{create-sig-input}}. Not all HTTP fields can be combined into a single value and still be a valid value for the field. However, for the purposes of generating the signature base, the message component value is never read back out of the signature base string and used in anyway. Therefore it is considered best practice to treat the signature base generation algorithm separately from processing the field values by the application, particularly for fields that are known to have this property.
Copy link
Contributor

Choose a reason for hiding this comment

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

"and used in anyway" ?


When an HTTP field occurs multiple times in a single message, these values need to be combined into a single one-line string value to be included in the HTTP signature base, as described in {{create-sig-input}}. Not all HTTP fields can be combined into a single value and still be a valid value for the field. However, for the purposes of generating the signature base, the message component value is never read back out of the signature base string and used in anyway. Therefore it is considered best practice to treat the signature base generation algorithm separately from processing the field values by the application, particularly for fields that are known to have this property.

For example, the Set-Cookie field {{COOKIE}} defines an internal syntax that does not conform to the List syntax in {{STRUCTURED-FIELDS}}. This field is also typically sent as multiple fields with distinct values when sending multiple cookies. When multiple Set-Cookie fields are sent in the same message, it is not possible to combine these into a single and be able to parse and use the results, as discussed in {{HTTP, Section 5.3}}. Therefore, all the cookies need to be processed from their separate header values, without being combined, while the signature base needs to be processed from the special combined value generated solely for this purpose.
Copy link
Contributor

Choose a reason for hiding this comment

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

This misses the attack.

Say you have:

Set-Cookie: that, happens, to, contain, a, comma

An attacker is now able to construct a variant message that produces the same signature:

Set-Cookie: that, happens
Set-Cookie: to, contain, a, comma

This will be processed differently, but it will be signed and treated as such.

For me, this is an unacceptable property of a signature scheme.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I didn't call out the commas specifically here but I'll tweak the text to address that.

@jricher jricher merged commit 48485cd into main Jun 17, 2022
@jricher jricher deleted the text-ref-cleanup branch June 17, 2022 20:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants