[RFC][IN-PROGRESS] RFC: Migrate to edk2-Crypto#8
Conversation
|
|
||
| ### What Moves to edk2-crypto | ||
|
|
||
| - **OpensslPkg** - OpenSSL-based BaseCryptLib + OpensslLib + IntrinsicLib |
There was a problem hiding this comment.
EDK2 didn't allow to use submodule's submodule:
https://github.com/tianocore/edk2?tab=readme-ov-file#submodules
There was a problem hiding this comment.
In the spirit of what this change is trying to do I suggest we move more crypto details out of the edk2 repository so code there depends on project-authorized public interfaces that support crypto flexibility. edk2 builds can then be satisfied with null instances of those interfaces avoiding submodules to concrete crypto providers entirely in the edk2 repo.
The edk2-crypto repo needs those crypto provider implementations and it would be where those dependency details are controlled:
- edk2-crypto (OpenSslPkg, MbedTlsPkg, CryptoBinPkg)
- External crypto provider submodule 1: openssl
- External crypto provider submodule 2: mbedtls
- ...
In that model, crypto provider packages wrap around the external crypto provider source to abstract it to edk2 code including CryptoBinPkg.
edk2 repo
- Depends only on crypto public interfaces defined in CryptoPkg (e.g.
BaseCryptLib) - Platforms in the edk2 repo such as ArmVirtPkg and OvmfPkg will specify a package-specific edk2 crypto binary to use for their actual crypto operations (which implements a CryptoPkg public interface such as
BaseCryptLib)- Those demonstrates proposed platform best practices to use the edk2 crypto binary
This means: Any code not using proper crypto abstractions in edk2 today will need to either be refactored to use those abstractions or move out of edk2.
Platforms
Downstream platforms have several options:
- Pure Binary path:
- Interfaces: edk2 (CryptoPkg)
- Implementation: edk2-crypto binary
- Pure Source path:
- Interfaces: edk2 (CryptoPkg)
- Implementation: edk2-crypto source provider (OpenSslPkg, MbedTlsPkg, etc.)
- Hybrid path:
- Interfaces: edk2 (CryptoPkg)
- Implementation:
- edk2-crypto binary
- other
Recommendation is to use edk2-crypto binary anywhere possible. Practically, other crypto code may be needed to accomplish platform security goals.
There was a problem hiding this comment.
I believe the updated RFC includes this feedback
|
|
||
| - Platforms must be able to build with edk2-crypto as a submodule | ||
| - No C code changes required for migration (DSC/submodule only) | ||
| - Both OpensslPkg and MbedTlsPkg must implement BaseCryptLib.h |
There was a problem hiding this comment.
You might want to consider the breakout of crypto providers to OpensslPkg and MbedTlsPkg in a separate and dedicated RFC.
That can happen independently of a repo switch. The edk2-crytpo repo RFC could then refer to that RFC.
There was a problem hiding this comment.
I think I want to keep the OpensslPkg and MbedTlsPkgs in this RFC given the scope of the required changes and that this may be the moment to make a "breaking" change.
This responds to some feedback provided and updates the rfc accordingly Signed-off-by Doug Flick <dougflick@microsoft.com>
cf3472d to
dee9d33
Compare
|
|
||
| ## Backward Compatibility | ||
|
|
||
| No crypto API changes are required in EDK2. Existing platforms can continue source-based mappings or adopt binary mappings. |
There was a problem hiding this comment.
I think we need consider the forward compatibility. What if EDK2 wants to add new APIs?
Beyond normal RSA signature and SHA, I have seen the platform keep adding new requirement to expose more crypto primitive APIs, including RSA-OEAP, RSA-PSS, AES, HMAC, HKDF, ECC, BigNumber, parallel hash, HBS (LMS, XMSS), PQC (ML-DSA, SLH-DSA), ...
Is the scope only focus on UEFI/TCG specification required feature? Or to serve any platform specific usage?
There was a problem hiding this comment.
Yeah I absolutely agree,
We would need to think about how this works because edk2-crypto would depend on BaseCryptLib.h and thus needs this as something it can refererence.
In practice BaseCryptLib.h still lives in CryptoPkg in edk2 and thus when a new API is needed.
In Edk2:
- Add the header definition
- Create a null implementation
- Add Unit Tests that would validate the implementation
This would allow the CI to run (but obviously not implement the behavior)
Then in edk2-crypt:
- Get the latest
BaseCryptLib.h - Implement the OpensslPkg and MbedTlsPkg implementations
- Execute unit tests locally
- Produce the binary
Edk2:
- Integrate binaries into OVMF and validate
|
|
||
| ### Platform Integration | ||
|
|
||
| - OVMF as primary validation (Secure Boot, authenticated variables, TLS) |
There was a problem hiding this comment.
Agree. Secure Boot and Auth Variable can be validated on OVMF.
But I am not sure how to use OVMF to validate TLS today. Thought?
There was a problem hiding this comment.
Actually a little bit although this isn't complete yet.
You can query what the cipher suite provides through a unit test without a server.
From my Openssl-4.0.0 enablement.
[TLS Cipher Suite Report]
TLS_RSA_WITH_AES_128_CBC_SHA (0x002F) .. SUPPORTED
TLS_RSA_WITH_AES_256_CBC_SHA (0x0035) .. SUPPORTED
TLS_RSA_WITH_AES_128_CBC_SHA256 (0x003C) .. SUPPORTED
TLS_RSA_WITH_AES_256_CBC_SHA256 (0x003D) .. SUPPORTED
TLS_DHE_RSA_WITH_AES_128_CBC_SHA256 (0x0067) .. SUPPORTED
TLS_DHE_RSA_WITH_AES_256_CBC_SHA256 (0x006B) .. SUPPORTED
TLS_RSA_WITH_AES_128_GCM_SHA256 (0x009C) .. SUPPORTED
TLS_RSA_WITH_AES_256_GCM_SHA384 (0x009D) .. SUPPORTED
TLS_DHE_RSA_WITH_AES_128_GCM_SHA256 (0x009E) .. SUPPORTED
TLS_DHE_RSA_WITH_AES_256_GCM_SHA384 (0x009F) .. SUPPORTED
TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 (0xC027) .. SUPPORTED
TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384 (0xC028) .. SUPPORTED
TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (0xC02F) .. SUPPORTED
TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (0xC030) .. SUPPORTED
Total supported: 14 / 14
GCM ciphers supported: 6
[DFCI Required Cipher Suites]
TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (0xC030) .. OK
TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (0xC02F) .. OK
TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384 (0xC028) .. OK
TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 (0xC027) .. OK
All 4 DFCI required ciphers present.
[TLS Version Support Report]
TLS 1.0 .............. SUPPORTED
TLS 1.1 .............. SUPPORTED
TLS 1.2 .............. SUPPORTED
[TLS EC Curve Support Report]
P-256 (secp256r1) ......... SUPPORTED
P-384 (secp384r1) ......... SUPPORTED
P-521 (secp521r1) ......... SUPPORTED
X25519 ......... SUPPORTED
X448 ......... SUPPORTED
EC curves supported: 5 / 5
[TLS 1.3 Support Report]
TLS 1.3 context creation .. SUPPORTED
TLS_AES_128_GCM_SHA256 (0x1301) .. SUPPORTED
TLS_AES_256_GCM_SHA384 (0x1302) .. SUPPORTED
TLS_CHACHA20_POLY1305_SHA256 (0x1303) .. SUPPORTED
TLS_AES_128_CCM_SHA256 (0x1304) .. SUPPORTED
TLS 1.3 ciphers supported: 4 / 4
[PQC Hybrid Key Exchange Group Report]
X25519MLKEM768 .... SUPPORTED
SecP256r1MLKEM768 .... SUPPORTED
SecP384r1MLKEM1024 .... SUPPORTED
PQC groups supported: 3 / 3
[TLS Security Level Report]
Security Level 0 .. UNSUPPORTED
Security Level 1 .. UNSUPPORTED
Security Level 2 .. OK
Security Level 3 .. OK
Security Level 4 .. OK
Security Level 5 .. OK
For full functionality we would have to likely write a TLS app and have a orchestrator
| |-------------------------------------|-----------------------------------------| | ||
| | Interface changes during transition | Freeze BaseCryptLib.h during migration | | ||
| | CI issues | Document manual build/test procedures | | ||
| | Integration complexity | Provide setup scripts and documentation | |
There was a problem hiding this comment.
I think we can provide a sample DSC.inc for platform to include.
| ### What Moves to edk2-crypto | ||
|
|
||
| - **OpensslPkg** - OpenSSL-based BaseCryptLib + OpensslLib + IntrinsicLib | ||
| - **MbedTlsPkg** - MbedTLS-based BaseCryptLib + MbedTlsLib + IntrinsicLib |
There was a problem hiding this comment.
Today, MbedTls build will include OpenSSL for SM3.
Is that something forbiden?
There was a problem hiding this comment.
I don't think I'm against it from a 50,000 ft level. I think the goal should be to eventually use MbedTls as a "pure" implementation but as you're alluding to has a bunch of issues
I think I found another issue where Pkcs7Verify(..) in MbedTls where it can't verify a self signed CA.
Let me think more about this -- I think my goal would be that all crypto providers must pass the same set of tests -- but how it achieves that (mbedtls depending on openssl) could be up for debate. Just from a dependency tracking perspective this does make it hard to know if you are vulnerable to a openssl or mbedtls CVE.
| This reorganization is **transparent to end users** - firmware behavior remains identical: | ||
|
|
||
| - **No visible changes** - Boot experience, Secure Boot, and all crypto-dependent features work exactly as before | ||
| - **No new capabilities exposed** - This is an internal architecture change, not a feature addition |
There was a problem hiding this comment.
Do we drop old capabilities? I would prefer to say "No capabilities changed"
| ### Platform Integration | ||
|
|
||
| - OVMF as primary validation (Secure Boot, authenticated variables, TLS) | ||
| - Parallel builds during transition to verify equivalence |
There was a problem hiding this comment.
Question: Is this platform integration test required step BEFORE edk2-crypto release?
| ### For Package Developers | ||
|
|
||
| - No code changes required - BaseCryptLib.h unchanged and remains the contract | ||
| - Crypto changes happen in edk2-crypto, opaque to EDK2 mainline |
There was a problem hiding this comment.
How we handle the Crypto Interface change?
For example, when we want to add ECIT in SecurityPkg, we may require crypto API change in CryptoPkg to let OpenSSL or MbedTLS expose capability, which impact to edk2-Crypto as well.
What is the expected working flow?
Especially the validation/test flow to ensure the change in edk2-crypto is good to enable new CryptoPkg API, and server the ECIT driver in SecurityPkg.
| edk2-crypto repo impact: | ||
|
|
||
| - Owns provider wrappers and their dependency control: | ||
| - OpensslPkg |
There was a problem hiding this comment.
Even one crypto provider may support different configuration.
E.g. Up to date OpenSSL LTS version, v.s. old FIPS certified version.
Any consideration for that?
There was a problem hiding this comment.
So admittedly I havent spent a lot of time thinking about this.
My initial thought was that we could mirror what Openssl is doing with branches
https://github.com/openssl/openssl/branches
Edk2-Crypto-LTS-2.0.0
That being said I might need to think more on this aspect. Open to suggestions if you have them.
5 participants
RFC: Migrate to Edk2-Crypto
This RFC proposes new separation boundaries between edk2 and edk2-crypto.
The end goal here is to unify crypto history and improve reviews by cryptographic auditors.
There are additional separate benefits to this model detailed in the RFC.
This proposed RFC describes how to separate and what items would go where.
Display the Rich Text for the best view of the diagrams.