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

RFE: cryptenroll custom TPM2 PCR values #19204

Closed
grawity opened this issue Apr 3, 2021 · 7 comments
Closed

RFE: cryptenroll custom TPM2 PCR values #19204

grawity opened this issue Apr 3, 2021 · 7 comments
Labels
cryptsetup RFE 🎁 Request for Enhancement, i.e. a feature request tpm2

Comments

@grawity
Copy link
Contributor

grawity commented Apr 3, 2021

Is your feature request related to a problem? Please describe.

I would like to use systemd-cryptenroll --tpm2-pcrs= against the PCR values I specify, instead of the ones currently read from the TPM.

Describe the solution you'd like

An option to provide the PCR values as a file, or a documented format to generate LUKS tokens that would be understood by systemd-cryptsetup-generator.

Describe alternatives you've considered

  • Using my existing non-systemd unlock script.
  • Trying to adapt my existing script to manually create a LUKS token that would be understood by systemd-cryptsetup.

The systemd version you checked that didn't have the feature you are asking for

v248

@grawity grawity closed this as completed Apr 4, 2021
@grawity grawity reopened this Apr 4, 2021
@poettering
Copy link
Member

Yeah, this make sense (and is already listed in the TODO file, actually)

@poettering
Copy link
Member

let's drop this from the milestone. I think this makes a ton of sense to have, but I don't think this should really marked for the milestone, I don't think any of the maintainers need this, and hence are unlikely to work on this. Would be delighted to review a patch for this howver, for example if #24597 is resuscitated.

@poettering poettering removed this from the v254 milestone Jun 3, 2023
@flixman
Copy link

flixman commented Jul 9, 2023

@poettering I have just forked systemd and I am trying to implement this functionality in https://github.com/flixman/systemd/pull/1. The idea is to reuse as much code as possible, so the specification of the registers would go like

 --tpm2-pcrs=7+11:sha256=<hash>+14

I have made a number of changes triggered by the update of the signatures in some inner functions, but the idea is that the changes are transparent for methods that do not use them. I have added a couple of tests for the new specification to test-tpm2 and seems to go ok, but I need still to extend the tpm2_unseal and tpm2_make_luks2_json. Do you think you can give it a preliminary look, to assess if it goes in the direction you expect?

@poettering
Copy link
Member

@flixman please submit this as a PR on this repo, and mark it as draft. let's do reviews here, so that they are generally visible to anyone doing reviews here.

generally: please follow coding style. (https://systemd.io/CODING_STYLE/#formatting) We are usually not to keen on reviewing patches that don't even get these superficialities right.

And no fixed-size pre-allocation of data structures please. Also, needs to be somewhat hash func independent, i.e. cover for sha1 and sha256 reasonably. I think it would make sense to store the hash values in a Hashmap object, and make the values a structure that contains pcr nr, hash func, and he hash value and hash size. Then simply pass that around.

@flixman
Copy link

flixman commented Jul 10, 2023

@poettering Good! Then, do I commit to the main on my fork, create the draft PR to yours. See you on the other side!

@akostadinov
Copy link

Is this fixed by #28398 ?

And just for a reference, 2 closed PRs for this same issue #24597 #28339

@Foxboron
Copy link
Contributor

@akostadinov I believe so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cryptsetup RFE 🎁 Request for Enhancement, i.e. a feature request tpm2
Development

Successfully merging a pull request may close this issue.

6 participants