Addition of generic NIST-DSA PKEY and ASN1 to support ML-DSA #1963

jakemas · 2024-11-04T20:30:36Z

Issues:

Resolves #CryptoAlg-2725

Description of changes:

This is a sizable PR, I've worked extremely hard to keep the size down in a single PR, but with ML-DSA touching X.509, PKEY, ASN1, there are a lot of changes that need to happen simultaneously to preserve library functionality, so I will document this PR well.

1. PKEY structure changes

This PR adds the support to include additional signature algorithms (in this case ML-DSA-44 and ML-DSA-87) to AWS-LC. Before this PR AWS-LC only supported ML-DSA-65, as such, we were utilizing the void *ptr of evp_pkey_st, rather than a distinct structure for ML-DSA.

This PR introduces the new structure PQDSA_KEY * pqdsa_key; inside evp_pkey_st to support ML-DSA and any additional FIPS digital signature algorithms provided by NIST, should AWS-LC wish to include them in the library.

  union {
    void *ptr;
    RSA *rsa;
    DSA *dsa;
    DH *dh;
    EC_KEY *ec;
    KEM_KEY *kem_key;
    PQDSA_KEY * pqdsa_key;
  } pkey;

While the structure DSA already exists it does not provide support for FIPS-like signature APIs (see more at NIST api conventions. Rather than create a PKEY struct of the form MLDSA_KEY that can only support ML-DSA, this is an opportunity to build support for future signature algorithms that have similar. calling structures, de-randomized testing modes, API converntions, etc. by making a more generic structure type -- much like we did for KEM_KEY. Under the design in this PR, adding SLH-DSA to the PKEY struct would be very simple, as it conforms to a PQDSA_KEY in design. So too will be true of any additional signature algorithms produced by NISTs call for additional signature algorithms.

As, such, PQDSA_KEY utilizes a new structure to hold public/secret keys, as well as a PQDSA struct which defines signature algorithm specific information:

struct pqdsa_key_st {
  const PQDSA *pqdsa;
  uint8_t *public_key;
  uint8_t *secret_key;
};

typedef struct {
  int nid;
  const uint8_t *oid;
  uint8_t oid_len;
  const char *comment;
  size_t public_key_len;
  size_t secret_key_len;
  size_t signature_len;
  size_t keygen_seed_len;
  size_t sign_seed_len;
  const PQDSA_METHOD *method;
} PQDSA;

This allows us to use a single PKEY structure for all NIST FIPS signature algorithms, much like the existing PKEY struct KEM_KEY *kem_key for KEMS. As a consequence, we are now able to define signature methods such as:

DEFINE_LOCAL_DATA(PQDSA_METHOD, sig_ml_dsa_65_method) {
  out->keygen = ml_dsa_65_keypair;
  out->sign = ml_dsa_65_sign;
  out->verify = ml_dsa_65_verify;
}

DEFINE_LOCAL_DATA(PQDSA, sig_ml_dsa_65) {
  out->nid = NID_MLDSA65;
  out->oid = kOIDMLDSA65;
  out->oid_len = sizeof(kOIDMLDSA65);
  out->comment = "MLDSA65 ";
  out->public_key_len = MLDSA65_PUBLIC_KEY_BYTES;
  out->secret_key_len = MLDSA65_PRIVATE_KEY_BYTES;
  out->signature_len = MLDSA65_SIGNATURE_BYTES;
  out->keygen_seed_len = MLDSA65_KEYGEN_SEED_BYTES;
  out->sign_seed_len = MLDSA65_SIGNATURE_SEED_BYTES;
  out->method = sig_ml_dsa_65_method();
}

much like the existing kem.c file.

These structures will be incredibly useful for subsequent PRs, in which we will be adding de-randomized APIs to support FIPS validation and KATs from seeds. In effect, we will be extending the method to include:

DEFINE_LOCAL_DATA(PQDSA_METHOD, sig_ml_dsa_65_method) {
  out->keygen = ml_dsa_65_keypair;
  out->keygen_internal = ml_dsa_65_keypair_internal;
  out->sign = ml_dsa_65_sign;
  out->sign_internal = ml_dsa_65_sign_internal;
  out->verify = ml_dsa_65_verify;
}

The directory dilithium/p_dilithium3.c is used to house generic PKEY operations, as well as PQDSA specific EVP functions such as EVP_PKEY_pqdsa_set_params, EVP_PKEY_pqdsa_set_params, which work in a similar way to p_kem.c functions EVP_PKEY_CTX_kem_set_params and EVP_PKEY_kem_set_params.

The directory dilithium/p_dilithium3_asn1.c is used to house generic asn.1 operations for NIST FIPS DSA algorithms.

Rather than rename/relocate these files in this PR (which will convolute the CR), I will move these files in a subsequent PR.

2. Internal Design

We introduce new structures: PQDSA_METHOD, PQDSA, PQDSA_KEY which are very similar to how both EC Crypto, and the KEM API is implemented. The figure above shows the newly proposed structures and their integration into the existing EVP structures.

PQDSA_METHOD is a table of function pointers with 5 functions defined for a PQDSA: key generation, key generation internal (for testing/FIPS), sign, sign internal (for testing/FIPS), and verify. Every PQDSA implementation has to implement this API, for example sig_ml_dsa_44_method implements the three functions for ML-DSA-44 (analogous to EC_METHOD and for example, EC_GFp_nistz256_method).
PQDSA is a structure that holds basic information about the DSA: the id, size of parameters, and the pointer to the implementation of the corresponding PQDSA_METHOD.
PQDSA_KEY structure is a helper structure that holds pointers to public and secret key and the pointer to the corresponding PQDSA.
PQDSA_PKEY_CTX is a helper structure used to store DSA parameters in an EVP_PKEY_CTX object (the same as EC_PKEY_CTX). Since PQDSA has everything we need, that’s what we store in PQDSA_PKEY_CTX.

3. OID changes

We now have real OIDs available from https://csrc.nist.gov/projects/computer-security-objects-register/algorithm-registration. These have been added to the obj files and automatically populated by go run objects.go. We include the following NIDs for ML-DSA:

//2.16.840.1.101.3.4.3.17
static const uint8_t kOIDMLDSA44[]  = {0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x03, 0x11};
//2.16.840.1.101.3.4.3.18
static const uint8_t kOIDMLDSA65[]  = {0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x03, 0x12};
//2.16.840.1.101.3.4.3.19
static const uint8_t kOIDMLDSA87[]  = {0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x03, 0x13};

and a "top-level" NID EVP_PKEY_NISTDSA (similar to EVP_PKEY_KEM to subset all NIST FIPS DSA algorithms. Much like for the KEM API, we include functions SIG_find_dsa_by_nid(int nid) to return actual algorithm specific methods.

4. Testing structure changes

Prior to this PR all testing for ML-DSA-65 was done as a series of g-tests. As we now have the ability to get algorithm/security level specific parameters from the NISTDSA struct, I have overhauled the testing suite to be parameterized over the currently supported signature algorithms:

static const struct ML_DSA parameterSet[] = {
  {"MLDSA65", NID_MLDSA65, 1952, 4032, 3309,  "dilithium/kat/mldsa65.txt", mldsa65kPublicKey, mldsa65kPublicKeySPKI, 1973},
};

For each of the above parameter sets we run:

TEST_P(MLDSAParameterTest, KAT)
TEST_P(MLDSAParameterTest, KeyGen)
TEST_P(MLDSAParameterTest, KeyCmp) 
TEST_P(MLDSAParameterTest, KeySize) 
TEST_P(MLDSAParameterTest, NewKeyFromBytes) 
TEST_P(MLDSAParameterTest, RawFunctions) 
TEST_P(MLDSAParameterTest, SIGOperations) 
TEST_P(MLDSAParameterTest, MarshalParse)

5. X.509 changes

Changes to X.509 code have been minimized to only the required changes to support this PR. This is already a big PR and I want to avoid expansion wherever possible. For X.509 the changes predominantly are regarding the change of NIDs from the old Dilithium NIDs to the new NIST DSA NIDs/OIDs.

As the OIDs changed, so too much the test certificates kDilithium3Cert, kDilithium3CertNull, and kDilithium3CertParam. These have been regenerated using the following:

Tool used https://github.com/google/der-ascii

1. First generate a valid test certificate in PEM encoding, say cert.pem
2. Convert PEM to DER: openssl x509 -in cert.pem -out cert.der -outform DER
3. Convert DER to ASCII: der2ascii -i cert.der >> cert.txt
4. Make edits to cert.txt as desired, say; certnew.txt
5. Convert ASCII to DER: ascii2der -i certnew.txt >> certnew.der
6. Convert DER to PEM: openssl x509 -in certnew.der -inform DER -out certnew.pem -outform PEM

For step (1) "generate a valid test certificate" you can use the X.509 test Dilithium3SignVerifyCert with added code to print the certificate (or store to file):

TEST(X509Test, Dilithium3SignVerifyCert) {
  // Generate mldsa key
  bssl::UniquePtr<EVP_PKEY_CTX> ctx(EVP_PKEY_CTX_new_id(EVP_PKEY_NISTDSA, nullptr));
  ASSERT_TRUE(ctx);
  ASSERT_TRUE(EVP_PKEY_CTX_nistdsa_set_params(ctx.get(), NID_MLDSA65));
  ASSERT_TRUE(EVP_PKEY_keygen_init(ctx.get()));
  EVP_PKEY *raw = nullptr;
  ASSERT_TRUE(EVP_PKEY_keygen(ctx.get(), &raw));
  bssl::UniquePtr<EVP_PKEY> pkey(raw);
  ctx.reset(EVP_PKEY_CTX_new(pkey.get(), nullptr));

  bssl::UniquePtr<X509> leaf =
      MakeTestCert("Intermediate", "Leaf", pkey.get(), false);
  ASSERT_TRUE(leaf);

  bssl::ScopedEVP_MD_CTX md_ctx;
  EVP_DigestSignInit(md_ctx.get(), nullptr, nullptr, nullptr, pkey.get());
  ASSERT_TRUE(X509_sign_ctx(leaf.get(), md_ctx.get()));

// print certificate and PEM encoding:
  bssl::UniquePtr<BIO> bio(BIO_new(BIO_s_mem()));
  X509_print(bio.get(), leaf.get());
  PEM_write_bio_X509(bio.get(),leaf.get());
  const uint8_t *out;
  size_t outlen;
  ASSERT_TRUE(BIO_mem_contents(bio.get(), &out, &outlen));
  printf("%s",out);
}

6. Speed Tool

ML-DSA-65 has been added to the speed tool. To facilitate the new APIs used in this PR, the API version has been incremented by 1. The speed tool now produces benchmarks for all three parameter levels, example output:

Did 2871 MLDSA65 keygen operations in 1051869us (2729.4 ops/sec)
Did 774 MLDSA65 signing operations in 1039468us (744.6 ops/sec)
Did 2893 MLDSA65 verify operations in 1089059us (2656.4 ops/sec)

Call-outs:

algorithm.c : modified OIDs from DILITHIUM to PQDSA
base.h : added PQDSA_KEY struct
evp.h : added EVP_PKEY_pqdsa_set_params and EVP_PKEY_CTX_pqdsa_set_params functions to setting the specific OID (NID_MLDSAXX) for a PKEY of type EVP_PKEY_NISTDSA
evp_extra_test.cc : updated example MLDSA65 private key with new OID and encoding
dilithium/internal.h : new file to hold ML-DSA structure
evp_extra/internal.h : modified ML-DSA-65 specific ASN.1 and PKEY method to a generic one for all NIST PQC signature algorithms
fipsmodule/evp/internal.h : added PQDSA_KEY to PKEY structures to hold PKEY types for NIST PQC signature algorithms
nid.h, obj_dat.h, obj_mac.num, obj_xref.c, objects.txt: all modified to include designated OIDs for ML-DSA.
p_dilithium3.c: temporarily holds PQDSA new/clear/init functions. All existing PKEY functions (keygen/sign/verify) have been modified to use generic PQDSA and extract algorithm specific parameters/function methods.
p_dilithium3_asn1.c: All existing ASN1 functions (get/set/encode/decode/size/bits/free) have been modified to use generic PQDSA and extract algorithm specific parameters/function methods.
p_dilithium_test.cc : has been modified to parameterize all tests into a single test suite that use algorithm specific parameterization
evp_extra/p_methods: dilithium specific methods renamed
print.c, speed.cc, x509_test.cc : updated to retain functionality

Testing:

See above for description of changes to testing framework.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.

codecov-commenter · 2024-11-04T20:53:32Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.90%. Comparing base (48a4057) to head (fb8631e).
Report is 38 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1963      +/-   ##
==========================================
+ Coverage   78.73%   78.90%   +0.17%     
==========================================
  Files         590      595       +5     
  Lines      101428   102095     +667     
  Branches    14384    14476      +92     
==========================================
+ Hits        79856    80559     +703     
+ Misses      20935    20887      -48     
- Partials      637      649      +12

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

dkostic

I reviewed up to p_dilithium3_asn1.c, will continue later.

I was also checking p_dilithium3.c against kem.c and p_kem.c, so far everything looks consistent to me.

crypto/dilithium/internal.h

crypto/dilithium/p_dilithium3.c

dkostic · 2024-11-06T15:08:21Z

crypto/dilithium/p_dilithium3.c

+    return 0;
+  }
+
+  // if the public key has already been set either by EVP_parse_public_key or


this doesn't sound safe. For example, what happens to the security of ML-DSA if you use one variant parameters with another variant keys?

Seconding that, we can check whether the params of the existing key match the params being set. But that seems not needed and it should be an error as is done in EVP_PKEY_CTX_nistdsa_set_params. What is this function used for? Does it need to be exported? EVP_PKEY_kem_set_params in https://github.com/aws/aws-lc/blob/main/crypto/fipsmodule/evp/p_kem.c#L357 is static and is used in importing keys.

Okay, I moved EVP_PKEY_pqdsa_set_params from evp.h so it is no longer exported (I was having problems getting the benchmarking suite speed.cc to find the new API and I thought this was because it needed to be exported, but I just needed to increment the API level) in e47ef83. The function now lives within internal.h

As to what this function is used for, and how that differs to the KEM param set function:

Both EVP_PKEY_pqdsa_set_params and EVP_PKEY_kem_set_params set the NID specific parameters for a given PKEY. Why is this required? We use a higher level NID, namely EVP_PKEY_KEM for KEMS and EVP_PKEY_NISTDSA for PQ DSA. The PKEY has this NID, but doesn't know which specific algorithm is being referred to, so the set_params function sets algorithm specific parameters and methods.

For signature algorithms, we use a flurry of APIs that generate PKEYs based on OIDs, that will then require the specific algorithm OID to fully complete the PKEY structure. For example, consider the ASN.1 utility function:

EVP_PKEY *EVP_parse_public_key(CBS *cbs);

This function returns a newly-allocated EVP PKEY on success, however, this PKEY OID will be set to EVP_PKEY_NISTDSA and not a specific OID like NID_MLDSA65. Thus, to populate the PKEY with the correct parameters and methods, we call EVP_PKEY_pqdsa_set_params on the PKEY. You can see examples of this in the test code, for example, within TEST_P(MLDSAParameterTest, MarshalParse) :

// decode the DER structure, then parse as a PKEY. CBS cbs; CBS_init(&cbs, der, der_len); bssl::UniquePtr<EVP_PKEY> pkey_from_der(EVP_parse_public_key(&cbs)); ASSERT_TRUE(pkey_from_der); // set the correct params for the PKEY EVP_PKEY_pqdsa_set_params(pkey_from_der.get(), nid);

I've been working on an implementation more similar to how this is done for KEMs based upon Nevine's comments above.

Due to the single EVP PKEY type EVP_PKEY_PQDSA that covers different signature algorithms, we have the same issue as EC and KEM EVP PKEY types, in that a more specific algorithm identifier is required to set the appropriate sized/algorithm specific key buffers.

Rather than to use EVP_PKEY_pqdsa_set_params to retroactively set the correct params once a PKEY has been created, I have implemented EVP_PKEY_pqdsa_new_raw_public_key and EVP_PKEY_pqdsa_new_raw_secret_key that take a nid so they can assign the correct algorithm specific methods/params:

// EVP_PKEY_pqdsa_new_raw_public_key generates a new EVP_PKEY object of type // EVP_PKEY_PQDSA, initializes the PQDSA key based on |nid| and populates the // public key part of the PQDSA key with the contents of |in|. It returns the // pointer to the allocated PKEY on sucess and NULL on error. OPENSSL_EXPORT EVP_PKEY *EVP_PKEY_pqdsa_new_raw_public_key(int nid, const uint8_t *in, size_t len); // EVP_PKEY_pqdsa_new_raw_secret_key generates a new EVP_PKEY object of type // EVP_PKEY_PQDSA, initializes the PQDSA key based on |nid| and populates the // secret key part of the PQDSA key with the contents of |in|. It returns the // pointer to the allocated PKEY on sucess and NULL on error. OPENSSL_EXPORT EVP_PKEY *EVP_PKEY_pqdsa_new_raw_secret_key(int nid, const uint8_t *in, size_t len);

see the new implementation in c0d4e65.

EVP_PKEY_pqdsa_set_params now behaves as EVP_PKEY_kem_set_params does, and is not exported.

crypto/dilithium/p_dilithium3.c

nebeid

Still reviewing

crypto/dilithium/internal.h

nebeid · 2024-11-06T18:54:41Z

crypto/dilithium/internal.h

+  int (*sign)(uint8_t *sig, size_t *sig_len,
+              const uint8_t *message,
+              size_t message_len,
+              const uint8_t *ctx,


Can ctx be used via the EVP API?

The argument ctx perhaps is a confusing name, it comes directly from the FIPS 204 standard page 17 and 18.

"The signing algorithm ML-DSA.Sign (Algorithm 2) takes a private key, a message, and a context string as
input. Algorithm 2 ML-DSA.Sign(sk, M, ctx)".

This is not a PKEY, EVP, (or any other type of) ctx.

Upstream reference Dilithium call it "pre" (https://github.com/pq-crystals/dilithium/blob/master/ref/sign.c#L89) short for "prefix string". But I wanted it to be obvious from the standard what this argument was, so went with the standard name.

Thanks. I was asking whether the EVP_PKEY API will be able to accommodate that extra parameter.

Ahh I understand now! Currently no, there is no way to set ctx from the EVP API. Should we require that functionality, it can be added.

For additional background, the standard states: "By default, the context is the empty string, though applications may specify the use of a non-empty context string." I believe this was added as an optional meta-data field when adding in the concept of domain separation for pre-hash vs pure signing modes.

Should we want to make the ctx settable at the EVP API we'd need to make changes to the EVP API that I currently do not see an application for.

please document that in the code. btw, ctx is very unfortunate name, can we change it to ctx_string or something like that?

Yes! Changed to pre for prefix string as has now also been changed upstream (https://github.com/pq-crystals/dilithium/blob/master/ref/sign.c#L78). See bcbb832

nebeid · 2024-11-06T18:57:43Z

crypto/dilithium/internal.h

+  int (*keygen)(uint8_t *public_key,
+                uint8_t *secret_key);
+
+  int (*sign)(uint8_t *sig, size_t *sig_len,


I think it would be good to have the order of parameters close the EVP API, pkey_nistdsa_sign_message and pkey_nistdsa_verify_message; that is
key (replacing ctx in those APIs)
signature
tbs
ctx

Changed in 221c533

the higher level EVP API:

int EVP_DigestSign(EVP_MD_CTX *ctx, uint8_t *out_sig, size_t *out_sig_len, const uint8_t *data, size_t data_len); int EVP_DigestVerify(EVP_MD_CTX *ctx, const uint8_t *sig, size_t sig_len, const uint8_t *data, size_t len);

now more closely matches the format of:

int ml_dsa_65_sign(const uint8_t *secret_key, uint8_t *sig, size_t *sig_len, const uint8_t *message, size_t message_len, const uint8_t *ctx, size_t ctx_len); int ml_dsa_65_verify(const uint8_t *public_key, const uint8_t *sig, size_t sig_len, const uint8_t *message, size_t message_len, const uint8_t *ctx, size_t ctx_len);

crypto/dilithium/internal.h

nebeid · 2024-11-06T19:53:35Z

crypto/dilithium/p_dilithium3.c

+//2.16.840.1.101.3.4.3.18
+static const uint8_t kOIDMLDSA65[]  = {0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x03, 0x12};
+
+// PQDSA functions: these are init/new/clear/free/get_sig functions for PQDSA_KEY


I think it would be better to move them in this PR for better clarity. I'm thinking that because if this PR is introducing a new architecture, then renaming and moving files is part of it.

I originally made the file move in this PR, but it made review difficult because all of the diffs no longer were comparing like for like changes. Andrew suggested making the file moves after the PR is merged to help review, but I can add back in the file changes if you'd prefer.

Essentially, those PQDSA functions will live in their own p_pqdsa.c file (within fipsmodule/evp/p_pqdsa.c) much like p_kem.c p_ec.c etc.

I think if you move the file with git mv it should keep the diff as expected?

Okay, now that we are deeper into review, I feel less apprehensive about file structure changes, as you are now more familiar with the review. I've made the file changes in 91056cb, note how github treats the re-names as new files, which is what I originally wanted to avoid. Nevertheless, the new file structure is as follows:

KEM PQDSA PQDSA NEW Location

crypto/fipsmodule/evp/p_kem.c crypto/dilithium/p_dilithium3.c crypto/dilithium/p_pqdsa.c

crypto/evp_extra/p_kem_asn1.c crypto/dilithium/p_dilithium3_asn1.c crypto/dilithium/p_pqdsa_asn1.c

crypto/fipsmodule/kem/kem.c crypto/dilithium/p_dilithium3.c crypto/dilithium/pqdsa.c

crypto/fipsmodule/ml_kem/ml_kem.c crypto/dilithium/sig_dilithium.c crypto/dilithium/mldsa.c

crypto/fipsmodule/ml_kem/ml_kem.h crypto/dilithium/sig_dilithium.h crypto/dilithium/mldsa.h

Then, when we eventually move to the fipsmodule, we'll have two directories:

crypto/fipsmodule/pqdsa/ which will contain generic pqdsa content, such as pqdsa.c

crypto/fipsmodule/ml_dsa/ which will contain ml_dsa specific content, such as mldsa.c mldsa.h

crypto/fipsmodule/evp can contain p_pqdsa.c

crypto/evp_extra can contain p_pqdsa_asn1.c

nebeid · 2024-11-06T20:18:15Z

crypto/dilithium/p_dilithium3.c


+const PQDSA *PQDSA_KEY_get0_sig(PQDSA_KEY* key) {


Suggested change

const PQDSA *PQDSA_KEY_get0_sig(PQDSA_KEY* key) {

const PQDSA *PQDSA_KEY_get0_dsa(PQDSA_KEY* key) {

fixed in e2234e3

nebeid · 2024-11-06T20:21:42Z

crypto/dilithium/p_dilithium3.c

+  out->method = sig_ml_dsa_65_method();
+}
+
+const PQDSA *PQDSA_find_dsa_by_nid(int nid) {


This function will need to move with the PQDSA_KEY_xx functions above.

Agreed! I put it down here for now, otherwise we get the warning/error sig_ml_dsa_65() defined but not used.

crypto/dilithium/internal.h

nebeid · 2024-11-06T20:28:54Z

crypto/dilithium/internal.h

+  int (*sign)(uint8_t *sig, size_t *sig_len,
+              const uint8_t *message,
+              size_t message_len,
+              const uint8_t *ctx,


Thanks. I was asking whether the EVP_PKEY API will be able to accommodate that extra parameter.

nebeid · 2024-11-06T20:30:17Z

crypto/dilithium/p_dilithium3.c

    return 0;
+      }


Suggested change

}

}

fixed in e2234e3

nebeid · 2024-11-06T20:34:12Z

crypto/dilithium/p_dilithium3.c

+      ctx->pkey->type != EVP_PKEY_NISTDSA) {
+    OPENSSL_PUT_ERROR(EVP, EVP_R_OPERATON_NOT_INITIALIZED);
+    return 0;
+      }


Suggested change

}

}

fixed in e2234e3

nebeid · 2024-11-06T20:37:07Z

crypto/dilithium/p_dilithium3.c

+    ctx->pkey->type != EVP_PKEY_NISTDSA) {
+    OPENSSL_PUT_ERROR(EVP, EVP_R_OPERATON_NOT_INITIALIZED);
+    return 0;
+    }


Suggested change

}

}

