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

Progress on MS extension support #22

Merged
merged 2 commits into from
Feb 14, 2024
Merged

Conversation

Firstyear
Copy link
Member

Add support for MS OAPXBC key management in the soft-tpm, with a view to have this work with HW TPM's.

Relates kanidm/hsm-crypto#30

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

@Firstyear Firstyear marked this pull request as draft February 8, 2024 08:49
@dmulder
Copy link
Collaborator

dmulder commented Feb 8, 2024

Hrm, maybe I can make things work using msoapxbc_rsa_key_import()?
I guess I need to understand your vision here. How would you expect someone to use the msextensions features of compact_jwt without the hsm-crypto features? How would I use compact_jwt without the TPM/soft hsm?

@Firstyear
Copy link
Member Author

Hrm, maybe I can make things work using msoapxbc_rsa_key_import()? I guess I need to understand your vision here. How would you expect someone to use the msextensions features of compact_jwt without the hsm-crypto features? How would I use compact_jwt without the TPM/soft hsm?

My thinking here is that we want to model our implementation around the TPM as the key storage mechanism. That's why to me I wanted to create this such that you have to use the TPM to protect the key.

Put the other way, is there a reason someone would want to use this feature without their machine identity keys being in a TPM? Part of the thinking about this is to "raise the bar" as well - we want users of this library to be doing the right thing, so why should we let them use keys without protecting those keys appropriately? There is a bigger block of work in parallel to this to have everything cryptographic in kanidm and related libraries be protected in a TPM or HSM.

Initially I think we will be the only user, and our interfaces (inside the kani unixd framework) all rely on the tpm and provide it, so we can always rely on it being there. For testing and users without a HW TPM, the softtpm interface exists to fill that gap. And if we need it, we can always use the rsa key import feature that I added to the softtpm for tests.

@Firstyear
Copy link
Member Author

PS: This isn't saying I'm hard-against adding it back. I just think we should have a good reason to add it back because there are risks entailed and I do like to "bring up" the quality standard of all our users around us. :)

@dmulder
Copy link
Collaborator

dmulder commented Feb 9, 2024

PS: This isn't saying I'm hard-against adding it back. I just think we should have a good reason to add it back because there are risks entailed and I do like to "bring up" the quality standard of all our users around us. :)

Well, my only argument in favor of providing it without the TPM is that it's slightly easier for testing! I suppose that's not a very strong argument. Let's stick with TPM/soft hsm only.

@dmulder
Copy link
Collaborator

dmulder commented Feb 9, 2024

Well, I've integrated this in himmelblau (via my msal library), and all looks good! I did some basic testing, and all the fundamentals are working as expected (with me splitting the transport and certificate keys).

@Firstyear
Copy link
Member Author

@dmulder Sounds good. I'll do some more on this today then, I wanted your feedback first before I kept going deeper into this :)

Did you want me to add the bcrypt format export to hsm-crypto too here then?

@dmulder
Copy link
Collaborator

dmulder commented Feb 12, 2024

Did you want me to add the bcrypt format export to hsm-crypto too here then?

I don't think it matters. Up to you.

@Firstyear
Copy link
Member Author

for now I think we leave it as is then,

@Firstyear Firstyear marked this pull request as ready for review February 14, 2024 05:12
@Firstyear Firstyear merged commit c9ce2fa into main Feb 14, 2024
1 check passed
@Firstyear Firstyear deleted the 20240208-ms-extension-support branch February 14, 2024 05:13
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