Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 28 commits into from
Nov 18, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 67 additions & 0 deletions crypto/dilithium/internal.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0 OR ISC

#ifndef AWSLC_HEADER_SIG_INTERNAL_H
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#ifndef AWSLC_HEADER_SIG_INTERNAL_H
#ifndef AWSLC_HEADER_PQDSA_INTERNAL_H

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed in 7435f9b

#define AWSLC_HEADER_SIG_INTERNAL_H

#include <openssl/base.h>

#if defined(__cplusplus)
extern "C" {
#endif

// PQDSA_METHOD structure and helper functions.
typedef struct {
dkostic marked this conversation as resolved.
Show resolved Hide resolved
int (*keygen)(uint8_t *public_key,
dkostic marked this conversation as resolved.
Show resolved Hide resolved
uint8_t *secret_key);

int (*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,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can ctx be used via the EVP API?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

size_t ctx_len);

int (*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);

} PQDSA_METHOD;

// PQDSA structure and helper functions.
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;

// PQDSA_KEY structure and helper functions.
struct pqdsa_key_st {
const PQDSA *pqdsa;
uint8_t *public_key;
uint8_t *secret_key;
};

int PQDSA_KEY_init(PQDSA_KEY *key, const PQDSA *pqdsa);
const PQDSA * PQDSA_find_dsa_by_nid(int nid);
const PQDSA *PQDSA_KEY_get0_dsa(PQDSA_KEY* key);
PQDSA_KEY *PQDSA_KEY_new(void);
void PQDSA_KEY_free(PQDSA_KEY *key);

#if defined(__cplusplus)
} // extern C
#endif

#endif // AWSLC_HEADER_DSA_TEST_INTERNAL_H
Loading
Loading