fixed in e2234e3

nebeid · 2024-11-06T20:37:20Z

crypto/dilithium/p_dilithium3.c

    OPENSSL_PUT_ERROR(EVP, EVP_R_INVALID_SIGNATURE);
+    return 0;
+      }


Suggested change

}

}

fixed in e2234e3

nebeid · 2024-11-06T20:40:42Z

continuing to review

nebeid · 2024-11-06T22:28:29Z

crypto/dilithium/p_dilithium3_asn1.c

-  dilithium3_free(pkey);
-  pkey->pkey.ptr = key;
+  // NOTE: No checks are done in this function, the caller has to ensure
+  //       that the pointers are valid and |in| has the correct size.


what's |in|?

Fixed comment in c72e3e6

nebeid · 2024-11-06T22:44:00Z

crypto/dilithium/p_dilithium3_asn1.c

+  //       that the pointers are valid and |in| has the correct size.
+  key->public_key = NULL;
+  key->secret_key = OPENSSL_memdup(privkey, privkey_len);
+  pqdsa_free(pkey);


Won't this free the memory? How is there an assignment afterwards? Maybe you want to use OPENSSL_cleanse?

My understanding is that the PKEY coming into the function is not allocated, it's a fresh PKEY for which we are setting the private key. I'm following the implementation of ed25519_set_priv_raw in https://github.com/jakemas/aws-lc/blob/3662fc13a4c4bccadb40a55218d4e71a491f1e5b/crypto/evp_extra/p_ed25519_asn1.c#L63 which also free's the PKEY before assignment.

