ArmVirtPkg: Reorganize crypto dependencies

[ardbiesheuvel]

Alternative approach to enabling h/w accelerated OpenSSL on ArmVirtPkg, switching to MbedTls in cases where OpenSSL is either overkill, or should be avoided due to its dependency on floating point (32-bit ARM)

[ardbiesheuvel]

@os-d

[kraxel]

Hmm, this patch does a bunch of different changes. Can we untangle this please?

Change 1 is switching ARM (32-bit) to mbedtls, to allow dropping the softfloat library. No objections here.

Change 2 is switching AARCH64 to mbed tls too in some cases. I'd very much prefer to stick to openssl here, or at least have the option to build all components against openssl. First because mbedtls is problematic for linux distros downstream. Second because using openssl in PEI should not be too much of a problem. On x86 I've checked the size impact openssl has, and with PEI needing only some hash functions for TPM measurements the size impact is quite small due to LTO removing >90% of openssl. Third because having the asm-accelerated openssl hash function for measurements should improve boot performance.

Change 3 is switching to the crypto driver. That makes sense, but should IMO be a separate PR request. I have an outdated patch series to switch ovmf to the crypto driver, including some changes for cryptopkg such as creating include files for the PCD-based crypto driver feature configuration. See https://github.com/kraxel/edk2/commits/ovmf-crypto/, feel free to pick changes from there.

[kraxel]

While being at it a slightly related issue I've trapped into: We have an RNG problem with openssl.

RngDxe supports both trng and rndr, by switching between RngLib (handling rndr) and ArmTrngLib (handling trng) at runtime.

RngLib supports rngr only, which is not supported by kvm. OpensslLib uses RngLib -> rng not working for openssl on kvm -> breaks TlsDxe -> breaks https boot.

[ardbiesheuvel]

Hmm, this patch does a bunch of different changes. Can we untangle this please?

Fair point.

Change 1 is switching ARM (32-bit) to mbedtls, to allow dropping the softfloat library. No objections here.

Ack

Change 2 is switching AARCH64 to mbed tls too in some cases. I'd very much prefer to stick to openssl here, or at least have the option to build all components against openssl. First because mbedtls is problematic for linux distros downstream.

Is this because of of silly FIPS/certification concerns?

Second because using openssl in PEI should not be too much of a problem. On x86 I've checked the size impact openssl has, and with PEI needing only some hash functions for TPM measurements the size impact is quite small due to LTO removing >90% of openssl. Third because having the asm-accelerated openssl hash function for measurements should improve boot performance.

Good point.

Change 3 is switching to the crypto driver. That makes sense, but should IMO be a separate PR request. I have an outdated patch series to switch ovmf to the crypto driver, including some changes for cryptopkg such as creating include files for the PCD-based crypto driver feature configuration. See https://github.com/kraxel/edk2/commits/ovmf-crypto/, feel free to pick changes from there.

I'll take a look.

While being at it a slightly related issue I've trapped into: We have an RNG problem with openssl.

RngDxe supports both trng and rndr, by switching between RngLib (handling rndr) and ArmTrngLib (handling trng) at runtime.

RngLib supports rngr only, which is not supported by kvm. OpensslLib uses RngLib -> rng not working for openssl on kvm -> breaks TlsDxe -> breaks https boot.

cc @pierregondois

Could we use MdePkg/Library/DxeRngLib/DxeRngLib.inf instead?

[pierregondois]

RngDxe supports both trng and rndr, by switching between RngLib (handling rndr) and ArmTrngLib (handling trng) at runtime.
RngLib supports rngr only, which is not supported by kvm. OpensslLib uses RngLib -> rng not working for openssl on kvm -> breaks TlsDxe -> breaks https boot.

cc @pierregondois

Could we use MdePkg/Library/DxeRngLib/DxeRngLib.inf instead?

@kraxel

IIRC RNDR is supported by kvm. I think it worked while testing it with kvmtool. Did you face some issue on that front, or am I not remembering well ?

I agree that using DxeRngLib would be better and give much more flexibility. If FEAT_RNRD is not implemented, we can fallback to the TRNG.

Right now the RngDxe (which is called through DxeRngLib) can provide:

• Depending on the RngLib:
  • CTR-DRBG/ Arm custom DRBG through RNDR instruction
  • Unsafe BaseRngTimerLib
• TRNG through firmware

The RngLib is also used at many places without safety check (i.e. checking that the implementation is safe, meaning not the Timerlib). I was briefly working on something around that.

