Bug 1213945

Hi all,

a bit more pointers about what packages / components / code we are actually
talking about would be helpful for the review.

Benni answered some of our default questions in the mail thread. I'll add it here:

> - Which package in which codestream should be reviewed?

Affected main-components are
- grub2
- fde-tools
- pcr-oracle

All packages are available in
- openSUSE:Factory
- SUSE:ALP:Source:Standard:1.0

> - Are there part of the source container that don’t need to be
> reviewed?
> - Are there attack scenarios that should not be considered?
>   * E.g. DoS, system runs only in trusted network, desktop/server
> usage.
>   * Is there documentation that makes it clear such scenarios are
> unsupported/insecure?
> - Are there recommendations for security (e.g. configuration) for
> this
>   component? Are they documented somewhere?

For Tumbleweed, documentation can be found at
https://en.opensuse.org/SDB:Encrypted_root_file_system#Unattended_boot_with_TPM_2.0
, for ALP there are already pre-build FDE-images.

> - Apart from the reporter: Who can be contacted and in which form
> should
>   findings be discussed/documented. Our usual/preferred workflow is
> documented at
>
> https://en.opensuse.org/openSUSE:Package_security_guidelines#Audit_Bug_Lifecycle

- Gary Lin
- Olaf Kirch
- Michael Chang

For findings, bootloader-maintainers@suse.de would be the best
assignee.

> - How is your relationship to upstream? Are you upstream yourself?

grub2:
Patches are only partially upstreamed yet.

pcr-oracle, fde-tools:
We (Olaf and Gary/bootloader-maintainers@suse.de) are upstream.

> - Are you willing to add hardenings to the software that are not
> directly
>   related to vulnerabilities? E.g. prevent http usage unless
> specifically
>   configured.

Yes, to some extend with reasonable efforts.

> - What is the reason for asking for this audit? Are there specific
> concerns?

Full Disk Encryption is a security-relevant topic and TPM
authentication via grub2 is not yet well tested by upstream, nor is
there a documented best practice yet.

> - Is there someone that can give us a introduction to the software?

Gary or Olaf are probably the best candidates for an introduction of
the overall picture.

> - Is this audit required for a product release? Is there another
> deadline until
>   you need/want the results?

Ideally during ALP-development to give us some time to react on
feedback.