Ah, I see. It's because it's the member pointer that's being freed. Thanks for the pointer.

nebeid · 2024-11-06T22:46:28Z

crypto/dilithium/p_dilithium3_asn1.c


-  dilithium3_free(pkey);
-  pkey->pkey.ptr = key;
+  pqdsa_free(pkey);


same comment about free and assignment afterwards.

Resolved above.

crypto/dilithium/p_dilithium3_asn1.c

include/openssl/evp.h

crypto/dilithium/p_dilithium_test.cc

dkostic · 2024-11-08T14:53:46Z

crypto/dilithium/internal.h

+  int (*sign)(uint8_t *sig, size_t *sig_len,
+              const uint8_t *message,
+              size_t message_len,
+              const uint8_t *ctx,


please document that in the code. btw, ctx is very unfortunate name, can we change it to ctx_string or something like that?

crypto/dilithium/ml_dsa.h

dkostic · 2024-11-08T14:56:21Z

crypto/dilithium/ml_dsa.h

+#define MLDSA65_SIGNATURE_SEED_BYTES 32
+
+// ml_dsa_65_keypair generates an ML-DSA-65 keypair and assigns a public key to
+// |public_key| and a private key to |secret_key|. It returns 0 upon success.


It returns 0 upon success.

Can it fail?

it can return -1 when prefix string too long, added comment to indicate this bcbb832

crypto/dilithium/ml_dsa.h

dkostic · 2024-11-08T15:04:51Z

crypto/dilithium/p_dilithium3.c

+}
+
+static int pkey_nistdsa_sign_message(EVP_PKEY_CTX *ctx, uint8_t *sig,
+                                        size_t *siglen, const uint8_t *tbs,


since the function is called sign_message maybe it's better to rename tbs to message

crypto/fipsmodule/evp/evp.c

crypto/dilithium/p_pqdsa.c

dkostic · 2024-11-08T15:18:54Z

crypto/dilithium/p_pqdsa_test.cc

+#include "../rand_extra/pq_custom_randombytes.h"
+#include "ml_dsa.h"
+
+// mldsa65kPublicKey is an example ML-DSA-65 public key


how did you generate it?

// given an ML-DSA PKEY with a public key present: // extract public key std::vector<uint8_t> pub_buf(pk_len); size_t pub_len; // get size EVP_PKEY_get_raw_public_key(pkey.get(), nullptr, &pub_len); // fill buffer with pk EVP_PKEY_get_raw_public_key(pkey.get(), pub_buf.data(), &pub_len); // encode public key as DER bssl::ScopedCBB cbb; uint8_t *der; size_t der_len; CBB_init(cbb.get(), 0); EVP_marshal_public_key(cbb.get(), pkey_pk_new.get()); CBB_finish(cbb.get(), &der, &der_len);

we usually get test examples from "independent" implementations instead of our own. Are you aware of another source you can take the test value from?

Will be pulling examples from an independent implementation at https://github.com/lamps-wg/dilithium-certificates/pull/51/files#diff-d2a836a5afde5fbfd8925500dc1e25dfc63f78bc4749ff36c62697e3a0c0355e

dkostic · 2024-11-12T18:42:18Z

crypto/CMakeLists.txt

-    dilithium/p_dilithium3_asn1.c
-    dilithium/sig_dilithium3.c
+    dilithium/pqdsa.c
+          dilithium/p_pqdsa.c


fixed in 7435f9b

dkostic · 2024-11-12T18:42:45Z

crypto/CMakeLists.txt

@@ -774,7 +775,7 @@ if(BUILD_TESTING)
    ecdh_extra/ecdh_test.cc
    dh_extra/dh_test.cc
    digest_extra/digest_test.cc
-    dilithium/p_dilithium_test.cc
+          dilithium/p_pqdsa_test.cc


fixed in 7435f9b

dkostic · 2024-11-12T18:43:08Z

crypto/dilithium/internal.h

+// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0 OR ISC
+
+#ifndef AWSLC_HEADER_SIG_INTERNAL_H


Suggested change

#ifndef AWSLC_HEADER_SIG_INTERNAL_H

#ifndef AWSLC_HEADER_PQDSA_INTERNAL_H

fixed in 7435f9b

dkostic · 2024-11-12T18:45:05Z

crypto/dilithium/ml_dsa.c

+                   size_t *sig_len            /* OUT */,
+                   const uint8_t *message     /* IN */,
+                   size_t message_len         /* IN */,
+                   const uint8_t *pre         /* IN */,


what's pre?

dkostic · 2024-11-12T18:46:52Z

crypto/dilithium/ml_dsa.c

  ml_dsa_params params;
  ml_dsa_65_params_init(&params);
-  return crypto_sign_keypair(&params, public_key, secret_key);
+  return (crypto_sign_keypair(&params, public_key, secret_key) == 0);


how did this work so far? aren't we checking the return value? because this change inverts the return value

crypto_sign_keypair returns 0 on success -1 for failure. This is an anti-pattern with how EVP expects 1 for success. The return values are checked within the PKEY functions:

keygen: https://github.com/jakemas/aws-lc/blob/7435f9b2920753efc71a6461c55be213dc24da65/crypto/dilithium/p_pqdsa.c#L52 prior to this change we check: ml_dsa_65_keypair(key->pub, key->priv) != 0. For more consistency, we do this check at the lowest level API, and instead within PKEY keygen we check: !pqdsa->method->keygen(key->public_key, key->secret_key)

dkostic · 2024-11-12T18:50:29Z

crypto/dilithium/p_pqdsa.c

+}
+
+static int pkey_pqdsa_sign_signature(EVP_PKEY_CTX *ctx, uint8_t *sig,
+                                     size_t *siglen, const uint8_t *message,


Suggested change

size_t *siglen, const uint8_t *message,

size_t *sig_len, const uint8_t *message,

fixed in 7435f9b

dkostic · 2024-11-12T18:50:55Z

crypto/dilithium/p_pqdsa.c

+}
+
+static int pkey_pqdsa_verify_signature(EVP_PKEY_CTX *ctx, const uint8_t *sig,
+                                       size_t siglen, const uint8_t *message,


Suggested change

size_t siglen, const uint8_t *message,

size_t sig_len, const uint8_t *message,

fixed in 7435f9b

dkostic · 2024-11-12T18:52:16Z

crypto/dilithium/p_pqdsa_asn1.c

+    return 0;
+  }
+
+  // NOTE: No checks are done in this function, the caller has to ensure


