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

Tweak signature verification #15069

Merged

Conversation

ClearlyClaire
Copy link
Contributor

  • check request body digest separately to give software implementers more accurate feedback in case the signature check fails
  • relax the requirement for the Host header to be signed for POST requests, to restore compatibility with software that did not sign it (I'm not sure there is any software that was broken by this single change)

This may help other implementors debug their implementation.
The only POST requests processed by Mastodon need objects/actors (including
their host) to be explicitly mentioned in the request's body, so replaying
a legitimate request to another host should not be a security issue.
@ClearlyClaire ClearlyClaire force-pushed the fixes/signature-verification-tweaks branch from 9a91a2e to ca567c7 Compare October 30, 2020 12:12
@Gargron
Copy link
Member

Gargron commented Oct 30, 2020

Relax the requirement for the Host header to be signed for POST requests, to restore compatibility with software that did not sign it (I'm not sure there is any software that was broken by this single change)

Not sure why this? If I understand correctly, without Host being signed, the same signature could be used on multiple different recipients in the allowed time window? Also, if no known software was broken by this requirement, why remove it?

@ClearlyClaire
Copy link
Contributor Author

Relax the requirement for the Host header to be signed for POST requests, to restore compatibility with software that did not sign it (I'm not sure there is any software that was broken by this single change)

Not sure why this? If I understand correctly, without Host being signed, the same signature could be used on multiple different recipients in the allowed time window?

Yes, that is an issue with GET. But as far as POST is concerned, we won't do anything if we haven't received an Activity targeting one of our actors, so something targeting another fediverse instance would be irrelevant to us and we would discard it even if it were replayed.

Also, if no known software was broken by this requirement, why remove it?

#15016 (comment) but I don't know if more are affected.

Either way, I'm not opposed to drop this PR, since we already have a release with that restriction anyway

@ClearlyClaire ClearlyClaire force-pushed the fixes/signature-verification-tweaks branch from 14223c7 to 175b9a6 Compare November 1, 2020 12:46
@ClearlyClaire
Copy link
Contributor Author

Changed it again to accept actually valid Digest headers that include multiple hashes (provided sha-256 is in it) or use different casing for the sha-256 algorithm (the specs says algorithm names are case-insensitive and that lowercase is preferred, so our current code is wrong)

@Gargron Gargron merged commit fa929d8 into mastodon:master Nov 1, 2020
umonaca pushed a commit to umonaca/mastodon that referenced this pull request Nov 8, 2020
* Add more specific error message when request body digest is invalid

This may help other implementors debug their implementation.

* Relax Host parameter requirement to GET requests

The only POST requests processed by Mastodon need objects/actors (including
their host) to be explicitly mentioned in the request's body, so replaying
a legitimate request to another host should not be a security issue.

* Support Digest headers using multiple algorithms or lowercase alogirthm names
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.

None yet

2 participants