(In reply to Johannes Segitz from comment #2)
> Benni answered some of our default questions in the mail thread. I'll add it
> here:
> 
> > - Which package in which codestream should be reviewed?
> 
> Affected main-components are
> - grub2
> - fde-tools
> - pcr-oracle
> 
> All packages are available in
> - openSUSE:Factory
> - SUSE:ALP:Source:Standard:1.0
> 

> > - Are there part of the source container that don’t need to be
> > reviewed?
Some unused parts in grub2 such as efiemu probably can be excluded from the review.

> > - Are there attack scenarios that should not be considered?
> >   * E.g. DoS, system runs only in trusted network, desktop/server
> > usage.
> >   * Is there documentation that makes it clear such scenarios are
> > unsupported/insecure?


> > - Are there recommendations for security (e.g. configuration) for
> > this
> >   component? Are they documented somewhere?
> 
> For Tumbleweed, documentation can be found at
> https://en.opensuse.org/SDB:
> Encrypted_root_file_system#Unattended_boot_with_TPM_2.0
> , for ALP there are already pre-build FDE-images.
> 
> > - Apart from the reporter: Who can be contacted and in which form
> > should
> >   findings be discussed/documented. Our usual/preferred workflow is
> > documented at
> >
> > https://en.opensuse.org/openSUSE:Package_security_guidelines#Audit_Bug_Lifecycle
> 
> - Gary Lin
> - Olaf Kirch
> - Michael Chang
> 
> For findings, bootloader-maintainers@suse.de would be the best
> assignee.
> 
> > - How is your relationship to upstream? Are you upstream yourself?
> 
> grub2:
> Patches are only partially upstreamed yet.
> 
> pcr-oracle, fde-tools:
> We (Olaf and Gary/bootloader-maintainers@suse.de) are upstream.
> 
> > - Are you willing to add hardenings to the software that are not
> > directly
> >   related to vulnerabilities? E.g. prevent http usage unless
> > specifically
> >   configured.
> 
> Yes, to some extend with reasonable efforts.
> 
> > - What is the reason for asking for this audit? Are there specific
> > concerns?
> 
> Full Disk Encryption is a security-relevant topic and TPM
> authentication via grub2 is not yet well tested by upstream, nor is
> there a documented best practice yet.
> 
> > - Is there someone that can give us a introduction to the software?
> 
> Gary or Olaf are probably the best candidates for an introduction of
> the overall picture.
> 
> > - Is this audit required for a product release? Is there another
> > deadline until
> >   you need/want the results?
> 
> Ideally during ALP-development to give us some time to react on
> feedback.

I already reviewed the fde-tools package not too long ago, because of the
systemd service it contains. Although I'm not too thrilled about elaborate
bash scripting in this area, the scripting looked mostly sane.

There was only a strange bit about a cleartext password being stored in
/etc/default/grub.cfg in some context. I will investigate this once more.

Which patches in grub2 are the relevant ones in this context?
I believe I already talked to someone about these patches but can't remember
with whom or over which channel any more. I definitely reviewed some grub /
tpm related patches.

The pcr-oracle I didn't hear about yet. I will look into it.

(In reply to matthias.gerstner@suse.com from comment #4)
> Which patches in grub2 are the relevant ones in this context?
> I believe I already talked to someone about these patches but can't remember
> with whom or over which channel any more. I definitely reviewed some grub /
> tpm related patches.

Found it. It was in bug 1197427. Do these patches happen to be the same as you
now use for the FDE feature?

(In reply to Matthias Gerstner from comment #4)
> I already reviewed the fde-tools package not too long ago, because of the
> systemd service it contains. Although I'm not too thrilled about elaborate
> bash scripting in this area, the scripting looked mostly sane.
> 
> There was only a strange bit about a cleartext password being stored in
> /etc/default/grub.cfg in some context. I will investigate this once more.
> 
> Which patches in grub2 are the relevant ones in this context?
> I believe I already talked to someone about these patches but can't remember
> with whom or over which channel any more. I definitely reviewed some grub /
> tpm related patches.
> 
The cleartext password is for the firstboot since fdectl/pcr-oracle cannot predict the PCR values in the installation system. To avoid the password typing during the firstboot, we set a temporary password and then remove the password when the fde-tpm-enroll service is up.

> The pcr-oracle I didn't hear about yet. I will look into it.
It's the key program to predict the PCR values and seal the disk key.

(In reply to Matthias Gerstner from comment #5)
> (In reply to matthias.gerstner@suse.com from comment #4)
> > Which patches in grub2 are the relevant ones in this context?
> > I believe I already talked to someone about these patches but can't remember
> > with whom or over which channel any more. I definitely reviewed some grub /
> > tpm related patches.
> 
> Found it. It was in bug 1197427. Do these patches happen to be the same as
> you
> now use for the FDE feature?

That's the base patches. We have improved the patch set and add a few more features. I recently sent the updated patch set to upstream:

https://lists.gnu.org/archive/html/grub-devel/2023-08/msg00113.html

(In reply to glin@suse.com from comment #7)
> (In reply to Matthias Gerstner from comment #5)
> > (In reply to matthias.gerstner@suse.com from comment #4)
> > > Which patches in grub2 are the relevant ones in this context?
> > > I believe I already talked to someone about these patches but can't remember
> > > with whom or over which channel any more. I definitely reviewed some grub /
> > > tpm related patches.
> > 
> > Found it. It was in bug 1197427. Do these patches happen to be the same as
> > you
> > now use for the FDE feature?
> 
> That's the base patches. We have improved the patch set and add a few more features. I recently sent the updated patch set to upstream:
> 
> https://lists.gnu.org/archive/html/grub-devel/2023-08/msg00113.html

Thanks for the hints. Am I correct that the changes since then are accessible
in the Git repository https://github.com/lcp/grub2.git by looking at `git show
tpm2-unlock-v2..tpm2-unlock-v5` ?

(In reply to Matthias Gerstner from comment #8)
> (In reply to glin@suse.com from comment #7)
> > (In reply to Matthias Gerstner from comment #5)
> > > (In reply to matthias.gerstner@suse.com from comment #4)
> > > > Which patches in grub2 are the relevant ones in this context?
> > > > I believe I already talked to someone about these patches but can't remember
> > > > with whom or over which channel any more. I definitely reviewed some grub /
> > > > tpm related patches.
> > > 
> > > Found it. It was in bug 1197427. Do these patches happen to be the same as
> > > you
> > > now use for the FDE feature?
> > 
> > That's the base patches. We have improved the patch set and add a few more features. I recently sent the updated patch set to upstream:
> > 
> > https://lists.gnu.org/archive/html/grub-devel/2023-08/msg00113.html
> 
> Thanks for the hints. Am I correct that the changes since then are accessible
> in the Git repository https://github.com/lcp/grub2.git by looking at `git
> show
> tpm2-unlock-v2..tpm2-unlock-v5` ?

'tpm2-unlock' is my v1 branch containing the unmodified TPM patches from Hernan Gatta plus my follow-up patches. The upstream maintainer then requested to squash the follow-up patches into the original patches, and that is 'tpm2-unlock-v2'. So the starting point for comparison is

https://github.com/lcp/grub2/commit/60f927df1792a3b240d58b22f7e9120bbd1a2df7

60f927df1792a3b240d58b22f7e9120bbd1a2df7
util/grub-protect: Add new tool

I formally start the review now. It will take some time though due to the
complexity of the matter.

I finished going through all components. Following is a summary and some
comments, also a few action items.

General Impression
==================

Each individual component makes sense so far and code quality is generally
good. As usual when cryptography and especially the TPM is involved then
complexity rises quickly.

This use case with unattended LUKS dencryption based on the TPM feels brittle
overall, due to the complexities involving the prediction of the TPM PCR
register values and frequent updates of the sealed key file are to be
expected. One might wonder whether this is can still be considered good
practice in disk encryption, but it is by design in this case, and cannot be
changed.

Chances are rather high that something will break in the toolchain and/or
prediction logic over time and the unattended boot fails. Since there is still
an interactive LUKS passphrase available this can be recovered from but it
will be an unlucky situation since this is especially targeted toward
unattended servers.

From an information security point of view using this feature on servers is
certainly an improvement over not using disk encryption at all. One should be
careful though, that the use of this doesn't spill over to other setups
like desktop machines as well, for reasons of perceived easiness, for example
(not ever having to type in a password). I consider traditional passphrase
based LUKS encryption still superior to the unattended TPM based approach both
in reliability and in security aspects. This could also be stressed in the
documentation to prevent users from going this route when not appropriate.

On the technical level I am a bit surprised by the choice of programming
language for newly written code: bash for fde-tools and plain C for
pcr-oracle. The code is clean but shows the typical difficulties that these
languages tend to have. The bash code is not all that easy to follow due to
the sourcing of scripts, global state etc. The C code has to deal with
quite some things that are not part of the core business logic like file
handling, memory management and string processing. A choice like Python/C++ or
Python/Golang would have felt more natural these days for such a task.

Regarding the TPM specifics there is quite some code duplication between grub,
pcr-oracle and the tpm2-tss library. For grub this is somewhat understandable
given the minimal runtime environment that is available there. Still there
might be ways to share some data structures and routines between grub2 and
tpm2-tss, as I already mentioned in bug 1197427. The pcr-oracle codebase
already uses tpm2-tss for accessing the TPM but there are still some things
that feel duplicated like parsing of PCR register selection strings. Logic
like this certainly already exists somewhere in tpm2-tools, but I don't know
whether these are publicly accessible functions.

grub
====

The key unsealing specific functionality is embedded well into existing
framework-like infrastructure in grub. A new configuration command
'tpm2_key_protector_init' allows to specify a file to unseal using the TPM.
The already existing `cryptomount` command can use the tpm module for
accessing the cleartext passphrase. Grub generates additional entries in the
initrd on-the-fly in /etc/cryptsetup-keys.d/<uuid>.key containing the
cleartext keys. This is infrastructure in Grub that is independent of the TPM
unsealing logic.

systemd during early boot in the initrd context picks up these forwarded
credentials (`cryptsetup.c`) to use the for the further boot process. The
initrd is supposed to be cleared completely when boot is finished, at least in
the default setup we find with systemd on x86.

fde-tools
=========

This is mostly shell glue that configures Grub, operates on LUKS devices and
uses the additional utilities provided by pcr-oracle to setup the unattended
TPM disk unlocking.

Apart from the temporary plaintext password that is stored in grub.cfg in the
installation context (see comment 6) I don't see much that is worth attention
here.

pcr-oracle
==========

The main program shipped here is fdectl. It operates on the TPM using the
Intel tpm2 stack. It also performs various cryptographic operations that are
necessary to setup the sealed keyfile and correctly setup the TPM for
unsealing it. The prediction of PCR register values based on the tpm eventlog
is also included here. This component is the most complex part of the whole
feature.

Here are a few specific pointers to things that can be improved in the code:

- main(), oracle.c:993: this bunch of `opt_<option>` variables seems a bit too
  much to declare them one by one. This could use a proper option struct,
  which might generally improve readability.
- write_public_key(), pcr-policy.c:276: where are the 128 extra bytes for the
  buffer allocation coming from?
- buffer_free_secret(), bufparser.h:213: using regular memset() won't help
  much. Since C11 there's `memset_s()` available for this use case.

cryptography
============

- On LUKS level only PBKDF2 is currently supported. We currently don't even
  use better algorithms (like argon2id) on regular LUKS setups. Still we
  should be working on getting better defaults here, especially when new
  features are implemented like in this case.
- pcr-oracle: for the auth policy the RSA key size is currently restricted to
  2048 bits without a passphrase (oracle.c:1195). This is a bit on the weak
  end nowadays and going to 4096 bits is a good idea, ideally making
  it configurable. Optionally supporting a passphrase to regenerate the sealed
  keyfile would be a hardening improvement.
- The sealed keyfile is stored on the EFI partition, accessible by everyone in
  the system during regular system operation (/boot/EFI is mounted by
  default).

  In the tpm2 software stack, especially when the abrmd daemon is around,
  every local user may by default talk to the TPM [1]. I changed this default
  configuration for openSUSE, but there is still some danger that this will be
  possible due to different admin needs. In such a case any local user might
  be able to unseal the keyfile found on the EFI partition and thus get access
  to the cleartext.

  This should be improved upon, although I'm not sure how, because the EFI
  partition is FAT, and doesn't support access settings.
  
  [1]: https://www.openwall.com/lists/oss-security/2022/04/20/3

(In reply to Matthias Gerstner from comment #11)
> I finished going through all components. Following is a summary and some
> comments, also a few action items.
> 
> General Impression
> ==================
> 
> Each individual component makes sense so far and code quality is generally
> good. As usual when cryptography and especially the TPM is involved then
> complexity rises quickly.
> 
> This use case with unattended LUKS dencryption based on the TPM feels brittle
> overall, due to the complexities involving the prediction of the TPM PCR
> register values and frequent updates of the sealed key file are to be
> expected. One might wonder whether this is can still be considered good
> practice in disk encryption, but it is by design in this case, and cannot be
> changed.
>
To mitigate PCR brittleness, we minimize the measured programs for the sealed key to the firmware, shim, and grub. It's not perfect but at least lowers the chances to update the sealed key.

> Chances are rather high that something will break in the toolchain and/or
> prediction logic over time and the unattended boot fails. Since there is
> still
> an interactive LUKS passphrase available this can be recovered from but it
> will be an unlucky situation since this is especially targeted toward
> unattended servers.
> 
> From an information security point of view using this feature on servers is
> certainly an improvement over not using disk encryption at all. One should be
> careful though, that the use of this doesn't spill over to other setups
> like desktop machines as well, for reasons of perceived easiness, for example
> (not ever having to type in a password). I consider traditional passphrase
> based LUKS encryption still superior to the unattended TPM based approach
> both
> in reliability and in security aspects. This could also be stressed in the
> documentation to prevent users from going this route when not appropriate.
> 
Sure, and the user should disable auto-login when enabling TPM auto-unlocking...

> On the technical level I am a bit surprised by the choice of programming
> language for newly written code: bash for fde-tools and plain C for
> pcr-oracle. The code is clean but shows the typical difficulties that these
> languages tend to have. The bash code is not all that easy to follow due to
> the sourcing of scripts, global state etc. The C code has to deal with
> quite some things that are not part of the core business logic like file
> handling, memory management and string processing. A choice like Python/C++
> or
> Python/Golang would have felt more natural these days for such a task.
> 
> Regarding the TPM specifics there is quite some code duplication between
> grub,
> pcr-oracle and the tpm2-tss library. For grub this is somewhat understandable
> given the minimal runtime environment that is available there. Still there
> might be ways to share some data structures and routines between grub2 and
> tpm2-tss, as I already mentioned in bug 1197427.

The definitions of TPM2 structs in grub2 are mostly copied from tpm2-tss. I have to admit that I didn't think about reusing the routines from tpm2-tss since the original patch directly maps the TPM2 commands to functions, and it's intuitive enough for me to use them.

> The pcr-oracle codebase
> already uses tpm2-tss for accessing the TPM but there are still some things
> that feel duplicated like parsing of PCR register selection strings. Logic
> like this certainly already exists somewhere in tpm2-tools, but I don't know
> whether these are publicly accessible functions.
> 
I'd leave this to Olaf.

> grub
> ====
> 
> The key unsealing specific functionality is embedded well into existing
> framework-like infrastructure in grub. A new configuration command
> 'tpm2_key_protector_init' allows to specify a file to unseal using the TPM.
> The already existing `cryptomount` command can use the tpm module for
> accessing the cleartext passphrase. Grub generates additional entries in the
> initrd on-the-fly in /etc/cryptsetup-keys.d/<uuid>.key containing the
> cleartext keys. This is infrastructure in Grub that is independent of the TPM
> unsealing logic.
> 
> systemd during early boot in the initrd context picks up these forwarded
> credentials (`cryptsetup.c`) to use the for the further boot process. The
> initrd is supposed to be cleared completely when boot is finished, at least
> in
> the default setup we find with systemd on x86.
> 
> fde-tools
> =========
> 
> This is mostly shell glue that configures Grub, operates on LUKS devices and
> uses the additional utilities provided by pcr-oracle to setup the unattended
> TPM disk unlocking.
> 
> Apart from the temporary plaintext password that is stored in grub.cfg in the
> installation context (see comment 6) I don't see much that is worth attention
> here.
> 
> pcr-oracle
> ==========
> 
> The main program shipped here is fdectl. It operates on the TPM using the
> Intel tpm2 stack. It also performs various cryptographic operations that are
> necessary to setup the sealed keyfile and correctly setup the TPM for
> unsealing it. The prediction of PCR register values based on the tpm eventlog
> is also included here. This component is the most complex part of the whole
> feature.
> 
> Here are a few specific pointers to things that can be improved in the code:
> 
> - main(), oracle.c:993: this bunch of `opt_<option>` variables seems a bit
> too
>   much to declare them one by one. This could use a proper option struct,
>   which might generally improve readability.
> - write_public_key(), pcr-policy.c:276: where are the 128 extra bytes for the
>   buffer allocation coming from?
> - buffer_free_secret(), bufparser.h:213: using regular memset() won't help
>   much. Since C11 there's `memset_s()` available for this use case.
> 
> cryptography
> ============
> 
> - On LUKS level only PBKDF2 is currently supported. We currently don't even
>   use better algorithms (like argon2id) on regular LUKS setups. Still we
>   should be working on getting better defaults here, especially when new
>   features are implemented like in this case.
Michael is still working on the Argon2 support in grub2, and we hope to make it work soon.

> - pcr-oracle: for the auth policy the RSA key size is currently restricted to
>   2048 bits without a passphrase (oracle.c:1195). This is a bit on the weak
>   end nowadays and going to 4096 bits is a good idea, ideally making
>   it configurable. 

There may be some TPM 2.0 modules only supporting up to RSA 2048. For example, Dell only lists RSA 2048 in their document:

https://www.dell.com/support/kbdoc/en-us/000131631/tpm-1-2-vs-2-0-features?lwp=rt

So we have to test the capacity of the TPM module in ether fde-tools or pcr-oracle. Grub2 also has to add a new parameter for the RSA key size.

>   Optionally supporting a passphrase to regenerate the sealed
>   keyfile would be a hardening improvement.
>
> - The sealed keyfile is stored on the EFI partition, accessible by everyone
> in
>   the system during regular system operation (/boot/EFI is mounted by
>   default).
> 
>   In the tpm2 software stack, especially when the abrmd daemon is around,
>   every local user may by default talk to the TPM [1]. I changed this default
>   configuration for openSUSE, but there is still some danger that this will
> be
>   possible due to different admin needs. In such a case any local user might
>   be able to unseal the keyfile found on the EFI partition and thus get
> access
>   to the cleartext.
> 
>   This should be improved upon, although I'm not sure how, because the EFI
>   partition is FAT, and doesn't support access settings.
>   
>   [1]: https://www.openwall.com/lists/oss-security/2022/04/20/3

The key is sealed against the PCR values (0,2,4,7,9) when the 1st grub.cfg (in the EFI partition) is loaded. After grub2 loads the 2nd grub.cfg in /boot, PCR 9 will be extended, so the key cannot be unsealed afterward.

(In reply to glin@suse.com from comment #12)
> The key is sealed against the PCR values (0,2,4,7,9) when the 1st grub.cfg (in the EFI partition) is loaded. After grub2 loads the 2nd grub.cfg in /boot, PCR 9 will be extended, so the key cannot be unsealed afterward.

Ah, this is good, thanks for clarifying.

(In reply to Gary Ching-Pang Lin from comment #12)
> (In reply to Matthias Gerstner from comment #11)
> > I finished going through all components. Following is a summary and some
> > comments, also a few action items.
> > 
> > General Impression
> > ==================
> > 
> > Each individual component makes sense so far and code quality is generally
> > good. As usual when cryptography and especially the TPM is involved then
> > complexity rises quickly.
> > 
> > This use case with unattended LUKS dencryption based on the TPM feels brittle
> > overall, due to the complexities involving the prediction of the TPM PCR
> > register values and frequent updates of the sealed key file are to be
> > expected. One might wonder whether this is can still be considered good
> > practice in disk encryption, but it is by design in this case, and cannot be
> > changed.
> >
> To mitigate PCR brittleness, we minimize the measured programs for the
> sealed key to the firmware, shim, and grub. It's not perfect but at least
> lowers the chances to update the sealed key.
> 
> > Chances are rather high that something will break in the toolchain and/or
> > prediction logic over time and the unattended boot fails. Since there is
> > still
> > an interactive LUKS passphrase available this can be recovered from but it
> > will be an unlucky situation since this is especially targeted toward
> > unattended servers.
> > 
> > From an information security point of view using this feature on servers is
> > certainly an improvement over not using disk encryption at all. One should be
> > careful though, that the use of this doesn't spill over to other setups
> > like desktop machines as well, for reasons of perceived easiness, for example
> > (not ever having to type in a password). I consider traditional passphrase
> > based LUKS encryption still superior to the unattended TPM based approach
> > both
> > in reliability and in security aspects. This could also be stressed in the
> > documentation to prevent users from going this route when not appropriate.
> > 
> Sure, and the user should disable auto-login when enabling TPM
> auto-unlocking...
> 
> > On the technical level I am a bit surprised by the choice of programming
> > language for newly written code: bash for fde-tools and plain C for
> > pcr-oracle. The code is clean but shows the typical difficulties that these
> > languages tend to have. The bash code is not all that easy to follow due to
> > the sourcing of scripts, global state etc. The C code has to deal with
> > quite some things that are not part of the core business logic like file
> > handling, memory management and string processing. A choice like Python/C++
> > or
> > Python/Golang would have felt more natural these days for such a task.
> > 
> > Regarding the TPM specifics there is quite some code duplication between
> > grub,
> > pcr-oracle and the tpm2-tss library. For grub this is somewhat understandable
> > given the minimal runtime environment that is available there. Still there
> > might be ways to share some data structures and routines between grub2 and
> > tpm2-tss, as I already mentioned in bug 1197427.
> 
> The definitions of TPM2 structs in grub2 are mostly copied from tpm2-tss. I
> have to admit that I didn't think about reusing the routines from tpm2-tss
> since the original patch directly maps the TPM2 commands to functions, and
> it's intuitive enough for me to use them.
> 
> > The pcr-oracle codebase
> > already uses tpm2-tss for accessing the TPM but there are still some things
> > that feel duplicated like parsing of PCR register selection strings. Logic
> > like this certainly already exists somewhere in tpm2-tools, but I don't know
> > whether these are publicly accessible functions.
> > 
> I'd leave this to Olaf.
> 
> > grub
> > ====
> > 
> > The key unsealing specific functionality is embedded well into existing
> > framework-like infrastructure in grub. A new configuration command
> > 'tpm2_key_protector_init' allows to specify a file to unseal using the TPM.
> > The already existing `cryptomount` command can use the tpm module for
> > accessing the cleartext passphrase. Grub generates additional entries in the
> > initrd on-the-fly in /etc/cryptsetup-keys.d/<uuid>.key containing the
> > cleartext keys. This is infrastructure in Grub that is independent of the TPM
> > unsealing logic.
> > 
> > systemd during early boot in the initrd context picks up these forwarded
> > credentials (`cryptsetup.c`) to use the for the further boot process. The
> > initrd is supposed to be cleared completely when boot is finished, at least
> > in
> > the default setup we find with systemd on x86.
> > 
> > fde-tools
> > =========
> > 
> > This is mostly shell glue that configures Grub, operates on LUKS devices and
> > uses the additional utilities provided by pcr-oracle to setup the unattended
> > TPM disk unlocking.
> > 
> > Apart from the temporary plaintext password that is stored in grub.cfg in the
> > installation context (see comment 6) I don't see much that is worth attention
> > here.
> > 
Forgot to mention that I'm working on the RPM macros/helper to update the sealed key after the bootloader updates.

https://github.com/lcp/fde-tools/tree/tpm-auto-update

Does this raise any security concern?

> > pcr-oracle
> > ==========
> > 
> > The main program shipped here is fdectl. It operates on the TPM using the
> > Intel tpm2 stack. It also performs various cryptographic operations that are
> > necessary to setup the sealed keyfile and correctly setup the TPM for
> > unsealing it. The prediction of PCR register values based on the tpm eventlog
> > is also included here. This component is the most complex part of the whole
> > feature.
> > 
> > Here are a few specific pointers to things that can be improved in the code:
> > 
> > - main(), oracle.c:993: this bunch of `opt_<option>` variables seems a bit
> > too
> >   much to declare them one by one. This could use a proper option struct,
> >   which might generally improve readability.
> > - write_public_key(), pcr-policy.c:276: where are the 128 extra bytes for the
> >   buffer allocation coming from?
> > - buffer_free_secret(), bufparser.h:213: using regular memset() won't help
> >   much. Since C11 there's `memset_s()` available for this use case.
> > 
> > cryptography
> > ============
> > 
> > - On LUKS level only PBKDF2 is currently supported. We currently don't even
> >   use better algorithms (like argon2id) on regular LUKS setups. Still we
> >   should be working on getting better defaults here, especially when new
> >   features are implemented like in this case.
> Michael is still working on the Argon2 support in grub2, and we hope to make
> it work soon.
> 
> > - pcr-oracle: for the auth policy the RSA key size is currently restricted to
> >   2048 bits without a passphrase (oracle.c:1195). This is a bit on the weak
> >   end nowadays and going to 4096 bits is a good idea, ideally making
> >   it configurable. 
> 
> There may be some TPM 2.0 modules only supporting up to RSA 2048. For
> example, Dell only lists RSA 2048 in their document:
> 
> https://www.dell.com/support/kbdoc/en-us/000131631/tpm-1-2-vs-2-0-
> features?lwp=rt
> 

It seems that RSA4096 is not widely supported among the TPM 2.0 chips :-(
The following command failed on my laptop (shipped in 2022).

$ tpm2_createprimary -C o -g sha256 -G rsa4096 -c context.out
WARNING:esys:src/tss2-esys/api/Esys_CreatePrimary.c:400:Esys_CreatePrimary_Finish() Received TPM Error 
ERROR:esys:src/tss2-esys/api/Esys_CreatePrimary.c:135:Esys_CreatePrimary() Esys Finish ErrorCode (0x000002c4) 
ERROR: Esys_CreatePrimary(0x2C4) - tpm:parameter(2):value is out of range or is not correct for the context
ERROR: Unable to run tpm2_createprimary

> So we have to test the capacity of the TPM module in ether fde-tools or
> pcr-oracle. Grub2 also has to add a new parameter for the RSA key size.
> 
> >   Optionally supporting a passphrase to regenerate the sealed
> >   keyfile would be a hardening improvement.
> >
> > - The sealed keyfile is stored on the EFI partition, accessible by everyone
> > in
> >   the system during regular system operation (/boot/EFI is mounted by
> >   default).
> > 
> >   In the tpm2 software stack, especially when the abrmd daemon is around,
> >   every local user may by default talk to the TPM [1]. I changed this default
> >   configuration for openSUSE, but there is still some danger that this will
> > be
> >   possible due to different admin needs. In such a case any local user might
> >   be able to unseal the keyfile found on the EFI partition and thus get
> > access
> >   to the cleartext.
> > 
> >   This should be improved upon, although I'm not sure how, because the EFI
> >   partition is FAT, and doesn't support access settings.
> >   
> >   [1]: https://www.openwall.com/lists/oss-security/2022/04/20/3
> 
> The key is sealed against the PCR values (0,2,4,7,9) when the 1st grub.cfg
> (in the EFI partition) is loaded. After grub2 loads the 2nd grub.cfg in
> /boot, PCR 9 will be extended, so the key cannot be unsealed afterward.

(In reply to Gary Ching-Pang Lin from comment #14)
> (In reply to Gary Ching-Pang Lin from comment #12)
> > (In reply to Matthias Gerstner from comment #11)
> > > - pcr-oracle: for the auth policy the RSA key size is currently restricted to
> > >   2048 bits without a passphrase (oracle.c:1195). This is a bit on the weak
> > >   end nowadays and going to 4096 bits is a good idea, ideally making
> > >   it configurable. 
> > 
> > There may be some TPM 2.0 modules only supporting up to RSA 2048. For
> > example, Dell only lists RSA 2048 in their document:
> > 
> > https://www.dell.com/support/kbdoc/en-us/000131631/tpm-1-2-vs-2-0-
> > features?lwp=rt
> > 
> 
> It seems that RSA4096 is not widely supported among the TPM 2.0 chips :-(
> The following command failed on my laptop (shipped in 2022).
> 
> $ tpm2_createprimary -C o -g sha256 -G rsa4096 -c context.out
> WARNING:esys:src/tss2-esys/api/Esys_CreatePrimary.c:400:
> Esys_CreatePrimary_Finish() Received TPM Error 
> ERROR:esys:src/tss2-esys/api/Esys_CreatePrimary.c:135:Esys_CreatePrimary()
> Esys Finish ErrorCode (0x000002c4) 
> ERROR: Esys_CreatePrimary(0x2C4) - tpm:parameter(2):value is out of range or
> is not correct for the context
> ERROR: Unable to run tpm2_createprimary
> 
Or, should we switch to ECC? My laptop supports the following 3 ECC curves:

$ tpm2_getcap ecc-curves
TPM2_ECC_NIST_P256: 0x3
TPM2_ECC_NIST_P384: 0x4
TPM2_ECC_BN_P256: 0x10

It seems that some people consider those curves unsafe though.

The RPM aspects (automatically updating the sealed key) should be fine.

If possible I would try RSA4096 first, and fallback to RSA2048 if that doesn't
work out. The ECC curves ... opinions differ but I wouldn't trust the NIST
curves very much. Also there will likely be similar problems that TPM chips
don't widely support ECC.

On my older laptop the setup with fdectl fails, by the way, because for some
reason the TPM event log is in the wrong format. As usual when TPMs are around
there's a lot of pitfalls ...

(In reply to Matthias Gerstner from comment #16)
> The RPM aspects (automatically updating the sealed key) should be fine.
> 
Thanks, then I'll keep working on the auto-update feature.

> If possible I would try RSA4096 first, and fallback to RSA2048 if that
> doesn't
> work out. The ECC curves ... opinions differ but I wouldn't trust the NIST
> curves very much. Also there will likely be similar problems that TPM chips
> don't widely support ECC.
> 
Alright, let's stick to RSA.

> On my older laptop the setup with fdectl fails, by the way, because for some
> reason the TPM event log is in the wrong format. As usual when TPMs are
> around there's a lot of pitfalls ...
True. And this leads to all kinds of quirks and workarounds...

[Update for RSA key size support]

In the latest "Platform TPM Profile" spec(*):

4.1 PC Client TPM Minimums and Maximums
...
Mandatory support for RSA 3072-bit keys, added in PTP version 1.05, ...
...

I also checked the latest libtpms, the backend of swtpm, and found that it only enabled RSA up to 3072(*2).

RSA 4096 probably won't be available for a while, but some systems may provide RSA 3072, so our choice for RSA key size would be: 4096 -> 3072 -> 2048.

(*1) https://trustedcomputinggroup.org/wp-content/uploads/PC-Client-Specific-Platform-TPM-Profile-for-TPM-2p0-v1p05p_r14_pub.pdf
(*2) https://github.com/stefanberger/libtpms/blob/master/src/tpm2/TpmProfile.h#L450-L465

Sounds good with the 3072 key size consideration.

Do you need anything else from us or can we close this audit bug? You are
welcome to raise any future questions by mail, Slack etc.

I have no question for now. Feel free to close this bug.
Thanks for the reviewing and auditing :)

closing as fixed

Hi Matthias,

Reopen this bug for the question about the key algorithm selection.

When upstreaming the grub2 patches, I found that the current implementation conflicts with the TPM 2.0 Key File spec(*). The spec expects the parent key (TPM SRK) uses ECC NIST-P256 in the key generation template, while we always use RSA.

After discussing with the spec author, James Bottomley, although he agrees to add a new field to mark the RSA parent, he strongly discourages the use of RSA as the parent key. He points out that TPM 2.0 SRK comprises of a asymmetric key and a symmetric key (We choose AES128 in our key template(*2)), and only the symmetric key is used to seal the object, so it's not necessary to waste time on RSA key generation which is much slower than ECC. I'm considering to change the default SRK template to ECC for the newer grub2/pcr-oracle. Is there any concern from security team?

(For backward compatibility, I plan to make grub2 fall back to the RSA SRK if it encounters "integrity check failed" error when using ECC SRK on TPM2_Load.)

As for the authorized policy signing, I'd keep using RSA for the time being.  James provides some insight about the rarity of RSA 3072 support in TPM 2.0 chips:

"I think TPM manufacturers are ambivalent about RSA 3k primarily because                                                                                                                        
although it gets to the 128 bit requirement, the next step is the post                                                                                                                         
quantum 256 bit one and that means RSA is effectively dead, so they                                                                                                                            
don't want to perturb everything for a legacy algorithm.                                                                                                                                       
                                                                                                                                                                                               
The upshot is that RSA 3k keys are never going to be a reliable TPM 2.0                                                                                                                        
feature but RSA 2k is."

However, the only mandatory ECC curves are NIST-P256 and NIST-P384, so we don't have a good choice other than RSA 2048.

(*1) https://www.hansenpartnership.com/draft-bottomley-tpm2-keys.html#name-parent
(*2) https://github.com/okirch/pcr-oracle/blob/0.5.4/src/pcr-policy.c#L65-L88

Hi Gary,

I would say what we have here is a conflict between spec compatibility, slow
hardware evolvement and the need for stronger algorithms.

I'm not sure exactly where the "slow RSA key generation" hits the process, I
wouldn't expect it to cause major havok in speed though. Not using RSA 3k keys
sounds like a sensible plan then. Broken cryptography is likely even worse
than weak cryptography.

I'm not the one to reject NIST ECC curves for such a scenario. I just know
that some some actors that are deeper into cryptography than myself don't
trust them fully. If these ECC algorithms solve a major problem for fde-tools,
and there is no alternative, then I would still use them.

(In reply to Matthias Gerstner from comment #28)
> Hi Gary,
> 
> I would say what we have here is a conflict between spec compatibility, slow
> hardware evolvement and the need for stronger algorithms.
> 
> I'm not sure exactly where the "slow RSA key generation" hits the process, I
> wouldn't expect it to cause major havok in speed though. Not using RSA 3k
> keys
> sounds like a sensible plan then. Broken cryptography is likely even worse
> than weak cryptography.
> 
We currently generate the TPM SRK at runtime, so the slow key generation may introduce delay to the boot time. I heard that it may take more than 20 seconds to generate a RSA key pair with some old TPM chips, so the delay could be noticeable.

> I'm not the one to reject NIST ECC curves for such a scenario. I just know
> that some some actors that are deeper into cryptography than myself don't
> trust them fully. If these ECC algorithms solve a major problem for
> fde-tools,
> and there is no alternative, then I would still use them.
Alright. I'll switch the default SRK algorithm to ECC for the grub2 upstream patches and gradually add ECC signing support to pcr-oracle and fde-tools as the RSA alternative.

(In reply to glin@suse.com from comment #29)

> We currently generate the TPM SRK at runtime, so the slow key generation may introduce delay to the boot time. I heard that it may take more than 20 seconds to generate a RSA key pair with some old TPM chips, so the delay could be noticeable.

But the key generation only happens when something has changed, right? So this
is not something that affects every boot IIUC.

> Alright. I'll switch the default SRK algorithm to ECC for the grub2 upstream patches and gradually add ECC signing support to pcr-oracle and fde-tools as the RSA alternative.

Sounds good. Thanks!

(In reply to Matthias Gerstner from comment #30)
> (In reply to glin@suse.com from comment #29)
> 
> > We currently generate the TPM SRK at runtime, so the slow key generation may introduce delay to the boot time. I heard that it may take more than 20 seconds to generate a RSA key pair with some old TPM chips, so the delay could be noticeable.
> 
> But the key generation only happens when something has changed, right? So
> this
> is not something that affects every boot IIUC.
> 
We don't store the SRK in the persistent handle but provide the consistent SRK template which guarantees to generate the same key. The benefit is to leave the limited persistent handles to the user, while the drawback is the additional overhead of SRK generation.

> > Alright. I'll switch the default SRK algorithm to ECC for the grub2 upstream patches and gradually add ECC signing support to pcr-oracle and fde-tools as the RSA alternative.
> 
> Sounds good. Thanks!

I see. Well with this design choice using ECC encryption surely is the better
option then.

I guess the additional topics have been discussed. Closing again.

SUSE-SU-2024:1368-1: An update that solves seven vulnerabilities, contains one feature and has five security fixes can now be installed.

Category: security (important)
Bug References: 1198101, 1205588, 1205855, 1210382, 1213945, 1215098, 1215099, 1215100, 1215101, 1215102, 1215103, 1219460
CVE References: CVE-2022-28737, CVE-2023-40546, CVE-2023-40547, CVE-2023-40548, CVE-2023-40549, CVE-2023-40550, CVE-2023-40551
Jira References: PED-922
Maintenance Incident: [SUSE:Maintenance:32617](https://smelt.suse.de/incident/32617/)
Sources used:
openSUSE Leap 15.3 (src):
 shim-15.8-150300.4.20.2, efitools-1.9.2-150300.7.3.1
openSUSE Leap Micro 5.3 (src):
 shim-15.8-150300.4.20.2
openSUSE Leap Micro 5.4 (src):
 shim-15.8-150300.4.20.2
openSUSE Leap 15.5 (src):
 shim-15.8-150300.4.20.2
SUSE Linux Enterprise Micro for Rancher 5.3 (src):
 shim-15.8-150300.4.20.2
SUSE Linux Enterprise Micro 5.3 (src):
 shim-15.8-150300.4.20.2
SUSE Linux Enterprise Micro for Rancher 5.4 (src):
 shim-15.8-150300.4.20.2
SUSE Linux Enterprise Micro 5.4 (src):
 shim-15.8-150300.4.20.2
SUSE Linux Enterprise Micro 5.5 (src):
 shim-15.8-150300.4.20.2
Basesystem Module 15-SP5 (src):
 shim-15.8-150300.4.20.2
SUSE Linux Enterprise High Performance Computing LTSS 15 SP3 (src):
 shim-15.8-150300.4.20.2
SUSE Linux Enterprise High Performance Computing ESPOS 15 SP4 (src):
 shim-15.8-150300.4.20.2
SUSE Linux Enterprise High Performance Computing LTSS 15 SP4 (src):
 shim-15.8-150300.4.20.2
SUSE Linux Enterprise Desktop 15 SP4 LTSS 15-SP4 (src):
 shim-15.8-150300.4.20.2
SUSE Linux Enterprise Server 15 SP3 LTSS 15-SP3 (src):
 shim-15.8-150300.4.20.2
SUSE Linux Enterprise Server 15 SP4 LTSS 15-SP4 (src):
 shim-15.8-150300.4.20.2
SUSE Linux Enterprise Server for SAP Applications 15 SP3 (src):
 shim-15.8-150300.4.20.2
SUSE Linux Enterprise Server for SAP Applications 15 SP4 (src):
 shim-15.8-150300.4.20.2
SUSE Manager Proxy 4.3 (src):
 shim-15.8-150300.4.20.2
SUSE Manager Retail Branch Server 4.3 (src):
 shim-15.8-150300.4.20.2
SUSE Manager Server 4.3 (src):
 shim-15.8-150300.4.20.2
SUSE Enterprise Storage 7.1 (src):
 shim-15.8-150300.4.20.2
SUSE Linux Enterprise Micro 5.1 (src):
 shim-15.8-150300.4.20.2
SUSE Linux Enterprise Micro 5.2 (src):
 shim-15.8-150300.4.20.2
SUSE Linux Enterprise Micro for Rancher 5.2 (src):
 shim-15.8-150300.4.20.2

NOTE: This line indicates an update has been released for the listed product(s). At times this might be only a partial fix. If you have questions please reach out to maintenance coordination.

SUSE-SU-2024:1462-1: An update that solves seven vulnerabilities, contains one feature and has five security fixes can now be installed.

Category: security (important)
Bug References: 1198101, 1205588, 1205855, 1210382, 1213945, 1215098, 1215099, 1215100, 1215101, 1215102, 1215103, 1219460
CVE References: CVE-2022-28737, CVE-2023-40546, CVE-2023-40547, CVE-2023-40548, CVE-2023-40549, CVE-2023-40550, CVE-2023-40551
Jira References: PED-922
Maintenance Incident: [SUSE:Maintenance:33581](https://smelt.suse.de/incident/33581/)
Sources used:
SUSE Linux Enterprise Server for SAP Applications 12 SP5 (src):
 shim-15.8-25.30.1
SUSE Linux Enterprise High Performance Computing 12 SP5 (src):
 shim-15.8-25.30.1
SUSE Linux Enterprise Server 12 SP5 (src):
 shim-15.8-25.30.1

NOTE: This line indicates an update has been released for the listed product(s). At times this might be only a partial fix. If you have questions please reach out to maintenance coordination.

SUSE-SU-2024:1461-1: An update that solves seven vulnerabilities, contains one feature and has five security fixes can now be installed.

Category: security (important)
Bug References: 1198101, 1205588, 1205855, 1210382, 1213945, 1215098, 1215099, 1215100, 1215101, 1215102, 1215103, 1219460
CVE References: CVE-2022-28737, CVE-2023-40546, CVE-2023-40547, CVE-2023-40548, CVE-2023-40549, CVE-2023-40550, CVE-2023-40551
Jira References: PED-922
Maintenance Incident: [SUSE:Maintenance:33579](https://smelt.suse.de/incident/33579/)
Sources used:
SUSE Linux Enterprise High Performance Computing 15 SP2 LTSS 15-SP2 (src):
 shim-15.8-150100.3.38.1
SUSE Linux Enterprise Server 15 SP2 LTSS 15-SP2 (src):
 shim-15.8-150100.3.38.1
SUSE Linux Enterprise Server for SAP Applications 15 SP2 (src):
 shim-15.8-150100.3.38.1

NOTE: This line indicates an update has been released for the listed product(s). At times this might be only a partial fix. If you have questions please reach out to maintenance coordination.

making public