The functions pqdsa_set_priv_raw and pqdsa_set_pub_raw are required by pqdsa_priv_decode and pqdsa_pub_decode respectively. These functions are used by EVP_parse_public_key for example.

These functions are not intended for setting a PKEY from raw bytes, due to the fact that we can't allocate memory for the key as we don't know it's size. We have the same issue in EC KEY and KEM KEY, as the top level NID (EVP_PKEY_KEM for example, doesn't specify algorithm specific parameter sizes). Instead, should we want to populate a PKEY with raw priv/pub, we use:

OPENSSL_EXPORT EVP_PKEY *EVP_PKEY_pqdsa_new_raw_public_key(int nid, const uint8_t *in, size_t len); OPENSSL_EXPORT EVP_PKEY *EVP_PKEY_pqdsa_new_raw_secret_key(int nid, const uint8_t *in, size_t len);

As the inclusion of the NID allows us to set the correct parameter level, and prevent PKEYs that have differing key sizes than expected.

We don't have information as to what key sizes are expected when calling pqdsa_set_priv/pub_raw, so cannot perform the checks.

dkostic · 2024-11-12T18:52:23Z

crypto/dilithium/p_pqdsa_asn1.c

+    return 0;
+  }
+
+  // NOTE: No checks are done in this function, the caller has to ensure