[kraxel]

Change 2 is switching AARCH64 to mbed tls too in some cases. I'd very much prefer to stick to openssl here, or at least have the option to build all components against openssl. First because mbedtls is problematic for linux distros downstream.

Is this because of of silly FIPS/certification concerns?

Yes. Distros, especially the enterprise ones, try to minimize the number of crypto libs they are using. The certification process needed for crypto stuff is one of the reasons.

[kraxel]

IIRC RNDR is supported by kvm. I think it worked while testing it with kvmtool. Did you face some issue on that front, or am I not remembering well ?

The behavior I see is the rndr bit in isar0 not being set, which leads to BaseRngLibConstructor setting mRndrSupported to FALSE. Which in turn makes all function fail without even trying to execute the rngr instruction. This is with qemu.

[pierregondois]

IIRC RNDR is supported by kvm. I think it worked while testing it with kvmtool. Did you face some issue on that front, or am I not remembering well ?

The behavior I see is the rndr bit in isar0 not being set, which leads to BaseRngLibConstructor setting mRndrSupported to FALSE. Which in turn makes all function fail without even trying to execute the rngr instruction. This is with qemu.

If the RNDR bit is not set, would it be possible that the emulated CPU doesn't support RNDR instruction ? Meaning that a more recent CPU being emulated would support it. FEAT_RNG was enabled in Armv8.5.

Is is possible to get the command line you are using ?

[samimujawar]

Is it possible to try the qemu '-cpu max' option, please?
I think you may also require '-M virt'

[ardbiesheuvel]

Is it possible to try the qemu '-cpu max' option, please? I think you may also require '-M virt'

-cpu max does not work with KVM. Under KVM, there is no CPU emulation, but only virtualization of the host CPU. This means that RNDR cannot be supported in the guest if it is not implemented on the host.

Whether or not we might be able to emulate a CPU that does support RNDR is rather immaterial: emulation is a development tool with 1-2 orders of magnitude worse performance whereas virtualization is the production use case for this firmware. Note that the same applies to kvmtool, which does not implement emulation at all. In those VMs, RNDR is also entirely unsupported unless the host CPU has support for it.

[pierregondois]

Is it possible to try the qemu '-cpu max' option, please? I think you may also require '-M virt'

-cpu max does not work with KVM. Under KVM, there is no CPU emulation, but only virtualization of the host CPU. This means that RNDR cannot be supported in the guest if it is not implemented on the host.

Whether or not we might be able to emulate a CPU that does support RNDR is rather immaterial: emulation is a development tool with 1-2 orders of magnitude worse performance whereas virtualization is the production use case for this firmware. Note that the same applies to kvmtool, which does not implement emulation at all. In those VMs, RNDR is also entirely unsupported unless the host CPU has support for it.

Yes right. I was not getting the sentence below. It is not that RNDR is not supported by kvm, it's that the underlying hardware doesn't have FEAT_RNG then.

RngLib supports rngr only, which is not supported by kvm. OpensslLib uses RngLib -> rng not working for openssl on kvm -> breaks TlsDxe -> breaks https boot.

Assuming we all agree on this, using the DxeRngLib is a good idea IMO

[kraxel]

Is it possible to try the qemu '-cpu max' option, please? I think you may also require '-M virt'

-cpu max does not work with KVM. Under KVM, there is no CPU emulation, but only virtualization of the host CPU. This means that RNDR cannot be supported in the guest if it is not implemented on the host.

Yes, it's kvm + -cpu host. How can I figure whenever the host cpu supports rngr?

Host machines I've tries are an raspberry pi 4 and a macbook (with M2).

[pierregondois]

cat /proc/cpuinfo | grep Features | head -1

should include rng as a supported feature, but this has only been on models so far

[kraxel]

cat /proc/cpuinfo | grep Features | head -1

should include rng as a supported feature, but this has only been on models so far

No rng on both machines.

[ardbiesheuvel]

cat /proc/cpuinfo | grep Features | head -1

should include rng as a supported feature, but this has only been on models so far

No rng on both machines.

Not surpising: check Marcin's ARM CPU feature table: https://gpages.juszkiewicz.com.pl/arm-socs-table/arm-socs.html, this has just started getting rolled out only on the most recent designs.

[ardbiesheuvel]

I've dropped the CryptoDxe changes for now, and focused on moving 32-bit ARM to MbedTls so we can get rid of ArmSoftFloatLib