From e8c61cc5ba5097e3f04bc71539b08440edde3688 Mon Sep 17 00:00:00 2001 From: Steffen Jaeckel Date: Sun, 2 Jun 2019 22:23:02 +0200 Subject: [PATCH] fix length-related wycheproof testcases --- src/headers/tomcrypt_pk.h | 8 ++++++++ src/headers/tomcrypt_private.h | 3 ++- src/pk/asn1/der/custom_type/der_decode_custom_type.c | 6 ++++-- src/pk/asn1/der/general/der_decode_asn1_length.c | 9 ++++++++- src/pk/ecc/ecc_verify_hash.c | 2 +- 5 files changed, 23 insertions(+), 5 deletions(-) diff --git a/src/headers/tomcrypt_pk.h b/src/headers/tomcrypt_pk.h index 01b8126c3..1497d4fb5 100644 --- a/src/headers/tomcrypt_pk.h +++ b/src/headers/tomcrypt_pk.h @@ -596,9 +596,17 @@ enum ltc_der_seq { LTC_DER_SEQ_RELAXED = LTC_DER_SEQ_ZERO, LTC_DER_SEQ_STRICT = 0x2u, + /** Bit2 - [0]=Relaxed Length Check + * [1]=Strict Length Check */ + LTC_DER_SEQ_LEN_RELAXED = LTC_DER_SEQ_ZERO, + LTC_DER_SEQ_LEN_STRICT = 0x4u, + /** Alternative naming */ LTC_DER_SEQ_SET = LTC_DER_SEQ_UNORDERED, LTC_DER_SEQ_SEQUENCE = LTC_DER_SEQ_ORDERED, + + LTC_DER_SEQ_ALL_STRICT = LTC_DER_SEQ_STRICT | LTC_DER_SEQ_LEN_STRICT, + }; int der_decode_sequence_ex(const unsigned char *in, unsigned long inlen, diff --git a/src/headers/tomcrypt_private.h b/src/headers/tomcrypt_private.h index 4c3ea9fd3..fe00029ec 100644 --- a/src/headers/tomcrypt_private.h +++ b/src/headers/tomcrypt_private.h @@ -351,7 +351,8 @@ int der_decode_asn1_identifier(const unsigned char *in, unsigned long *inlen, lt int der_length_asn1_identifier(const ltc_asn1_list *id, unsigned long *idlen); int der_encode_asn1_length(unsigned long len, unsigned char* out, unsigned long* outlen); -int der_decode_asn1_length(const unsigned char *in, unsigned long *inlen, unsigned long *outlen); +int der_decode_asn1_length_ex(const unsigned char *in, unsigned long *inlen, unsigned long *outlen, unsigned int flags); +#define der_decode_asn1_length(i, il, ol) der_decode_asn1_length_ex(i, il, ol, 0) int der_length_asn1_length(unsigned long len, unsigned long *outlen); int der_length_sequence_ex(const ltc_asn1_list *list, unsigned long inlen, diff --git a/src/pk/asn1/der/custom_type/der_decode_custom_type.c b/src/pk/asn1/der/custom_type/der_decode_custom_type.c index 4c4d52d35..60f5054c7 100644 --- a/src/pk/asn1/der/custom_type/der_decode_custom_type.c +++ b/src/pk/asn1/der/custom_type/der_decode_custom_type.c @@ -47,6 +47,7 @@ int der_decode_custom_type_ex(const unsigned char *in, unsigned long inlen, int err, seq_err, i, ordered; ltc_asn1_type type; ltc_asn1_list ident; + unsigned int f; unsigned long size, x, y, z, blksize; unsigned char* in_new = NULL; void *data; @@ -63,7 +64,8 @@ int der_decode_custom_type_ex(const unsigned char *in, unsigned long inlen, LTC_ARGCHK(list != NULL); /* sequence type? We allow 0x30 SEQUENCE and 0x31 SET since fundamentally they're the same structure */ - if (in[x] != 0x30 && in[x] != 0x31) { + f = flags & ~(LTC_DER_SEQ_ALL_STRICT); + if (((f == LTC_DER_SEQ_SEQUENCE) && (in[x] != 0x30)) || (((f == LTC_DER_SEQ_SET) && (in[x] != 0x31)))) { return CRYPT_INVALID_PACKET; } ++x; @@ -116,7 +118,7 @@ int der_decode_custom_type_ex(const unsigned char *in, unsigned long inlen, } else { y = inlen - x; - if ((err = der_decode_asn1_length(&in[x], &y, &blksize)) != CRYPT_OK) { + if ((err = der_decode_asn1_length_ex(&in[x], &y, &blksize, flags)) != CRYPT_OK) { goto LBL_ERR; } x += y; diff --git a/src/pk/asn1/der/general/der_decode_asn1_length.c b/src/pk/asn1/der/general/der_decode_asn1_length.c index 0e5a3939d..c46e02d66 100644 --- a/src/pk/asn1/der/general/der_decode_asn1_length.c +++ b/src/pk/asn1/der/general/der_decode_asn1_length.c @@ -15,7 +15,7 @@ @param outlen [out] The decoded ASN.1 length @return CRYPT_OK if successful */ -int der_decode_asn1_length(const unsigned char *in, unsigned long *inlen, unsigned long *outlen) +int der_decode_asn1_length_ex(const unsigned char *in, unsigned long *inlen, unsigned long *outlen, unsigned int flags) { unsigned long real_len, decoded_len, offset, i; @@ -42,10 +42,17 @@ int der_decode_asn1_length(const unsigned char *in, unsigned long *inlen, unsign if (real_len > (*inlen - 1)) { return CRYPT_BUFFER_OVERFLOW; } + flags &= LTC_DER_SEQ_LEN_STRICT; decoded_len = 0; offset = 1 + real_len; for (i = 0; i < real_len; i++) { decoded_len = (decoded_len << 8) | in[1 + i]; + if ((flags == LTC_DER_SEQ_LEN_STRICT) && (decoded_len == 0)) { + return CRYPT_PK_ASN1_ERROR; + } + } + if ((flags == LTC_DER_SEQ_LEN_STRICT) && (real_len == 1) && (decoded_len < 128)) { + return CRYPT_PK_ASN1_ERROR; } } diff --git a/src/pk/ecc/ecc_verify_hash.c b/src/pk/ecc/ecc_verify_hash.c index 377b4d311..657934691 100644 --- a/src/pk/ecc/ecc_verify_hash.c +++ b/src/pk/ecc/ecc_verify_hash.c @@ -63,7 +63,7 @@ int ecc_verify_hash_ex(const unsigned char *sig, unsigned long siglen, if (sigformat == LTC_ECCSIG_ANSIX962) { /* ANSI X9.62 format - ASN.1 encoded SEQUENCE{ INTEGER(r), INTEGER(s) } */ - if ((err = der_decode_sequence_multi_ex(sig, siglen, LTC_DER_SEQ_SEQUENCE | LTC_DER_SEQ_STRICT, + if ((err = der_decode_sequence_multi_ex(sig, siglen, LTC_DER_SEQ_SEQUENCE | LTC_DER_SEQ_ALL_STRICT, LTC_ASN1_INTEGER, 1UL, r, LTC_ASN1_INTEGER, 1UL, s, LTC_ASN1_EOL, 0UL, NULL)) != CRYPT_OK) { goto error; }