See #1963 (comment)

crypto/evp_extra/evp_extra_test.cc

nebeid · 2024-11-12T23:54:50Z

crypto/dilithium/p_dilithium3_asn1.c

+  //       that the pointers are valid and |in| has the correct size.
+  key->public_key = NULL;
+  key->secret_key = OPENSSL_memdup(privkey, privkey_len);
+  pqdsa_free(pkey);


Ah, I see. It's because it's the member pointer that's being freed. Thanks for the pointer.

nebeid · 2024-11-12T23:55:09Z

crypto/dilithium/p_dilithium3_asn1.c


-  dilithium3_free(pkey);
-  pkey->pkey.ptr = key;
+  pqdsa_free(pkey);


Resolved above.

nebeid · 2024-11-13T19:34:07Z

crypto/dilithium/pqdsa.c

+  out->oid_len = sizeof(kOIDMLDSA65);
+  out->comment = "MLDSA65 ";
+  out->public_key_len = MLDSA65_PUBLIC_KEY_BYTES;
+  out->secret_key_len = MLDSA65_PRIVATE_KEY_BYTES;


Why we're using both "secret" and "private" to designate the same thing? Why not call it "private" everywhere?

changed all occurrences to private in 739a3be

nebeid · 2024-11-13T20:08:50Z

