From fe85bcd52b68475567ba9e2551e67fcf7e986606 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 fc196d01a..246e59d11 100644 --- a/src/headers/tomcrypt_pk.h +++ b/src/headers/tomcrypt_pk.h @@ -539,9 +539,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 0d4842e41..810647151 100644 --- a/src/headers/tomcrypt_private.h +++ b/src/headers/tomcrypt_private.h @@ -311,7 +311,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 17e24b6ab..29666fbdc 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 @@ -53,6 +53,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; @@ -69,7 +70,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; @@ -122,7 +124,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 bf8595dee..2dd620e17 100644 --- a/src/pk/asn1/der/general/der_decode_asn1_length.c +++ b/src/pk/asn1/der/general/der_decode_asn1_length.c @@ -21,7 +21,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; @@ -48,10 +48,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 ec9e7f101..3b644d6fe 100644 --- a/src/pk/ecc/ecc_verify_hash.c +++ b/src/pk/ecc/ecc_verify_hash.c @@ -69,7 +69,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; }