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

Provide the ability to specify the header x5c #13

Merged
merged 2 commits into from
Dec 13, 2023

Conversation

dmulder
Copy link
Collaborator

@dmulder dmulder commented Dec 12, 2023

Himmelblau requires this for requesting a PRT.

  • cargo fmt has been run
  • cargo test has been run and passes
  • documentation has been updated with relevant examples (if relevant)

@Firstyear
Copy link
Member

I think this can be expanded on a bit:

First, we probably actually should make it so kid can be set or not set - we enforce it right now by default, but we should have a way to say "sorry I don't want it.". Probably when you set x5c, it should cause the kid to be cleared?

The other point is getting x509 certs are both vec https://github.com/kanidm/hsm-crypto/blob/main/src/lib.rs#L466 meaning we should probably have this take a vec, and use that, allowing us to format it in the correct way with base64.

Better, could we actually just pull the x5c from internally rather than this dance when we have a jwtSigner? Some way to have the JwtSigner automatically know if it should provide x5c vs kid? For example, https://github.com/kanidm/compact-jwt/blob/main/src/crypto/es256.rs#L257 which actually sets the kid. For our TPM type we could automatically use this to switch between x5c and kid if a x509 cert exists on the identity key.

Signed-off-by: David Mulder <dmulder@samba.org>
Signed-off-by: David Mulder <dmulder@samba.org>
@dmulder
Copy link
Collaborator Author

dmulder commented Dec 13, 2023

Better, could we actually just pull the x5c from internally rather than this dance when we have a jwtSigner? Some way to have the JwtSigner automatically know if it should provide x5c vs kid? For example, https://github.com/kanidm/compact-jwt/blob/main/src/crypto/es256.rs#L257 which actually sets the kid. For our TPM type we could automatically use this to switch between x5c and kid if a x509 cert exists on the identity key.

I've modified it to pull the x5c directly from the key, but I also left set_x5c() in case someone is trying to set a chain.

@dmulder
Copy link
Collaborator Author

dmulder commented Dec 13, 2023

Unfortunately removing the kid did not resolve the issue I'm facing in the himmelblau PRT req :-(

@dmulder
Copy link
Collaborator Author

dmulder commented Dec 13, 2023

Unfortunately removing the kid did not resolve the issue I'm facing in the himmelblau PRT req :-(

I got it working. My JWK containing the transport key which I sent during the join wasn't correct. Apparently Microsoft doesn't validate that transport key in any way until it first gets used! So it accepted garbage during the join and stored it for later.

@Firstyear Firstyear merged commit fd9c14f into kanidm:main Dec 13, 2023
1 check passed
@Firstyear
Copy link
Member

I got it working. My JWK containing the transport key which I sent during the join wasn't correct. Apparently Microsoft doesn't validate that transport key in any way until it first gets used! So it accepted garbage during the join and stored it for later.

Opppppsss. That's a bit of a rough discovery.

@dmulder dmulder deleted the dmulder/x5c_header branch December 14, 2023 15:50
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