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

Add support of hashes in base64 url encoded format without padding. End goal to support x5t (X.509 Certificate SHA-1 Thumbprint), x5t#S256 (X.509 Certificate SHA-256 Thumbprint) #2849

Closed
johanneslarsson opened this issue Oct 30, 2020 · 3 comments · Fixed by #2867

Comments

@johanneslarsson
Copy link
Contributor

johanneslarsson commented Oct 30, 2020

Expected Behaviour

As part of a custom JWS validation there is a need to calculate these hashes; x5t and x5t#S256 from a x509 public certificate.

Example:

x5t_s256 = r {
    r = crypto.sha256_base64url_encode(base64.decode(certs[0]["Raw"]))
}

The RFC https://tools.ietf.org/html/rfc7515#section-4.1.8, mentions that base64url-encoded SHA-256 thumbprint (a.k.a. digest) of the DER encoding of the X.509 certificate. Appendix C clarify that the base64 encoding must be unpadded (https://tools.ietf.org/html/rfc7515#appendix-C).

Actual Behaviour

All existing hashing algorithms (crypto.md5, crypto.sha1 and crypto.sha256) hex encode the result. Another solution could be to introduce hex decoding, but I'm not sure how useful it would be.

Additional Info

Feature branch:
master...johanneslarsson:feature/base64_url_encode_hash

@patrick-east
Copy link
Contributor

Another solution could be to introduce hex decoding, but I'm not sure how useful it would be.

I wonder about that, the function as described sounds pretty specific.. I would lean towards adding smaller building blocks that we could use with the existing hash functions and our encoding ones. Although I'm not sure how we could handle it. @tsandall any ideas? I guess the root of the problem is we essentially want to pass around a byte array, its not clear how best to do that.

@johanneslarsson
Copy link
Contributor Author

johanneslarsson commented Oct 31, 2020

Yes if these functions most likely won't be used it's also possible to add these x5t, x5t_256 fields to the certificate parsing, but that would make the Rego certificate struct to deviate from Golang.

Example:

x5t_s256 = r {
    r = certs[0]["x5t_s256"]
}

@johanneslarsson
Copy link
Contributor Author

Another solution could be to introduce hex decoding, but I'm not sure how useful it would be.

I wonder about that, the function as described sounds pretty specific.. I would lean towards adding smaller building blocks that we could use with the existing hash functions and our encoding ones. Although I'm not sure how we could handle it. @tsandall any ideas? I guess the root of the problem is we essentially want to pass around a byte array, its not clear how best to do that.

This feature branch implements three builtins; base64url.encode_no_pad, hex.encode and hex.decode.
master...johanneslarsson:feature/hex_encoding

Example:

x5t_s256 = r {
    r = base64url.encode_no_pad(hex.decode(crypto.sha256(base64.decode(certs[0]["Raw"]))))
}

johanneslarsson added a commit to johanneslarsson/opa that referenced this issue Nov 9, 2020
The builtin hash algorithms hex encode the result. To use the hash functions
there must be possible to decode the value and re-encode it in the expected format.
This enables validation of x5t/x5t_s256, which are base64 url encoded hashes.

Fixes: open-policy-agent#2849
Signed-off-by: Johannes Larsson <johannes.a.larsson@gmail.com>
tsandall pushed a commit that referenced this issue Nov 9, 2020
The builtin hash algorithms hex encode the result. To use the hash functions
there must be possible to decode the value and re-encode it in the expected format.
This enables validation of x5t/x5t_s256, which are base64 url encoded hashes.

Fixes: #2849
Signed-off-by: Johannes Larsson <johannes.a.larsson@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants