Skip to content

Commit

Permalink
ml-dsa extmu
Browse files Browse the repository at this point in the history
  • Loading branch information
@jakemas
jakemas committed Jan 10, 2025
1 parent 5084e19 commit 1f2f577
Show file tree
Hide file tree
Showing 9 changed files with 360 additions and 50 deletions.
75 changes: 75 additions & 0 deletions crypto/evp_extra/p_pqdsa_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

#include <vector>
#include "../fipsmodule/evp/internal.h"
#include "../fipsmodule/sha/internal.h"
#include "../internal.h"
#include "../fipsmodule/pqdsa/internal.h"

Expand Down Expand Up @@ -1519,6 +1520,80 @@ TEST_P(PQDSAParameterTest, ParsePublicKey) {
ASSERT_TRUE(pkey_from_der);
}

// ML-DSA specific test framework to test pre-hash modes only applicable to ML-DSA

This comment has been minimized.

Sign in to view

Copy link
@nebeid

nebeid Jan 10, 2025

Are there test vectors we can use for external mu? Or we'll leave this for later?

This comment has been minimized.

Sign in to view

Copy link
@jakemas

jakemas Jan 10, 2025

Author Owner

We're still waiting on NIST and ACVP for this. However, since External-mu is the same algorithm as ML-DSA - we already KAT the core algorithm.

When true test vectors exists I will include them -- it's just too early.

This comment has been minimized.

Sign in to view

Copy link
@nebeid

nebeid Jan 11, 2025

Can we include tests with those test vectors, so that we exercise the new APIs with test vectors?

This comment has been minimized.

Sign in to view

Copy link
@jakemas

jakemas Jan 11, 2025

Author Owner

Yes
struct KnownMLDSA {
const char name[20];
const int nid;
const size_t public_key_len;
const size_t private_key_len;
const size_t signature_len;
};

//todo remove and use the one above
static const struct KnownMLDSA kMLDSAs[] = {
{"MLDSA44", NID_MLDSA44, 1312, 2560, 2420},
{"MLDSA65", NID_MLDSA65, 1952, 4032, 3309},
{"MLDSA87", NID_MLDSA87, 2592, 4896, 4627},
};

class PerMLDSATest : public testing::TestWithParam<KnownMLDSA> {};

INSTANTIATE_TEST_SUITE_P(All, PerMLDSATest, testing::ValuesIn(kMLDSAs),
[](const testing::TestParamInfo<KnownMLDSA> &params)
-> std::string { return params.param.name; });

TEST_P(PerMLDSATest, ExternalMu) {
// ---- 1. Setup phase: generate PQDSA EVP KEY and sign/verify contexts ----
bssl::UniquePtr<EVP_PKEY> pkey(generate_key_pair(GetParam().nid));
EVP_PKEY_CTX *ctx = EVP_PKEY_CTX_new(pkey.get(), nullptr);
bssl::ScopedEVP_MD_CTX md_ctx, md_ctx_verify, md_ctx_verify1;

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

// ----2. Pre-hash setup phase: compute tr, mu ----
size_t TRBYTES = 64;
size_t CRHBYTES = 64;
size_t pk_len = GetParam().public_key_len;

std::vector<uint8_t> pk(pk_len);
std::vector<uint8_t> tr(TRBYTES);
std::vector<uint8_t> mu(TRBYTES);

uint8_t pre[2];
pre[0] = 0;
pre[1] = 0;

//get public key and hash it
ASSERT_TRUE(EVP_PKEY_get_raw_public_key(pkey.get(), pk.data(), &pk_len));
SHAKE256(pk.data(), pk.size(), tr.data(), TRBYTES);

//compute mu
KECCAK1600_CTX state;
SHAKE_Init(&state, SHAKE256_BLOCKSIZE);
SHA3_Update(&state, tr.data(), TRBYTES);
SHA3_Update(&state, pre, 2);
SHA3_Update(&state, msg1.data(), msg1.size());
SHAKE_Final(mu.data(), &state, CRHBYTES);

// ----2. Init signing, get signature size and allocate signature buffer ----
size_t sig_len = GetParam().signature_len;
std::vector<uint8_t> sig1(sig_len);

// ----3. Sign mu ----
ASSERT_TRUE(EVP_PKEY_sign_init(ctx));
ASSERT_TRUE(EVP_PKEY_sign(ctx, sig1.data(), &sig_len, mu.data(), mu.size()));

// ----4. Verify mu (pre-hash) ----
ASSERT_TRUE(EVP_PKEY_verify_init(ctx));
ASSERT_TRUE(EVP_PKEY_verify(ctx, sig1.data(), sig_len, mu.data(), mu.size()));

// ----5. Bonus: Verify raw message with digest verify (no pre-hash) ----
ASSERT_TRUE(EVP_DigestVerifyInit(md_ctx_verify.get(), nullptr, nullptr, nullptr, pkey.get()));
ASSERT_TRUE(EVP_DigestVerify(md_ctx_verify.get(), sig1.data(), sig_len, msg1.data(), msg1.size()));
}

