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

message-signatures: fix Content-Digest sha-512 digest on response #2049

Merged
merged 1 commit into from
Mar 30, 2022

Conversation

bobby-stripe
Copy link
Contributor

@bobby-stripe bobby-stripe commented Mar 29, 2022

I'm not sure what the current digest is, but it doesn't match the 23-byte HTTP body of that message.
We can verify the expected/new digest with:

$ echo -n '{"message": "good dog"}' | sha512sum | cut -d ' ' -f 1 | xxd -r -p | base64
mEWXIS7MaLRuGgxOBdODa3xqM1XdEvxoYhvlCFJ41QJgJc4GTsPp29l5oGX69wWdXymyU0rjJuahq4l5aGgfLQ==

(also verified in a go implementation of message signatures)

As the digest changed, the Signature for the ECDSA P-256 example also changed, I've included a new signature over the updated digest that verifies based on the key in the spec.

Thanks!

I'm not sure what the current digest is, but it doesn't match the 23-byte HTTP body of that message.
We can verify the new digest with:

$ echo -n '{"message": "good dog"}' | sha512sum | cut -d ' ' -f 1 | xxd -r -p | base64
mEWXIS7MaLRuGgxOBdODa3xqM1XdEvxoYhvlCFJ41QJgJc4GTsPp29l5oGX69wWdXymyU0rjJuahq4l5aGgfLQ==

(also verified in a go implementation of message signatures)

As the digest changed, the Signature for the ECDSA P-256 example also changed, I've included a new
signature over the updated digest that verifies based on the key in the spec.
@jricher
Copy link
Contributor

jricher commented Mar 29, 2022

@bobby-stripe Thanks, the signature examples are generated from a script, so it should be patched there first or else any errors might creep back in with future updates. These seem to be the offending lines:

https://github.com/bspk/sig-example-scripts/blob/main/http_sig_examples.py#L185-L191

We should probably be calculating the digest value within the appropriate section of the script here instead of hardcoding it in the source:

https://github.com/bspk/sig-example-scripts/blob/main/http_sig_examples.py#L1063-L1128

Once the generator is patched so that it gives the right values, we can take in this PR.

@yaronf
Copy link
Contributor

yaronf commented Mar 29, 2022

Both changes validate for me. Thanks!

bobby-stripe added a commit to bobby-stripe/sig-example-scripts that referenced this pull request Mar 29, 2022
related to: httpwg/http-extensions#2049

I'm not sure what the current digest is, but it doesn't match the 23-byte HTTP body of that message.
We can verify the expected/new digest with:

$ echo -n '{"message": "good dog"}' | sha512sum | cut -d ' ' -f 1 | xxd -r -p | base64
mEWXIS7MaLRuGgxOBdODa3xqM1XdEvxoYhvlCFJ41QJgJc4GTsPp29l5oGX69wWdXymyU0rjJuahq4l5aGgfLQ==
bobby-stripe added a commit to bobby-stripe/sig-example-scripts that referenced this pull request Mar 30, 2022
…essages

related to: httpwg/http-extensions#2049

I'm not sure what the current digest is, but it doesn't match the 23-byte HTTP body of that message.
We can verify the expected/new digest with:

$ echo -n '{"message": "good dog"}' | sha512sum | cut -d ' ' -f 1 | xxd -r -p | base64
mEWXIS7MaLRuGgxOBdODa3xqM1XdEvxoYhvlCFJ41QJgJc4GTsPp29l5oGX69wWdXymyU0rjJuahq4l5aGgfLQ==
@bobby-stripe
Copy link
Contributor Author

thanks @jricher ! added a PR there: https://github.com/bspk/sig-example-scripts/pull/2/files

@jricher
Copy link
Contributor

jricher commented Mar 30, 2022

I've updated the example generation package to generate the digest values throughout all the examples, and that's at bspk/sig-example-scripts#3 now. The script now generates the same content digest as the PR here, and this is included in the signature base.

"@status": 200
"content-type": application/json
"content-digest": sha-512=:mEWXIS7MaLRuGgxOBdODa3xqM1XdEvxoYhvlCFJ4\
  1QJgJc4GTsPp29l5oGX69wWdXymyU0rjJuahq4l5aGgfLQ==:
"content-length": 23
"@signature-params": ("@status" "content-type" "content-digest" "co\
  ntent-length");created=1618884473;keyid="test-key-ecc-p256"

The changes need to be propagated to httpsig.org as well, but we'll track that separately.

Thanks for finding and fixing this issue with the spec's examples!

@jricher jricher merged commit c63f79d into httpwg:main Mar 30, 2022
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.

None yet

3 participants