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

Unusual space inserted in signature string for Basic Reference Test #38

Open
liamdennehy opened this issue Oct 12, 2018 · 2 comments
Open
Labels
bug Defect or missing content/context quality Clarity, consistency, effectiveness

Comments

@liamdennehy
Copy link
Contributor

While implementing Reference Tests in http-signatures-php I noticed I could not generate the provided Authorization Header in the Basic Test under Reference Tests, as it has a space after a parameter delimiter:
headers="(request-target) host date", signature="qdx+H7PHHDZgy4...

Is this intentional? A Signature generator would not generate this line, so fail this reference test, and the specification doesn't mention being tolerant of spaces in between parameter keys for verification. I have scanned the document and can't actually find anywhere it explicitly describes how to compose the header line and the use of delimiters to separate parameters.

@msporny
Copy link
Contributor

msporny commented Oct 12, 2018

Hey @liamdennehy -- thanks for submitting the issue.

I'm not quite sure which space you're referring to. The space in the headers parameter, or the one between date", signature, or the line break?

In general, space between delimiters outside of double quotes is something that the HTTP spec covers. Delimiters inside of double quotes are typically covered by the spec, which usually defers to best practices (again, referring to a more base level HTTP spec). Does that help in any way?

I'll be able to provide more information once I know which specific space you're referring to.

Also, man oh man, do we need a test suite for HTTP Signatures. :( -- it's on the TODO list but everyone has been busy using it vs. locking down the implementations. Perhaps we can get @cwebber to help here... or some other brave volunteer.

@liamdennehy
Copy link
Contributor Author

liamdennehy commented Oct 12, 2018

Hey @msporny. Indeed, the date", signature space (I take the line breaks as simple wraps without spaces). In all other examples it's ",signature= without a space, so this looks like a typo.
The Reference Tests I'm implementing should both generate and validate the test data:

  • This is a bit random since all other reference lines have no spaces, so this should be removed for consistency unelss it is actually illustrating something specific e.g. tolerance of different header line layouts.
  • If the HTTP spec covers how to format the header line should we refer to it explicitly (if we haven't already)? It's a bit obvious from the examples that it's key="value1 value2",key2=..., but we haven't actually spelled it out or referred to a source that does (unless I've missed it).

On the final point, ping me directly by mail? I've basically got this fully covered in https://github.com/liamdennehy/http-signatures-php/blob/reference-tests/tests/ReferenceTest.php, waiting on two PRs to be merged in upstream (optional headers parameter and full RSA support) before this can go in.

@liamdennehy liamdennehy added bug Defect or missing content/context quality Clarity, consistency, effectiveness labels Aug 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Defect or missing content/context quality Clarity, consistency, effectiveness
Projects
None yet
Development

No branches or pull requests

2 participants