Skip to content

Commit

Permalink
DECODE_UINT32 without lengths checked fixed (#416)
Browse files Browse the repository at this point in the history 
* DECODE_UINT32 without lengths checked fixed

Signed-off-by: Bence Mali <bence.mali@tresorit.com>
Signed-off-by: Michael Baentsch <57787676+baentsch@users.noreply.github.com>

* fixing clang-14 package in CI

Signed-off-by: Michael Baentsch <57787676+baentsch@users.noreply.github.com>

---------

Signed-off-by: Bence Mali <bence.mali@tresorit.com>
Signed-off-by: Michael Baentsch <57787676+baentsch@users.noreply.github.com>
Co-authored-by: Bence Mali <bence.mali@tresorit.com>
  • Loading branch information
@baentsch @bencemali
baentsch and bencemali authored May 21, 2024
1 parent ca582f9 commit 67ad183
Show file tree
Hide file tree
Showing 5 changed files with 116 additions and 21 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/linux.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ jobs:
uses: actions/checkout@v2

- name: Install dependencies
run: apt-get update && apt-get install -y clang llvm ninja-build git cmake libclang-rt-14-dev libclang-common-14-dev
run: apt-get update && apt-get install -y clang llvm ninja-build git cmake libclang-14-dev libclang-common-14-dev

- name: Clone and build OpenSSL(3) with ASan
run: |
Expand Down
40 changes: 33 additions & 7 deletions oqsprov/oqs_encode_key2any.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -617,7 +617,7 @@ static int oqsx_pki_priv_to_der(const void *vxkey, unsigned char **pder)
{
OQSX_KEY *oqsxkey = (OQSX_KEY *)vxkey;
unsigned char *buf = NULL;
int buflen = 0, privkeylen;
uint32_t buflen = 0, privkeylen = 0;
ASN1_OCTET_STRING oct;
int keybloblen, nid;
STACK_OF(ASN1_TYPE) *sk = NULL;
Expand All @@ -643,9 +643,14 @@ static int oqsx_pki_priv_to_der(const void *vxkey, unsigned char **pder)
if (oqsxkey->keytype != KEY_TYPE_CMP_SIG) {
privkeylen = oqsxkey->privkeylen;
if (oqsxkey->numkeys > 1) { // hybrid
int actualprivkeylen;
uint32_t actualprivkeylen = 0;
size_t fixed_pq_privkeylen
= oqsxkey->oqsx_provider_ctx.oqsx_qs_ctx.kem->length_secret_key;
size_t space_for_classical_privkey
= privkeylen - SIZE_OF_UINT32 - fixed_pq_privkeylen;
DECODE_UINT32(actualprivkeylen, oqsxkey->privkey);
if (actualprivkeylen > oqsxkey->evp_info->length_private_key) {
if ((actualprivkeylen > oqsxkey->evp_info->length_private_key)
|| (actualprivkeylen > space_for_classical_privkey)) {
ERR_raise(ERR_LIB_USER, OQSPROV_R_INVALID_ENCODING);
return 0;
}
Expand Down Expand Up @@ -690,7 +695,7 @@ static int oqsx_pki_priv_to_der(const void *vxkey, unsigned char **pder)
ERR_raise(ERR_LIB_USER, ERR_R_MALLOC_FAILURE);
return -1;
}
OQS_ENC_PRINTF2("OQS ENC provider: saving privkey of length %d\n",
OQS_ENC_PRINTF2("OQS ENC provider: saving privkey of length %zu\n",
buflen);
memcpy(buf, oqsxkey->privkey, privkeylen);
#else
Expand Down Expand Up @@ -1726,7 +1731,8 @@ static int oqsx_to_text(BIO *out, const void *key, int selection)
if (okey->keytype == KEY_TYPE_CMP_SIG) {
char *name;
char label[200];
int i, privlen;
int i;
uint32_t privlen = 0;
for (i = 0; i < okey->numkeys; i++) {
if ((name = get_cmpname(OBJ_sn2nid(okey->tls_name), i))
== NULL) {
Expand Down Expand Up @@ -1761,10 +1767,20 @@ static int oqsx_to_text(BIO *out, const void *key, int selection)
} else {
if (okey->numkeys > 1) { // hybrid key
char classic_label[200];
int classic_key_len = 0;
uint32_t classic_key_len = 0;
size_t fixed_pq_privkey_len
= okey->oqsx_provider_ctx.oqsx_qs_ctx.kem
->length_secret_key;
size_t space_for_classical_privkey = okey->privkeylen
- SIZE_OF_UINT32
- fixed_pq_privkey_len;
sprintf(classic_label, "%s key material:",
OBJ_nid2sn(okey->evp_info->nid));
DECODE_UINT32(classic_key_len, okey->privkey);
if (classic_key_len > space_for_classical_privkey) {
ERR_raise(ERR_LIB_USER, OQSPROV_R_INVALID_ENCODING);
return 0;
}
if (!print_labeled_buf(out, classic_label,
okey->comp_privkey[0],
classic_key_len))
Expand Down Expand Up @@ -1809,8 +1825,18 @@ static int oqsx_to_text(BIO *out, const void *key, int selection)
} else {
if (okey->numkeys > 1) { // hybrid key
char classic_label[200];
int classic_key_len = 0;
uint32_t classic_key_len = 0;
size_t fixed_pq_pubkey_len
= okey->oqsx_provider_ctx.oqsx_qs_ctx.kem
->length_public_key;
size_t space_for_classical_pubkey = okey->pubkeylen
- SIZE_OF_UINT32
- fixed_pq_pubkey_len;
DECODE_UINT32(classic_key_len, okey->pubkey);
if (classic_key_len > space_for_classical_pubkey) {
ERR_raise(ERR_LIB_USER, OQSPROV_R_INVALID_ENCODING);
return 0;
}
sprintf(classic_label, "%s key material:",
OBJ_nid2sn(okey->evp_info->nid));
if (!print_labeled_buf(out, classic_label,
Expand Down
4 changes: 2 additions & 2 deletions oqsprov/oqs_kmgmt.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -364,8 +364,8 @@ static int oqsx_get_hybrid_params(OQSX_KEY *key, OSSL_PARAM params[])
const void *classical_privkey = NULL;
const void *pq_pubkey = NULL;
const void *pq_privkey = NULL;
int classical_pubkey_len = 0;
int classical_privkey_len = 0;
uint32_t classical_pubkey_len = 0;
uint32_t classical_privkey_len = 0;
int pq_pubkey_len = 0;
int pq_privkey_len = 0;

Expand Down
22 changes: 20 additions & 2 deletions oqsprov/oqs_sig.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -651,9 +651,13 @@ static int oqs_sig_verify(void *vpoqs_sigctx, const unsigned char *sig,

if (is_hybrid) {
const EVP_MD *classical_md;
size_t actual_classical_sig_len = 0;
uint32_t actual_classical_sig_len = 0;
int digest_len;
unsigned char digest[SHA512_DIGEST_LENGTH]; /* init with max length */
size_t max_pq_sig_len
= oqsxkey->oqsx_provider_ctx.oqsx_qs_ctx.sig->length_signature;
size_t max_classical_sig_len = oqsxkey->oqsx_provider_ctx.oqsx_evp_ctx
->evp_info->length_signature;

if ((ctx_verify = EVP_PKEY_CTX_new(oqsxkey->classical_pkey, NULL))
== NULL
Expand All @@ -668,7 +672,21 @@ static int oqs_sig_verify(void *vpoqs_sigctx, const unsigned char *sig,
goto endverify;
}
}
DECODE_UINT32(actual_classical_sig_len, sig);
if (siglen > SIZE_OF_UINT32) {
size_t actual_pq_sig_len = 0;
DECODE_UINT32(actual_classical_sig_len, sig);
actual_pq_sig_len
= siglen - SIZE_OF_UINT32 - actual_classical_sig_len;
if (siglen <= (SIZE_OF_UINT32 + actual_classical_sig_len)
|| actual_classical_sig_len > max_classical_sig_len
|| actual_pq_sig_len > max_pq_sig_len) {
ERR_raise(ERR_LIB_USER, OQSPROV_R_INVALID_ENCODING);
goto endverify;
}
} else {
ERR_raise(ERR_LIB_USER, OQSPROV_R_INVALID_ENCODING);
goto endverify;
}

/* same as with sign: activate if pre-existing hashing to be used:
* if (poqs_sigctx->mdctx == NULL) { // hashing not yet done
Expand Down
69 changes: 60 additions & 9 deletions oqsprov/oqsprov_keys.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -317,11 +317,16 @@ static int oqsx_key_set_composites(OQSX_KEY *key)
}
}
} else {
int classic_pubkey_len, classic_privkey_len;
uint32_t classic_pubkey_len = 0;
uint32_t classic_privkey_len = 0;

if (key->privkey) {
key->comp_privkey[0] = (char *)key->privkey + SIZE_OF_UINT32;
DECODE_UINT32(classic_privkey_len, key->privkey);
if (classic_privkey_len > key->evp_info->length_private_key) {
ERR_raise(ERR_LIB_USER, OQSPROV_R_INVALID_ENCODING);
goto err;
}
key->comp_privkey[1] = (char *)key->privkey
+ classic_privkey_len + SIZE_OF_UINT32;
} else {
Expand All @@ -331,6 +336,10 @@ static int oqsx_key_set_composites(OQSX_KEY *key)
if (key->pubkey) {
key->comp_pubkey[0] = (char *)key->pubkey + SIZE_OF_UINT32;
DECODE_UINT32(classic_pubkey_len, key->pubkey);
if (classic_pubkey_len > key->evp_info->length_public_key) {
ERR_raise(ERR_LIB_USER, OQSPROV_R_INVALID_ENCODING);
goto err;
}
key->comp_pubkey[1]
= (char *)key->pubkey + classic_pubkey_len + SIZE_OF_UINT32;
} else {
Expand Down Expand Up @@ -649,13 +658,13 @@ static OQSX_KEY *oqsx_key_op(const X509_ALGOR *palg, const unsigned char *p,
}
#endif
} else {
int classical_privatekey_len = 0;
uint32_t classical_privatekey_len = 0;
// for plain OQS keys, we expect OQS priv||OQS pub key
size_t actualprivkeylen = key->privkeylen;
// for hybrid keys, we expect classic priv key||OQS priv key||OQS pub
// key classic pub key must/can be re-created from classic private key
if (key->keytype == KEY_TYPE_CMP_SIG) {
size_t privlen = 0;
uint32_t privlen = 0;
size_t publen = 0;
size_t previous_privlen = 0;
size_t previous_publen = 0;
Expand Down Expand Up @@ -720,6 +729,13 @@ static OQSX_KEY *oqsx_key_op(const X509_ALGOR *palg, const unsigned char *p,
// keys. will recreate the pubkey later
if (key->oqsx_provider_ctx.oqsx_evp_ctx->evp_info->keytype
== EVP_PKEY_RSA) { // get the RSA real key size
if (previous_privlen + previous_publen + 4 > plen) {
OPENSSL_free(name);
OPENSSL_secure_clear_free(temp_priv, temp_priv_len);
OPENSSL_secure_clear_free(temp_pub, temp_pub_len);
ERR_raise(ERR_LIB_USER, OQSPROV_R_INVALID_ENCODING);
goto err_key_op;
}
unsigned char *enc_len = OPENSSL_strndup(
p + previous_privlen + previous_publen, 4);
OPENSSL_cleanse(enc_len, 2);
Expand All @@ -743,6 +759,13 @@ static OQSX_KEY *oqsx_key_op(const X509_ALGOR *palg, const unsigned char *p,
else
publen = 0;
}
if (previous_privlen + previous_publen + privlen > plen) {
OPENSSL_free(name);
OPENSSL_secure_clear_free(temp_priv, temp_priv_len);
OPENSSL_secure_clear_free(temp_pub, temp_pub_len);
ERR_raise(ERR_LIB_USER, OQSPROV_R_INVALID_ENCODING);
goto err_key_op;
}
memcpy(temp_priv + previous_privlen,
p + previous_privlen + previous_publen, privlen);
memcpy(temp_pub + previous_publen,
Expand All @@ -758,11 +781,30 @@ static OQSX_KEY *oqsx_key_op(const X509_ALGOR *palg, const unsigned char *p,
OPENSSL_secure_clear_free(temp_pub, temp_pub_len);
} else {
if (key->numkeys == 2) {
DECODE_UINT32(classical_privatekey_len,
p); // actual classic key len
// adjust expected size
if (classical_privatekey_len
> key->evp_info->length_private_key) {
size_t expected_pq_privkey_len
= key->oqsx_provider_ctx.oqsx_qs_ctx.kem->length_secret_key;
#ifndef NOPUBKEY_IN_PRIVKEY
expected_pq_privkey_len += key->oqsx_provider_ctx.oqsx_qs_ctx
.kem->length_public_key;
#endif
if (plen > (SIZE_OF_UINT32 + expected_pq_privkey_len)) {
size_t max_classical_privkey_len
= key->evp_info->length_private_key;
size_t space_for_classical_privkey
= plen - expected_pq_privkey_len - SIZE_OF_UINT32;
if (space_for_classical_privkey
> max_classical_privkey_len) {
ERR_raise(ERR_LIB_USER, OQSPROV_R_INVALID_ENCODING);
goto err_key_op;
}
DECODE_UINT32(classical_privatekey_len,
p); // actual classic key len
if (classical_privatekey_len
!= space_for_classical_privkey) {
ERR_raise(ERR_LIB_USER, OQSPROV_R_INVALID_ENCODING);
goto err_key_op;
}
} else {
ERR_raise(ERR_LIB_USER, OQSPROV_R_INVALID_ENCODING);
goto err_key_op;
}
Expand Down Expand Up @@ -970,14 +1012,19 @@ static int oqsx_key_recreate_classickey(OQSX_KEY *key, oqsx_key_op_t op)
}
} else {
if (key->numkeys == 2) { // hybrid key
int classical_pubkey_len, classical_privkey_len;
uint32_t classical_pubkey_len = 0;
uint32_t classical_privkey_len = 0;
if (!key->evp_info) {
ERR_raise(ERR_LIB_USER, OQSPROV_R_EVPINFO_MISSING);
goto rec_err;
}
if (op == KEY_OP_PUBLIC) {
const unsigned char *enc_pubkey = key->comp_pubkey[0];
DECODE_UINT32(classical_pubkey_len, key->pubkey);
if (classical_pubkey_len > key->evp_info->length_public_key) {
ERR_raise(ERR_LIB_USER, OQSPROV_R_INVALID_ENCODING);
goto rec_err;
}
if (key->evp_info->raw_key_support) {
key->classical_pkey = EVP_PKEY_new_raw_public_key(
key->evp_info->keytype, NULL, enc_pubkey,
Expand All @@ -1003,6 +1050,10 @@ static int oqsx_key_recreate_classickey(OQSX_KEY *key, oqsx_key_op_t op)
}
if (op == KEY_OP_PRIVATE) {
DECODE_UINT32(classical_privkey_len, key->privkey);
if (classical_privkey_len > key->evp_info->length_private_key) {
ERR_raise(ERR_LIB_USER, OQSPROV_R_INVALID_ENCODING);
goto rec_err;
}
const unsigned char *enc_privkey = key->comp_privkey[0];
unsigned char *enc_pubkey = key->comp_pubkey[0];
if (key->evp_info->raw_key_support) {
Expand Down

0 comments on commit 67ad183

Please sign in to comment.