#else

TEST(PQDSATest, EvpDisabled) {
Expand Down
6 changes: 3 additions & 3 deletions crypto/fipsmodule/evp/digestsign.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ static int do_sigver_init(EVP_MD_CTX *ctx, EVP_PKEY_CTX **pctx,
return 0;
}

if (uses_prehash(ctx, op) || used_for_hmac(ctx)) {
if ((uses_prehash(ctx, op) || used_for_hmac(ctx)) && pkey->type != EVP_PKEY_PQDSA) {

This comment has been minimized.

Sign in to view

Copy link
@nebeid

nebeid Jan 10, 2025

Do you think this extra check is clearer if it goes into uses_prehash itself?

This comment has been minimized.

Sign in to view

Copy link
@jakemas

jakemas Jan 10, 2025

Author Owner

everything about this line of code is pretty hacky from the start... but yes you are right, I think it makes more sense to put it into the uses_prehash check.

This comment has been minimized.

Sign in to view

Copy link
@nebeid

nebeid Jan 12, 2025

I was thinking again about this check (including the case of moving it within uses_prehash); it excludes all EVP_PKEY_PQDSA key types. So support for HashML-DSA and other new NIST DSA will be excluded by it. Is it possible to narrow it down to NID_MLDSA<44|65|87?

This comment has been minimized.

Sign in to view

Copy link
@jakemas

jakemas Jan 13, 2025 •
edited
Loading

Author Owner

I've been thinking about this too. Yes, we can get the PKEY NID from ctx->pctx->pkey->pkey->pqdsa_key->pqdsa->nid and check for ML-DSA explicitly (and hide in uses_prehash).
if (type == NULL) {
OPENSSL_PUT_ERROR(EVP, EVP_R_NO_DEFAULT_DIGEST);
return 0;
Expand Down Expand Up @@ -271,7 +271,7 @@ int EVP_DigestSign(EVP_MD_CTX *ctx, uint8_t *out_sig, size_t *out_sig_len,
SET_DIT_AUTO_RESET;
int ret = 0;

if (uses_prehash(ctx, evp_sign) || used_for_hmac(ctx)) {
if ((uses_prehash(ctx, evp_sign) || used_for_hmac(ctx)) && ctx->pctx->pkey->type != EVP_PKEY_PQDSA) {
// If |out_sig| is NULL, the caller is only querying the maximum output
// length. |data| should only be incorporated in the final call.
if (out_sig != NULL && !EVP_DigestSignUpdate(ctx, data, data_len)) {
Expand Down Expand Up @@ -310,7 +310,7 @@ int EVP_DigestVerify(EVP_MD_CTX *ctx, const uint8_t *sig, size_t sig_len,
SET_DIT_AUTO_RESET;
int ret = 0;

if (uses_prehash(ctx, evp_verify) && !used_for_hmac(ctx)) {
if ((uses_prehash(ctx, evp_verify) && !used_for_hmac(ctx)) && ctx->pctx->pkey->type != EVP_PKEY_PQDSA) {
ret = EVP_DigestVerifyUpdate(ctx, data, len) &&
EVP_DigestVerifyFinal(ctx, sig, sig_len);
goto end;
Expand Down
87 changes: 83 additions & 4 deletions crypto/fipsmodule/evp/p_pqdsa.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,53 @@ static int pkey_pqdsa_sign_message(EVP_PKEY_CTX *ctx, uint8_t *sig,
return 0;
}

if (!pqdsa->method->pqdsa_sign(key->private_key, sig, sig_len, message, message_len, NULL, 0)) {
if (!pqdsa->method->pqdsa_sign_message(key->private_key, sig, sig_len, message, message_len, NULL, 0)) {

This comment has been minimized.

Sign in to view

Copy link
@nebeid

nebeid Jan 11, 2025

Since sign_message/sign and verify_message/verify have exactly the same code instead of the final call we can make each pair wrap around another static function that takes as an extra parameter a flag external_mu.

This comment has been minimized.

Sign in to view

Copy link
@jakemas

jakemas Jan 13, 2025 •
edited
Loading

Author Owner

I thought of this too when I was writing this, but on second thought wanted to give it a unique function for clarity/separation between the methods. Happy to do it!

This comment has been minimized.

Sign in to view

Copy link
@nebeid

nebeid Jan 13, 2025

I think the wrapper functions will be distinct enough. I think code is more maintainable if not repeated, especially when it's exactly the same.
OPENSSL_PUT_ERROR(EVP, ERR_R_INTERNAL_ERROR);
return 0;
}
return 1;
}

static int pkey_pqdsa_sign(EVP_PKEY_CTX *ctx, uint8_t *sig, size_t *sig_len,
const uint8_t *message, size_t message_len) {
PQDSA_PKEY_CTX *dctx = ctx->data;
const PQDSA *pqdsa = dctx->pqdsa;
if (pqdsa == NULL) {
if (ctx->pkey == NULL) {
OPENSSL_PUT_ERROR(EVP, EVP_R_NO_PARAMETERS_SET);
return 0;
}
pqdsa = PQDSA_KEY_get0_dsa(ctx->pkey->pkey.pqdsa_key);
}

// Caller is getting parameter values.
if (sig == NULL) {
if (sig_len != NULL) {
*sig_len = pqdsa->signature_len;
return 1;
}
}

if (*sig_len != pqdsa->signature_len) {
OPENSSL_PUT_ERROR(EVP, EVP_R_BUFFER_TOO_SMALL);
return 0;
}

// Check that the context is properly configured.
if (ctx->pkey == NULL ||
ctx->pkey->pkey.pqdsa_key == NULL ||
ctx->pkey->type != EVP_PKEY_PQDSA) {
OPENSSL_PUT_ERROR(EVP, EVP_R_OPERATON_NOT_INITIALIZED);
return 0;
}

PQDSA_KEY *key = ctx->pkey->pkey.pqdsa_key;
if (!key->private_key) {
OPENSSL_PUT_ERROR(EVP, EVP_R_NO_KEY_SET);
return 0;
}

if (!pqdsa->method->pqdsa_sign(key->private_key, sig, sig_len, message, message_len)) {
OPENSSL_PUT_ERROR(EVP, ERR_R_INTERNAL_ERROR);
return 0;
}
Expand Down Expand Up @@ -129,14 +175,47 @@ static int pkey_pqdsa_verify_signature(EVP_PKEY_CTX *ctx, const uint8_t *sig,
PQDSA_KEY *key = ctx->pkey->pkey.pqdsa_key;

if (sig_len != pqdsa->signature_len ||
!pqdsa->method->pqdsa_verify(key->public_key, sig, sig_len, message, message_len, NULL, 0)) {
!pqdsa->method->pqdsa_verify_message(key->public_key, sig, sig_len, message, message_len, NULL, 0)) {
OPENSSL_PUT_ERROR(EVP, EVP_R_INVALID_SIGNATURE);
return 0;
}

return 1;
}

static int pkey_pqdsa_verify(EVP_PKEY_CTX *ctx, const uint8_t *sig,
size_t sig_len, const uint8_t *message,
size_t message_len) {
PQDSA_PKEY_CTX *dctx = ctx->data;
const PQDSA *pqdsa = dctx->pqdsa;

if (pqdsa == NULL) {
if (ctx->pkey == NULL) {
OPENSSL_PUT_ERROR(EVP, EVP_R_NO_PARAMETERS_SET);
return 0;
}

pqdsa = PQDSA_KEY_get0_dsa(ctx->pkey->pkey.pqdsa_key);
}
// Check that the context is properly configured.
if (ctx->pkey == NULL ||
ctx->pkey->pkey.pqdsa_key == NULL ||
ctx->pkey->type != EVP_PKEY_PQDSA) {
OPENSSL_PUT_ERROR(EVP, EVP_R_OPERATON_NOT_INITIALIZED);
return 0;
}

PQDSA_KEY *key = ctx->pkey->pkey.pqdsa_key;

if (sig_len != pqdsa->signature_len ||
!pqdsa->method->pqdsa_verify(key->public_key, sig, sig_len, message, message_len)) {
OPENSSL_PUT_ERROR(EVP, EVP_R_INVALID_SIGNATURE);
return 0;
}

return 1;
}

// Additional PQDSA specific EVP functions.

// This function sets pqdsa parameters defined by |nid| in |pkey|.
Expand Down Expand Up @@ -268,10 +347,10 @@ DEFINE_METHOD_FUNCTION(EVP_PKEY_METHOD, EVP_PKEY_pqdsa_pkey_meth) {
out->cleanup = pkey_pqdsa_cleanup;
out->keygen = pkey_pqdsa_keygen;
out->sign_init = NULL;
out->sign = NULL;
out->sign = pkey_pqdsa_sign;
out->sign_message = pkey_pqdsa_sign_message;
out->verify_init = NULL;
out->verify = NULL;
out->verify = pkey_pqdsa_verify;
out->verify_message = pkey_pqdsa_verify_signature;
out->verify_recover = NULL;
out->encrypt = NULL;
Expand Down
72 changes: 66 additions & 6 deletions crypto/fipsmodule/ml_dsa/ml_dsa.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,16 @@ int ml_dsa_44_sign(const uint8_t *private_key /* IN */,
ctx_string, ctx_string_len, private_key) == 0;
}

int ml_dsa_extmu_44_sign(const uint8_t *private_key /* IN */,

This comment has been minimized.

Sign in to view

Copy link
@nebeid

nebeid Jan 10, 2025

I suggest here and the other functions to keep the parameter size next to dsa, i.e. ml_dsa_44_extmu_sign

This comment has been minimized.

Sign in to view

Copy link
@jakemas

jakemas Jan 11, 2025

Author Owner

My rational was this:

It seems clearer to me to spot the differing security level between:
ml_dsa_extmu_44
ml_dsa_extmu_65
ml_dsa_extmu_87

than hiding it mid way in the string:

ml_dsa_44_extmu
ml_dsa_65_extmu
ml_dsa_87_extmu

But I don't feel too strongly.

This comment has been minimized.

Sign in to view

Copy link
@nebeid

nebeid Jan 12, 2025 •
edited
Loading

It's a good point. I thought of that because extmu is a qualifier of sign and verify. So for example, it won't appear with keygen. So I read it as extmu_sign, extmu_verify.

This comment has been minimized.

Sign in to view

Copy link
@jakemas

jakemas Jan 13, 2025 •
edited
Loading

Author Owner

I'm conceptalizing ML-DSA-EXTMU as the algorithm name (as opposed to ML-DSA for pure mode), so for me it feels more natural to call functions that were once ml_dsa_{paramater_set}_{specific_function_name} to become ml_dsa_ext_mu_{paramater_set}_{specific_function_name}

This comment has been minimized.

Sign in to view

Copy link
@nebeid

nebeid Jan 13, 2025

Sounds good to me
uint8_t *sig /* OUT */,
size_t *sig_len /* OUT */,
const uint8_t *message /* IN */,

This comment has been minimized.

Sign in to view

Copy link
@nebeid

nebeid Jan 11, 2025

do we want to replace message with mu

This comment has been minimized.

Sign in to view

Copy link
@jakemas

jakemas Jan 11, 2025

Author Owner

yes! I did do it already in places -- will do in PR
size_t message_len /* IN */) {
ml_dsa_params params;
ml_dsa_44_params_init(&params);
return ml_dsa_extmu_sign(&params, sig, sig_len, message, message_len, private_key) == 0;

This comment has been minimized.

Sign in to view

Copy link
@nebeid

nebeid Jan 11, 2025

just curious: why in sign, extmu_sign is called while in verify, verify_internal is called?

This comment has been minimized.

Sign in to view

Copy link
@jakemas

jakemas Jan 11, 2025 •
edited
Loading

Author Owner

More efficient/cleaner implementation. I did write extmu_verify, but there was no functionality in the function. The only thing the function did was call return ml_dsa_verify_internal(&params, sig, sig_len, message, message_len, NULL, 0, public_key, 1) == 0;. So i just inlined it.

Because there is no randomness to generate for verify, no ctx to process, there is nothing else to do other than call internal.
}

int ml_dsa_44_sign_internal(const uint8_t *private_key /* IN */,
uint8_t *sig /* OUT */,
size_t *sig_len /* OUT */,
Expand All @@ -62,7 +72,7 @@ int ml_dsa_44_sign_internal(const uint8_t *private_key /* IN */,
ml_dsa_params params;
ml_dsa_44_params_init(&params);
return ml_dsa_sign_internal(&params, sig, sig_len, message, message_len,
pre, pre_len, rnd, private_key) == 0;
pre, pre_len, rnd, private_key, 0) == 0;
}

int ml_dsa_44_verify(const uint8_t *public_key /* IN */,
Expand All @@ -78,6 +88,16 @@ int ml_dsa_44_verify(const uint8_t *public_key /* IN */,
ctx_string, ctx_string_len, public_key) == 0;
}

int ml_dsa_extmu_44_verify(const uint8_t *public_key /* IN */,
const uint8_t *sig /* IN */,
size_t sig_len /* IN */,
const uint8_t *message /* IN */,
size_t message_len /* IN */) {
ml_dsa_params params;
ml_dsa_44_params_init(&params);
return ml_dsa_verify_internal(&params, sig, sig_len, message, message_len, NULL, 0, public_key, 1) == 0;
}

int ml_dsa_44_verify_internal(const uint8_t *public_key /* IN */,
const uint8_t *sig /* IN */,
size_t sig_len /* IN */,
Expand All @@ -88,7 +108,7 @@ int ml_dsa_44_verify_internal(const uint8_t *public_key /* IN */,
ml_dsa_params params;
ml_dsa_44_params_init(&params);
return ml_dsa_verify_internal(&params, sig, sig_len, message, message_len,
pre, pre_len, public_key) == 0;
pre, pre_len, public_key, 0) == 0;
}

int ml_dsa_65_keypair(uint8_t *public_key /* OUT */,
Expand Down Expand Up @@ -119,6 +139,16 @@ int ml_dsa_65_sign(const uint8_t *private_key /* IN */,
ctx_string, ctx_string_len, private_key) == 0;
}

int ml_dsa_extmu_65_sign(const uint8_t *private_key /* IN */,
uint8_t *sig /* OUT */,
size_t *sig_len /* OUT */,
const uint8_t *message /* IN */,
size_t message_len /* IN */) {
ml_dsa_params params;
ml_dsa_65_params_init(&params);
return ml_dsa_extmu_sign(&params, sig, sig_len, message, message_len, private_key) == 0;
}

int ml_dsa_65_sign_internal(const uint8_t *private_key /* IN */,
uint8_t *sig /* OUT */,
size_t *sig_len /* OUT */,
Expand All @@ -130,7 +160,7 @@ int ml_dsa_65_sign_internal(const uint8_t *private_key /* IN */,
ml_dsa_params params;
ml_dsa_65_params_init(&params);
return ml_dsa_sign_internal(&params, sig, sig_len, message, message_len,
pre, pre_len, rnd, private_key) == 0;
pre, pre_len, rnd, private_key, 0) == 0;
}

int ml_dsa_65_verify(const uint8_t *public_key /* IN */,
Expand All @@ -146,6 +176,16 @@ int ml_dsa_65_verify(const uint8_t *public_key /* IN */,
ctx_string, ctx_string_len, public_key) == 0;
}

int ml_dsa_extmu_65_verify(const uint8_t *public_key /* IN */,
const uint8_t *sig /* IN */,
size_t sig_len /* IN */,
const uint8_t *message /* IN */,
size_t message_len /* IN */) {
ml_dsa_params params;
ml_dsa_65_params_init(&params);
return ml_dsa_verify_internal(&params, sig, sig_len, message, message_len, NULL, 0, public_key, 1) == 0;
}

int ml_dsa_65_verify_internal(const uint8_t *public_key /* IN */,
const uint8_t *sig /* IN */,
size_t sig_len /* IN */,
Expand All @@ -156,7 +196,7 @@ int ml_dsa_65_verify_internal(const uint8_t *public_key /* IN */,
ml_dsa_params params;
ml_dsa_65_params_init(&params);
return ml_dsa_verify_internal(&params, sig, sig_len, message, message_len,
pre, pre_len, public_key) == 0;
pre, pre_len, public_key, 0) == 0;
}

int ml_dsa_87_keypair(uint8_t *public_key /* OUT */,
Expand Down Expand Up @@ -187,6 +227,16 @@ int ml_dsa_87_sign(const uint8_t *private_key /* IN */,
ctx_string, ctx_string_len, private_key) == 0;
}

int ml_dsa_extmu_87_sign(const uint8_t *private_key /* IN */,
uint8_t *sig /* OUT */,
size_t *sig_len /* OUT */,
const uint8_t *message /* IN */,
size_t message_len /* IN */) {
ml_dsa_params params;
ml_dsa_87_params_init(&params);
return ml_dsa_extmu_sign(&params, sig, sig_len, message, message_len, private_key) == 0;
}

int ml_dsa_87_sign_internal(const uint8_t *private_key /* IN */,
uint8_t *sig /* OUT */,
size_t *sig_len /* OUT */,
Expand All @@ -198,7 +248,7 @@ int ml_dsa_87_sign_internal(const uint8_t *private_key /* IN */,
ml_dsa_params params;
ml_dsa_87_params_init(&params);
return ml_dsa_sign_internal(&params, sig, sig_len, message, message_len,
pre, pre_len, rnd, private_key) == 0;
pre, pre_len, rnd, private_key, 0) == 0;
}

int ml_dsa_87_verify(const uint8_t *public_key /* IN */,
Expand All @@ -214,6 +264,16 @@ int ml_dsa_87_verify(const uint8_t *public_key /* IN */,
ctx_string, ctx_string_len, public_key) == 0;
}

int ml_dsa_extmu_87_verify(const uint8_t *public_key /* IN */,
const uint8_t *sig /* IN */,
size_t sig_len /* IN */,
const uint8_t *message /* IN */,
size_t message_len /* IN */) {
ml_dsa_params params;
ml_dsa_87_params_init(&params);
return ml_dsa_verify_internal(&params, sig, sig_len, message, message_len, NULL, 0, public_key, 1) == 0;
}

int ml_dsa_87_verify_internal(const uint8_t *public_key /* IN */,
const uint8_t *sig /* IN */,
size_t sig_len /* IN */,
Expand All @@ -224,5 +284,5 @@ int ml_dsa_87_verify_internal(const uint8_t *public_key /* IN */,
ml_dsa_params params;
ml_dsa_87_params_init(&params);
return ml_dsa_verify_internal(&params, sig, sig_len, message, message_len,
pre, pre_len, public_key) == 0;
pre, pre_len, public_key, 0) == 0;
}
Loading

0 comments on commit 1f2f577

Please sign in to comment.