wolfSSL / wolfssl

The wolfSSL library is a small, fast, portable implementation of TLS/SSL for embedded devices to the cloud. wolfSSL supports up to TLS 1.3 and DTLS 1.3!
https://www.wolfssl.com
GNU General Public License v2.0
2.36k stars 834 forks source link

ML-DSA/Dilithium: obtain security level from DER when decoding #8177

Closed bigbrett closed 2 days ago

bigbrett commented 1 week ago

Description

Similar to #8129, wolfHSM needs to be able to "blindly" decode a DER key without knowing its exact contents. In this case it just knows that an item is an ML-DSA key, and nothing more. This change allows the security level to be set dynamically in the output MlDsaKey struct based on the algorithm OID in the DER file.

Testing

Added new wolfCrypt test coverage for decoding keys without specifying a level

Checklist

SparkiDev commented 1 week ago

Consider changing call to DecodeAsymKey_Assign() to call DecodeAsymKey_ex(), a new function, which returns the OID. Need to rework DecodeASymKey_Assign() to call DecodeAsymKey_ex() with an OID.

bigbrett commented 1 week ago

Consider changing call to DecodeAsymKey_Assign() to call DecodeAsymKey_ex(), a new function, which returns the OID. Need to rework DecodeASymKey_Assign() to call DecodeAsymKey_ex() with an OID.

@SparkiDev I had originally thought about modifying DecodeAsymKey_Assign() since it was already parsing these values out, but decided against it since it would make this change extremely intrusive (e.g. touching code called by basically every public key function). Instead I decided to do it this way to localize the addition to just the Dilithium code.

If you would rather see this rolled out globally then I can do that, but perhaps we could add that in a subsequent PR? I'd prefer to just get this in for Dilithium now, so some wolfHSM features depending on this functionality can be completed. I agree that this would be great generic functionality to have though, and would be happy to take a stab at it.

Let me know your thoughts.

SparkiDev commented 1 week ago

All the PQC algorithms go through here and they pass in the OID. Having the OID discovered would be useful to all these algorithms.

bigbrett commented 1 week ago

@SparkiDev Okay, spent some time today refactoring the code to pull OID detection into DecodeAsymKey_Assign_ex and DecodeAsymKeyPublic_Assign_ex. Added additional test coverage too.

bigbrett commented 1 week ago

Jenkins retest this please

bigbrett commented 3 days ago

Jenkins retest this please

bigbrett commented 3 days ago

Jenkins retest this please

dgarske commented 1 day ago

Hi @bigbrett ,

This PR seems to have broken some of the tests. Can you please review the issue and put up a PR?

[quantum-safe-wolfssl-all-g++-latest-debug] [6 of 39] [ef67b1c06a]
    configure...   real 0m24.978s  user 0m14.014s  sys 0m13.168s
    build...   real 0m28.446s  user 1m40.272s  sys 0m10.845s
    check...FAIL: scripts/unit.test
   real 0m19.072s  user 0m33.427s  sys 0m11.238s

scripts/unit.log tail:
 passed (  0.00001)
wolfSSL Entering wolfSSL_ERR_clear_error
   286: test_wc_dilithium_der                               :RNG_HEALTH_TEST_CHECK_SIZE = 128
sizeof(seedB_data)         = 128
opened /dev/urandom.
rnd read...
ERR TRACE: wolfcrypt/src/dilithium.c L 10026 BAD_FUNC_ARG (-173)
ERR TRACE: wolfcrypt/src/dilithium.c L 10026 BAD_FUNC_ARG (-173)
ERR TRACE: wolfcrypt/src/dilithium.c L 9179 BAD_FUNC_ARG (-173)
FAIL scripts/unit.test (exit status: 139)

stopped at config: 'quantum-safe-wolfssl-all-g++-latest-debug' '--srcdir' '.' '--disable-jobserver' '--enable-option-checking=fatal' '--enable-all' '--enable-testcert' '--enable-acert' '--enable-dtls13' '--enable-dtls-mtu' '--enable-dtls-frag-ch' '--enable-quic' '--enable-debug' '--enable-debug-trace-errcodes' '--enable-sp-math-all' '--enable-experimental' '--enable-kyber=all,original' '--enable-lms' '--enable-xmss' '--enable-dilithium' '--enable-dual-alg-certs' '--disable-qt' 'CPPFLAGS=-DNO_WOLFSSL_CIPHER_SUITE_TEST -DWOLFSSL_OLD_PRIME_CHECK' 'CC=g++-13'