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

support content detached JWS #310

Merged
merged 2 commits into from
Jun 28, 2021
Merged

support content detached JWS #310

merged 2 commits into from
Jun 28, 2021

Conversation

xli
Copy link
Contributor

@xli xli commented Jun 26, 2021

  1. Extract out JWS module without offchain types to diem package. We will need jws encoding and decoding message for mini-wallet API authentication later.
  2. Add content detached JWS support.

see #311 for the full plan.

src/diem/jws.py Outdated Show resolved Hide resolved
tests/test_jws.py Outdated Show resolved Hide resolved


def encode_headers(headers: Dict[str, Any]) -> bytes:
return encode_b64url(json.dumps(headers, separators=(",", ":")).encode(ENCODING))
Copy link
Contributor

@CapCap CapCap Jun 28, 2021

Choose a reason for hiding this comment

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

is this going to return the same thing for {"two":"two","one":"one"} as it would for {"one":"one","two":"two"}? Should it? These headers are functionally equivalent, but here we would be depending on implementation to ensure the headers we get are ordered the same on different machines, right? And if they're not the same order, we could consider this signature invalid; is this the expected behaviour?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They are different, and it does not matter.

When you validate signature, you verify against the JWS message header and body parts, which is the result of this function.
As long as you can decode the bytes back to a dict with same keys and values, then it's fine.

@xli xli merged commit 37f539e into diem:master Jun 28, 2021
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.

2 participants