-
Notifications
You must be signed in to change notification settings - Fork 11
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
ML-KEM and ML-KEM hybrid #103
base: main
Are you sure you want to change the base?
Changes from 21 commits
2305e45
09dfe6f
5bc0a0d
b42de06
abe8cf7
f4ba041
8bd3af6
495b852
4ddd8ca
78edcf2
b6c3c2d
600c0d6
62ae32d
739ce6b
6c5ff94
05f1c24
7054d72
e89449f
0a98fd5
bb3e09b
3b662e7
82ef0fa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,123 @@ | ||
// | ||
// Copyright (c) Microsoft Corporation. Licensed under the MIT license. | ||
// | ||
|
||
#include "p_scossl_bio.h" | ||
#include "p_scossl_decode_common.h" | ||
|
||
#include <openssl/asn1t.h> | ||
#include <openssl/core_object.h> | ||
|
||
#ifdef __cplusplus | ||
extern "C" { | ||
#endif | ||
|
||
ASN1_NDEF_SEQUENCE(SUBJECT_PUBKEY_INFO) = { | ||
ASN1_SIMPLE(SUBJECT_PUBKEY_INFO, x509Alg, X509_ALGOR), | ||
ASN1_SIMPLE(SUBJECT_PUBKEY_INFO, subjectPublicKey, ASN1_BIT_STRING), | ||
} ASN1_SEQUENCE_END(SUBJECT_PUBKEY_INFO) | ||
|
||
IMPLEMENT_ASN1_FUNCTIONS(SUBJECT_PUBKEY_INFO) | ||
|
||
const OSSL_PARAM p_scossl_der_to_key_settable_param_types[] = { | ||
OSSL_PARAM_END}; | ||
|
||
_Use_decl_annotations_ | ||
SCOSSL_DECODE_CTX *p_scossl_decode_newctx(SCOSSL_PROVCTX *provctx, const SCOSSL_DECODE_KEYTYPE_DESC *desc) | ||
{ | ||
SCOSSL_DECODE_CTX *ctx = OPENSSL_zalloc(sizeof(SCOSSL_DECODE_CTX)); | ||
|
||
if (ctx != NULL) | ||
{ | ||
ctx->provctx = provctx; | ||
ctx->desc = desc; | ||
} | ||
|
||
return ctx; | ||
} | ||
|
||
_Use_decl_annotations_ | ||
void p_scossl_decode_freectx(SCOSSL_DECODE_CTX *ctx) | ||
{ | ||
if (ctx == NULL) | ||
return; | ||
|
||
OPENSSL_free(ctx); | ||
} | ||
|
||
SCOSSL_STATUS p_scossl_decode_set_ctx_params(ossl_unused void *ctx, ossl_unused const OSSL_PARAM params[]) | ||
{ | ||
return SCOSSL_SUCCESS; | ||
} | ||
|
||
const OSSL_PARAM *p_scossl_decode_settable_ctx_params(ossl_unused void *ctx) | ||
{ | ||
return p_scossl_der_to_key_settable_param_types; | ||
} | ||
|
||
_Use_decl_annotations_ | ||
BOOL p_scossl_decode_does_selection(const SCOSSL_DECODE_KEYTYPE_DESC *desc, int selection) | ||
{ | ||
if (selection == 0) | ||
{ | ||
return TRUE; | ||
} | ||
|
||
// Supporting private key implies supporting public key. | ||
// Both imply supporting key parameters | ||
return ((selection & OSSL_KEYMGMT_SELECT_PRIVATE_KEY) != 0 && (desc->selection & OSSL_KEYMGMT_SELECT_PRIVATE_KEY) != 0) || | ||
((selection & OSSL_KEYMGMT_SELECT_PUBLIC_KEY) != 0 && (desc->selection & OSSL_KEYMGMT_SELECT_PUBLIC_KEY) != 0) || | ||
((selection & OSSL_KEYMGMT_SELECT_ALL_PARAMETERS) != 0 && (desc->selection & OSSL_KEYMGMT_SELECT_ALL_PARAMETERS) != 0); | ||
} | ||
|
||
// This function should return SCOSSL_SUCCESS if it successfully decodes something, | ||
// or decodes nothing at all. Another decoder may be able to decode the data into something. | ||
// This function should only return SCOSSL_FAILURE if the data could be decoded, but further | ||
// validation of the data failed in a way that another decoder could not handle. | ||
_Use_decl_annotations_ | ||
SCOSSL_STATUS p_scossl_decode(SCOSSL_DECODE_CTX *ctx, OSSL_CORE_BIO *in, int selection, | ||
OSSL_CALLBACK *dataCb, void *dataCbArg, | ||
ossl_unused OSSL_PASSPHRASE_CALLBACK *passphraseCb, ossl_unused void *passphraseCbArg) | ||
{ | ||
BIO *bio = NULL; | ||
PVOID *keyCtx = NULL; | ||
OSSL_PARAM cbParams[4]; | ||
SCOSSL_STATUS ret = SCOSSL_SUCCESS; | ||
|
||
if (selection == 0) | ||
{ | ||
selection = ctx->desc->selection; | ||
} | ||
|
||
if ((selection & ctx->desc->selection) != 0 && | ||
(bio = p_scossl_bio_new_from_core_bio(ctx->provctx, in)) != NULL) | ||
{ | ||
keyCtx = ctx->desc->decodeInternal(ctx, bio); | ||
} | ||
|
||
if (keyCtx != NULL) | ||
{ | ||
int objectType = OSSL_OBJECT_PKEY; | ||
|
||
cbParams[0] = OSSL_PARAM_construct_int(OSSL_OBJECT_PARAM_TYPE, &objectType); | ||
cbParams[1] = OSSL_PARAM_construct_utf8_string(OSSL_OBJECT_PARAM_DATA_TYPE, (char *)ctx->desc->dataType, 0); | ||
cbParams[2] = OSSL_PARAM_construct_octet_string(OSSL_OBJECT_PARAM_REFERENCE, &keyCtx, sizeof(keyCtx)); | ||
cbParams[3] = OSSL_PARAM_construct_end(); | ||
|
||
ret = dataCb(cbParams, dataCbArg); | ||
} | ||
|
||
BIO_free(bio); | ||
ctx->desc->freeKeyCtx(keyCtx); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. NP: Probably doesn't matter, but probably should free There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It doesn't really matter but it does look cleaner. |
||
|
||
return ret; | ||
} | ||
|
||
const ASN1_ITEM *p_scossl_decode_subject_pubkey_asn1_item() | ||
{ | ||
return ASN1_ITEM_rptr(SUBJECT_PUBKEY_INFO); | ||
} | ||
|
||
#ifdef __cplusplus | ||
} | ||
#endif |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,55 @@ | ||
// | ||
// Copyright (c) Microsoft Corporation. Licensed under the MIT license. | ||
// | ||
|
||
#include "p_scossl_base.h" | ||
|
||
#ifdef __cplusplus | ||
extern "C" { | ||
#endif | ||
|
||
#define select_PrivateKeyInfo OSSL_KEYMGMT_SELECT_PRIVATE_KEY | ||
#define select_SubjectPublicKeyInfo OSSL_KEYMGMT_SELECT_PUBLIC_KEY | ||
|
||
typedef PVOID (*PSCOSSL_DECODE_INTERNAL_FN) (_In_ PVOID decodeCtx, _In_ BIO *bio); | ||
|
||
typedef struct | ||
{ | ||
const char *dataType; | ||
int selection; | ||
|
||
PSCOSSL_DECODE_INTERNAL_FN decodeInternal; | ||
OSSL_FUNC_keymgmt_free_fn *freeKeyCtx; | ||
} SCOSSL_DECODE_KEYTYPE_DESC; | ||
|
||
typedef struct scossl_decode_ctx_st | ||
{ | ||
SCOSSL_PROVCTX *provctx; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems like we currently have a recipe for potential use-after-free with the number of provider structures keeping a pointer to the single provider context associated with a Should we have a refcount-ed provctx, or is OpenSSL guaranteed to ensure that all provider structures are freed before the provider is torn down? Perhaps it's not a huge deal if SCOSSL is the default provider, but could definitely be more of an issue for a place where an application dynamically loads/unloads SCOSSL. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think there's a legitimate case. The OpenSSL default provider doesn't refcount this so we should be ok. It seems like the providers were originally built with that being a valid use case (which is why we saw use-after-free issues in |
||
|
||
const SCOSSL_DECODE_KEYTYPE_DESC *desc; | ||
} SCOSSL_DECODE_CTX; | ||
|
||
typedef struct | ||
{ | ||
X509_ALGOR *x509Alg; | ||
ASN1_BIT_STRING *subjectPublicKey; | ||
} SUBJECT_PUBKEY_INFO; | ||
|
||
SCOSSL_DECODE_CTX *p_scossl_decode_newctx(_In_ SCOSSL_PROVCTX *provctx, _In_ const SCOSSL_DECODE_KEYTYPE_DESC *desc); | ||
void p_scossl_decode_freectx(_Inout_ SCOSSL_DECODE_CTX *ctx); | ||
|
||
SCOSSL_STATUS p_scossl_decode_set_ctx_params(ossl_unused void *ctx, ossl_unused const OSSL_PARAM params[]); | ||
const OSSL_PARAM *p_scossl_decode_settable_ctx_params(ossl_unused void *ctx); | ||
|
||
BOOL p_scossl_decode_does_selection(_In_ const SCOSSL_DECODE_KEYTYPE_DESC *desc, int selection); | ||
|
||
SCOSSL_STATUS p_scossl_decode(_In_ SCOSSL_DECODE_CTX *ctx, _In_ OSSL_CORE_BIO *in, | ||
int selection, | ||
_In_ OSSL_CALLBACK *dataCb, _In_ void *dataCbArg, | ||
ossl_unused OSSL_PASSPHRASE_CALLBACK *passphraseCb, ossl_unused void *passphraseCbArg); | ||
|
||
const ASN1_ITEM *p_scossl_decode_subject_pubkey_asn1_item(); | ||
|
||
#ifdef __cplusplus | ||
} | ||
#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
_In_
annotation on this function indicates thatcurve
is a required parameter which must not be NULL. In Windows and SymCrypt, we'd typically not do runtime parameter validation here, but would instead rely on assertions and/or PREfast to catch invalid calls.That said, if this is public function, it probably makes sense to be more defensive. I'm also not sure what the OpenSSL philosophy is regarding input validation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair point. This is an internal function so I can ensure we do the validation outside of this function. I don't think this needs to be changed to
_In_opt_