crypto/x509/x509_test.cc


  bssl::UniquePtr<X509> leaf =
-      MakeTestCert("Intermediate", "Leaf", pkey, /*is_ca=*/false);
+      MakeTestCert("Intermediate", "Leaf", pkey.get(), false);


Suggested change

MakeTestCert("Intermediate", "Leaf", pkey.get(), false);

MakeTestCert("Intermediate", "Leaf", pkey.get(), /*is_ca=*/ false);

fixed in ad0a24c

nebeid · 2024-11-13T20:21:27Z

crypto/x509/x509_test.cc

  ASSERT_FALSE(X509_verify(cert.get(), pkey.get()));
  uint32_t err = ERR_get_error();
-  ASSERT_EQ(ERR_LIB_X509, ERR_GET_LIB(err));
-  ASSERT_EQ(X509_R_INVALID_PARAMETER, ERR_GET_REASON(err));
+  ASSERT_EQ(ERR_LIB_ASN1, ERR_GET_LIB(err));


Why was the reason check omitted?

Added back reason check (and moved these tests back to be implemented how they were before) in 739a3be

nebeid · 2024-11-13T22:43:00Z

crypto/dilithium/p_pqdsa_test.cc

+  ASSERT_NE(private_pkey, nullptr);
+  EXPECT_EQ(private_pkey->pkey.pqdsa_key->public_key, nullptr);
+  EXPECT_NE(private_pkey->pkey.pqdsa_key->secret_key, nullptr);
+  EXPECT_EQ(0, OPENSSL_memcmp(private_pkey->pkey.pqdsa_key->secret_key, pkey->pkey.pqdsa_key->secret_key, sk_len));


~~Similarly, would you want to test EVP_PKEY_get_raw_private_key?~~ I saw later that it has its own test.

nebeid · 2024-11-13T22:55:23Z

crypto/dilithium/p_pqdsa_test.cc

+  ASSERT_TRUE(CBB_init(cbb.get(), 0));
+  ASSERT_TRUE(EVP_marshal_private_key(cbb.get(), pkey_sk_new.get()));
+  ASSERT_TRUE(CBB_finish(cbb.get(), &der, &der_len));
+  free_der.reset(der);


Maybe I should know the answer, but what is free_der used for above and here?

nebeid · 2024-11-13T22:56:05Z

crypto/dilithium/p_pqdsa_test.cc

+  std::vector<uint8_t> msg = {
+      0x4a, 0x41, 0x4b, 0x45, 0x20, 0x4d, 0x41, 0x53, 0x53, 0x49,
+      0x4d, 0x4f, 0x20, 0x41, 0x57, 0x53, 0x32, 0x30, 0x32, 0x32, 0x2e};
+  std::vector<uint8_t> badmsg = {


What makes it a bad msg? I suggest you name them msg1 and msg2 that differ in one byte.

changed in 2e5d891

nebeid · 2024-11-13T22:57:20Z

crypto/dilithium/p_pqdsa_test.cc

+  // Verify the signed message fails upon a bad signature
+  ASSERT_FALSE(EVP_DigestVerify(md_ctx.get(), signature1.data(), sig_len,
+                                msg.data(), msg.size()));
+  md_ctx.Reset();


Did you need to reset before signing a bad msg?

reset and re-init added in 739a3be

nebeid · 2024-11-13T22:58:35Z

crypto/dilithium/p_pqdsa_test.cc

+
+#else
+
+TEST(Dilithium3Test, EvpDisabled) {


Suggested change

TEST(Dilithium3Test, EvpDisabled) {

TEST(PQDSATest, EvpDisabled) {

fixed in 2e5d891

nebeid · 2024-11-15T16:54:09Z

crypto/evp_extra/evp_asn1.c

@@ -91,18 +91,27 @@ static const EVP_PKEY_ASN1_METHOD *parse_key_type(CBS *cbs) {
  if (OBJ_cbs2nid(&oid) == NID_rsa) {
    return &rsa_asn1_meth;
  }
+
+#ifdef ENABLE_DILITHIUM
+  // if |cbs| is empty we overwrite the contents with |oid| so that we can


Nit:

Suggested change

// if |cbs| is empty we overwrite the contents with |oid| so that we can

// if |cbs| is empty (after parsing |oid| from it), we overwrite the contents with |oid| so that we can

(line length will need adjustment)

fixed in c71bbac

nebeid · 2024-11-15T16:55:41Z

crypto/evp_extra/evp_asn1.c

+  // call pub_decode(ret, &algorithm, &key) with the |algorithm| populated as |oid|.
+  // We could probably use CBS_peek_asn1_tag for this, to conditionally set |algorithm|
+  // based on if peeking at the next tag to see if there is params.
+  if (CBS_len(cbs) == 0) {


Was this not needed for the private key?

fixed my comment to call out its use for private keys in c71bbac

nebeid · 2024-11-15T19:01:06Z

crypto/evp_extra/evp_asn1.c

+// as |cbs|. (This allows the specific OID, e.g. NID_MLDSA65 to be parsed to
+// type specific decoding functions within the algorithm parameter.


// as |cbs|. (This allows the specific OID, e.g. NID_MLDSA65 to be parsed by
// the type-specific decoding functions within the algorithm parameter.)

added in e419b99

nebeid · 2024-11-15T19:02:02Z

crypto/evp_extra/evp_asn1.c

@@ -70,6 +70,14 @@
 #include "internal.h"
 #include "../dilithium/internal.h"

+// The function parse_key_type takes the algorithm cbs sequence |cbs| and


Suggested change

// The function parse_key_type takes the algorithm cbs sequence |cbs| and

// parse_key_type takes the algorithm cbs sequence |cbs| and

added in e419b99

nebeid · 2024-11-15T19:02:39Z

crypto/evp_extra/evp_asn1.c

-  // call pub_decode(ret, &algorithm, &key) with the |algorithm| populated as |oid|.
-  // We could probably use CBS_peek_asn1_tag for this, to conditionally set |algorithm|
-  // based on if peeking at the next tag to see if there is params.
+  // if |cbs| is empty after parsing |oid| from it), we overwrite the contents


Suggested change

// if |cbs| is empty after parsing |oid| from it), we overwrite the contents

// if |cbs| is empty after parsing |oid| from it, we overwrite the contents

added in e419b99

dkostic · 2024-11-15T22:11:04Z

crypto/dilithium/p_pqdsa_asn1.c

+    return 0;
+  }
+  // set the pqdsa params on the fresh pkey
+  EVP_PKEY_pqdsa_set_params(out, OBJ_cbs2nid(params));


check the return value?

woops! checked in fb8631e

dkostic · 2024-11-15T22:11:22Z

crypto/dilithium/p_pqdsa_asn1.c

+    return 0;
+  }
+  // set the pqdsa params on the fresh pkey
+  EVP_PKEY_pqdsa_set_params(out, OBJ_cbs2nid(params));


check the return value?

woops! checked in 369080b

dkostic · 2024-11-15T22:11:41Z

crypto/dilithium/p_pqdsa_asn1.c

+  // The parameters must be omitted.
+
+  // the only parameter that can be included is the OID which has length 9


these two statement are contradicting :)

fixed in 369080b

dkostic · 2024-11-15T22:12:51Z

crypto/dilithium/p_pqdsa_asn1.c

+  // The parameters must be omitted.
+
+  // the only parameter that can be included is the OID which has length 9


these two statement are contradicting :)

fixed in 369080b

dkostic · 2024-11-15T22:37:48Z

crypto/evp_extra/evp_asn1.c

@@ -91,6 +100,19 @@ static const EVP_PKEY_ASN1_METHOD *parse_key_type(CBS *cbs) {
    return &rsa_asn1_meth;
  }

+#ifdef ENABLE_DILITHIUM
+  // if |cbs| is empty after parsing |oid| from it, we overwrite the contents


we should do this only if the oid corresponds to one of the pqdsa algorithms. In other words,

if (PQDSA_find_asn1_by_nid(OBJ_cbs2nid(&oid)) != NULL) { if (CBS_len(cbs) == 0) { ... }

Thanks, you're right 369080b

dkostic · 2024-11-15T22:41:00Z

crypto/x509/algorithm.c

+  if (EVP_PKEY_id(pkey) == EVP_PKEY_PQDSA) {
+    return X509_ALGOR_set0(algor, OBJ_nid2obj(NID_MLDSA65), V_ASN1_UNDEF, NULL);


how is this supposed to work when you add other PQDSA algorithms?

Thank you, found an elegant solution in 369080b. The pqdsa struct will hold information on the specific PKEY type. We could have used EVP_PKEY_base_id for this, if we didn't disable it in AWS-LC. Nevertheless, if the PKEY type is EVP_PKEY_PQDSA as in this conditional, we can get the actual NID from pkey->pkey.pqdsa_key->pqdsa->nid.

Generic NIST-DSA PKEY and ASN1 to support ML-DSA

6179241

jakemas marked this pull request as ready for review November 4, 2024 20:37

jakemas requested a review from a team as a code owner November 4, 2024 20:37

jakemas requested review from dkostic and nebeid November 4, 2024 20:37

jakemas mentioned this pull request Nov 4, 2024

Adding NISTDSA API to support ML-DSA-44 and ML-DSA-87 #1949

Closed

dkostic reviewed Nov 6, 2024

View reviewed changes

nebeid reviewed Nov 6, 2024

View reviewed changes

renamed NISTDSA to PQDSA

8542649

nebeid reviewed Nov 6, 2024

View reviewed changes

jakemas added 4 commits November 6, 2024 13:35

CR fixes

e2234e3

removed accidental code

163b50d

api argument consistency

221c533

moved evp_pkey_pqdsa_set_params

e47ef83

nebeid reviewed Nov 6, 2024

View reviewed changes

fixed comment

c72e3e6

dkostic reviewed Nov 7, 2024

View reviewed changes

jakemas added 4 commits November 7, 2024 08:22

CR fixes, name changes

7e524df

added PQDSA assign

8d6ff48

file structure change

91056cb

removed EVP_PKEY_nistdsa_set_params, implemented differently

c0d4e65

dkostic reviewed Nov 8, 2024

View reviewed changes

CR fixes

bcbb832

dkostic reviewed Nov 12, 2024

View reviewed changes

CR fixes

7435f9b

nebeid reviewed Nov 13, 2024

View reviewed changes

removed pqdsa_set_pub/priv_raw

297f76b

jakemas added 8 commits November 13, 2024 15:57

added dilithium ifdef

5a744cf

CR fixes

2e5d891

CR fixes

ad0a24c

CR fixes

739a3be

CR fixes

a1a15fe

documentation and names

c4afe50

cleaned up search for asn1 method by nid

2622a60

edits to evp_asn1.c

2c0d95a

nebeid reviewed Nov 15, 2024

View reviewed changes

added external certificate from standard

c71bbac

nebeid reviewed Nov 15, 2024

View reviewed changes

jakemas added 2 commits November 15, 2024 11:19

documentation nit

13b5886

documentation fix

e419b99

nebeid previously approved these changes Nov 15, 2024

View reviewed changes

alignment fix, re-run ci

53522b6

jakemas dismissed nebeid’s stale review via 53522b6 November 15, 2024 21:49

dkostic reviewed Nov 15, 2024

View reviewed changes

jakemas added 2 commits November 15, 2024 15:52

check values, documentation, x509 oid

369080b

check return value

fb8631e

nebeid approved these changes Nov 18, 2024

View reviewed changes

dkostic approved these changes Nov 18, 2024

View reviewed changes

dkostic merged commit 404fe0f into aws:main Nov 18, 2024
113 of 117 checks passed

KEM	PQDSA	PQDSA NEW Location
crypto/fipsmodule/evp/p_kem.c	crypto/dilithium/p_dilithium3.c	crypto/dilithium/p_pqdsa.c
crypto/evp_extra/p_kem_asn1.c	crypto/dilithium/p_dilithium3_asn1.c	crypto/dilithium/p_pqdsa_asn1.c
crypto/fipsmodule/kem/kem.c	crypto/dilithium/p_dilithium3.c	crypto/dilithium/pqdsa.c
crypto/fipsmodule/ml_kem/ml_kem.c	crypto/dilithium/sig_dilithium.c	crypto/dilithium/mldsa.c
crypto/fipsmodule/ml_kem/ml_kem.h	crypto/dilithium/sig_dilithium.h	crypto/dilithium/mldsa.h

	const PQDSA PQDSA_KEY_get0_sig(PQDSA_KEY key) {
	const PQDSA PQDSA_KEY_get0_dsa(PQDSA_KEY key) {

	#ifndef AWSLC_HEADER_SIG_INTERNAL_H
	#ifndef AWSLC_HEADER_PQDSA_INTERNAL_H

	size_t siglen, const uint8_t message,
	size_t sig_len, const uint8_t message,

	size_t siglen, const uint8_t *message,
	size_t sig_len, const uint8_t *message,

	MakeTestCert("Intermediate", "Leaf", pkey.get(), false);
	MakeTestCert("Intermediate", "Leaf", pkey.get(), /is_ca=/ false);

	TEST(Dilithium3Test, EvpDisabled) {
	TEST(PQDSATest, EvpDisabled) {

	// if \|cbs\| is empty we overwrite the contents with \|oid\| so that we can
	// if \|cbs\| is empty (after parsing \|oid\| from it), we overwrite the contents with \|oid\| so that we can

		// as \|cbs\|. (This allows the specific OID, e.g. NID_MLDSA65 to be parsed to
		// type specific decoding functions within the algorithm parameter.

	// The function parse_key_type takes the algorithm cbs sequence \|cbs\| and
	// parse_key_type takes the algorithm cbs sequence \|cbs\| and

	// if \|cbs\| is empty after parsing \|oid\| from it), we overwrite the contents
	// if \|cbs\| is empty after parsing \|oid\| from it, we overwrite the contents

		// The parameters must be omitted.

		// the only parameter that can be included is the OID which has length 9

		if (EVP_PKEY_id(pkey) == EVP_PKEY_PQDSA) {
		return X509_ALGOR_set0(algor, OBJ_nid2obj(NID_MLDSA65), V_ASN1_UNDEF, NULL);

Addition of generic NIST-DSA PKEY and ASN1 to support ML-DSA #1963

Addition of generic NIST-DSA PKEY and ASN1 to support ML-DSA #1963

Conversation

jakemas commented Nov 4, 2024 • edited Loading

Issues:

Description of changes:

1. PKEY structure changes

2. Internal Design

3. OID changes

4. Testing structure changes

5. X.509 changes

6. Speed Tool

Call-outs:

Testing:

codecov-commenter commented Nov 4, 2024 • edited Loading

Codecov Report

dkostic left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakemas Nov 6, 2024 • edited Loading

Choose a reason for hiding this comment

jakemas Nov 7, 2024 • edited Loading

Choose a reason for hiding this comment

nebeid left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakemas Nov 6, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nebeid commented Nov 6, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakemas commented Nov 4, 2024 •

edited

Loading

codecov-commenter commented Nov 4, 2024 •

edited

Loading

jakemas Nov 6, 2024 •

edited

Loading

jakemas Nov 7, 2024 •

edited

Loading

jakemas Nov 6, 2024 •

edited

Loading