add flags to `der_decode_sequence()`

src/headers/tomcrypt_pk.h

@@ -606,10 +606,32 @@ int der_encode_sequence_ex(ltc_asn1_list *list, unsigned long inlen,
 
 #define der_encode_sequence(list, inlen, out, outlen) der_encode_sequence_ex(list, inlen, out, outlen, LTC_ASN1_SEQUENCE)
 
+/** The supported bitmap for all the
+ * decoders with a `flags` argument.
+ */
+enum ltc_der_seq {
+   LTC_DER_SEQ_ZERO = 0x0u,
+
+   /** Bit0  - [0]=Unordered (SET or SETOF)
+    *          [1]=Ordered (SEQUENCE) */
+   LTC_DER_SEQ_UNORDERED = LTC_DER_SEQ_ZERO,
+   LTC_DER_SEQ_ORDERED = 0x1u,
+
+   /** Bit1  - [0]=Relaxed
+    *          [1]=Strict */
+   LTC_DER_SEQ_RELAXED = LTC_DER_SEQ_ZERO,
+   LTC_DER_SEQ_STRICT = 0x2u,
+
+   /** Alternative naming */
+   LTC_DER_SEQ_SET = LTC_DER_SEQ_UNORDERED,
+   LTC_DER_SEQ_SEQUENCE = LTC_DER_SEQ_ORDERED,
+};
+
 int der_decode_sequence_ex(const unsigned char *in, unsigned long  inlen,
-                           ltc_asn1_list *list,     unsigned long  outlen, int ordered);
+                           ltc_asn1_list *list,     unsigned long  outlen, unsigned int flags);
 
-#define der_decode_sequence(in, inlen, list, outlen) der_decode_sequence_ex(in, inlen, list, outlen, 1)
+#define der_decode_sequence(in, inlen, list, outlen) der_decode_sequence_ex(in, inlen, list, outlen, LTC_DER_SEQ_SEQUENCE | LTC_DER_SEQ_RELAXED)
+#define der_decode_sequence_strict(in, inlen, list, outlen) der_decode_sequence_ex(in, inlen, list, outlen, LTC_DER_SEQ_SEQUENCE | LTC_DER_SEQ_STRICT)
 
 int der_length_sequence(ltc_asn1_list *list, unsigned long inlen,
                         unsigned long *outlen);
@@ -647,7 +669,7 @@ extern const unsigned long der_asn1_type_to_identifier_map_sz;
 #endif /* LTC_SOURCE */
 
 /* SET */
-#define der_decode_set(in, inlen, list, outlen) der_decode_sequence_ex(in, inlen, list, outlen, 0)
+#define der_decode_set(in, inlen, list, outlen) der_decode_sequence_ex(in, inlen, list, outlen, LTC_DER_SEQ_SET)
 #define der_length_set der_length_sequence
 int der_encode_set(ltc_asn1_list *list, unsigned long inlen,
                    unsigned char *out,  unsigned long *outlen);
@@ -658,6 +680,10 @@ int der_encode_setof(ltc_asn1_list *list, unsigned long inlen,
 /* VA list handy helpers with triplets of <type, size, data> */
 int der_encode_sequence_multi(unsigned char *out, unsigned long *outlen, ...);
 int der_decode_sequence_multi(const unsigned char *in, unsigned long inlen, ...);
+#ifdef LTC_SOURCE
+/* internal helper functions */
+int der_decode_sequence_multi_ex(const unsigned char *in, unsigned long inlen, unsigned int flags, ...);
+#endif /* LTC_SOURCE */
 
 /* FLEXI DECODER handle unknown list decoder */
 int  der_decode_sequence_flexi(const unsigned char *in, unsigned long *inlen, ltc_asn1_list **out);

src/pk/asn1/der/sequence/der_decode_sequence_ex.c

@@ -22,13 +22,13 @@
    @param inlen    The size of the input
    @param list     The list of items to decode
    @param outlen   The number of items in the list
-   @param ordered  Search an unordeded or ordered list
+   @param flags    c.f. enum ltc_der_seq
    @return CRYPT_OK on success
 */
 int der_decode_sequence_ex(const unsigned char *in, unsigned long  inlen,
-                           ltc_asn1_list *list,     unsigned long  outlen, int ordered)
+                           ltc_asn1_list *list,     unsigned long  outlen, unsigned int flags)
 {
-   int           err, i;
+   int           err, seq_err, i, ordered;
    ltc_asn1_type type;
    unsigned long size, x, y, z, blksize;
    void          *data;
@@ -66,10 +66,12 @@ int der_decode_sequence_ex(const unsigned char *in, unsigned long  inlen,
    for (i = 0; i < (int)outlen; i++) {
        list[i].used = 0;
    }
+   ordered = flags & LTC_DER_SEQ_ORDERED;
 
    /* ok read data */
-   blksize = inlen;
-   inlen -= x;
+   seq_err  = CRYPT_OK;
+   blksize += x;
+   inlen   -= x;
    for (i = 0; i < (int)outlen; i++) {
        z    = 0;
        type = list[i].type;
@@ -258,7 +260,12 @@ int der_decode_sequence_ex(const unsigned char *in, unsigned long  inlen,
                }
 
                z = inlen;
-               if ((err = der_decode_sequence(in + x, z, data, size)) != CRYPT_OK) {
+               err = der_decode_sequence_ex(in + x, z, data, size, flags);
+               if (err == CRYPT_INPUT_TOO_LONG) {
+                  seq_err = CRYPT_INPUT_TOO_LONG;
+                  err = CRYPT_OK;
+               }
+               if (err != CRYPT_OK) {
                   if (!ordered || list[i].optional) { continue; }
                   goto LBL_ERR;
                }
@@ -306,8 +313,14 @@ int der_decode_sequence_ex(const unsigned char *in, unsigned long  inlen,
       }
    }
 
-   if (blksize == x) {
+   if (blksize == x && seq_err == CRYPT_OK && inlen == 0) {
+      /* everything decoded and no errors in nested sequences */
+      err = CRYPT_OK;
+   } else if (blksize == x && seq_err == CRYPT_INPUT_TOO_LONG && inlen == 0) {
+      /* a sequence reported too-long input, but now we've decoded everything */
       err = CRYPT_OK;
+   } else if (blksize != x && ((flags & LTC_DER_SEQ_RELAXED) != LTC_DER_SEQ_RELAXED)) {
+      err = CRYPT_INVALID_PACKET;
    } else {
       err = CRYPT_INPUT_TOO_LONG;
    }

src/pk/asn1/der/sequence/der_decode_sequence_multi.c

@@ -21,27 +21,27 @@
   Decode a SEQUENCE type using a VA list
   @param in    Input buffer
   @param inlen Length of input in octets
-  @remark <...> is of the form <type, size, data> (int, unsigned long, void*)
+  @param a1    Initialized argument list #1
+  @param a2    Initialized argument list #2 (copy of #1)
+  @param flags    c.f. enum ltc_der_seq
   @return CRYPT_OK on success
 */
-int der_decode_sequence_multi(const unsigned char *in, unsigned long inlen, ...)
+static int _der_decode_sequence_va(const unsigned char *in, unsigned long inlen, va_list a1, va_list a2, unsigned int flags)
 {
    int           err;
    ltc_asn1_type type;
    unsigned long size, x;
    void          *data;
-   va_list       args;
    ltc_asn1_list *list;
 
    LTC_ARGCHK(in    != NULL);
 
    /* get size of output that will be required */
-   va_start(args, inlen);
    x = 0;
    for (;;) {
-       type = (ltc_asn1_type)va_arg(args, int);
-       size = va_arg(args, unsigned long);
-       data = va_arg(args, void*);
+       type = (ltc_asn1_type)va_arg(a1, int);
+       size = va_arg(a1, unsigned long);
+       data = va_arg(a1, void*);
        LTC_UNUSED_PARAM(size);
        LTC_UNUSED_PARAM(data);
 
@@ -73,11 +73,9 @@ int der_decode_sequence_multi(const unsigned char *in, unsigned long inlen, ...)
 
            case LTC_ASN1_EOL:
            case LTC_ASN1_CUSTOM_TYPE:
-               va_end(args);
                return CRYPT_INVALID_ARG;
        }
    }
-   va_end(args);
 
    /* allocate structure for x elements */
    if (x == 0) {
@@ -90,12 +88,11 @@ int der_decode_sequence_multi(const unsigned char *in, unsigned long inlen, ...)
    }
 
    /* fill in the structure */
-   va_start(args, inlen);
    x = 0;
    for (;;) {
-       type = (ltc_asn1_type)va_arg(args, int);
-       size = va_arg(args, unsigned long);
-       data = va_arg(args, void*);
+       type = (ltc_asn1_type)va_arg(a2, int);
+       size = va_arg(a2, unsigned long);
+       data = va_arg(a2, void*);
 
        if (type == LTC_ASN1_EOL) {
           break;
@@ -128,13 +125,63 @@ int der_decode_sequence_multi(const unsigned char *in, unsigned long inlen, ...)
                 break;
        }
    }
-   va_end(args);
 
-   err = der_decode_sequence(in, inlen, list, x);
+   err = der_decode_sequence_ex(in, inlen, list, x, flags);
    XFREE(list);
    return err;
 }
 
+/**
+  Decode a SEQUENCE type using a VA list
+  @param in    Input buffer
+  @param inlen Length of input in octets
+  @remark <...> is of the form <type, size, data> (int, unsigned long, void*)
+  @return CRYPT_OK on success
+*/
+int der_decode_sequence_multi(const unsigned char *in, unsigned long inlen, ...)
+{
+   va_list       a1, a2;
+   int err;
+
+   LTC_ARGCHK(in    != NULL);
+
+   va_start(a1, inlen);
+   va_start(a2, inlen);
+
+   err = _der_decode_sequence_va(in, inlen, a1, a2, LTC_DER_SEQ_SEQUENCE | LTC_DER_SEQ_RELAXED);
+
+   va_end(a2);
+   va_end(a1);
+
+   return err;
+}
+
+/**
+  Decode a SEQUENCE type using a VA list
+  @param in    Input buffer
+  @param inlen Length of input in octets
+  @param flags c.f. enum ltc_der_seq
+  @remark <...> is of the form <type, size, data> (int, unsigned long, void*)
+  @return CRYPT_OK on success
+*/
+int der_decode_sequence_multi_ex(const unsigned char *in, unsigned long inlen, unsigned int flags, ...)
+{
+   va_list       a1, a2;
+   int err;
+
+   LTC_ARGCHK(in    != NULL);
+
+   va_start(a1, flags);
+   va_start(a2, flags);
+
+   err = _der_decode_sequence_va(in, inlen, a1, a2, flags);
+
+   va_end(a2);
+   va_end(a1);
+
+   return err;
+}
+
 #endif
 
 

src/pk/dsa/dsa_verify_hash.c

@@ -111,7 +111,7 @@ int dsa_verify_hash(const unsigned char *sig, unsigned long siglen,
    LTC_SET_ASN1(sig_seq, 0, LTC_ASN1_INTEGER, r, 1UL);
    LTC_SET_ASN1(sig_seq, 1, LTC_ASN1_INTEGER, s, 1UL);
 
-   err = der_decode_sequence(sig, siglen, sig_seq, 2);
+   err = der_decode_sequence_strict(sig, siglen, sig_seq, 2);
    if (err != CRYPT_OK) {
       goto LBL_ERR;
    }

src/pk/ecc/ecc_verify_hash.c

@@ -66,7 +66,7 @@ static int _ecc_verify_hash(const unsigned char *sig,  unsigned long siglen,
    }
    else {
       /* ASN.1 format */
-      if ((err = der_decode_sequence_multi(sig, siglen,
+      if ((err = der_decode_sequence_multi_ex(sig, siglen, LTC_DER_SEQ_SEQUENCE | LTC_DER_SEQ_STRICT,
                                      LTC_ASN1_INTEGER, 1UL, r,
                                      LTC_ASN1_INTEGER, 1UL, s,
                                      LTC_ASN1_EOL, 0UL, NULL)) != CRYPT_OK)                             { goto error; }

src/pk/rsa/rsa_verify_hash.c

@@ -142,10 +142,10 @@ int rsa_verify_hash_ex(const unsigned char *sig,      unsigned long siglen,
       LTC_SET_ASN1(siginfo,    0, LTC_ASN1_SEQUENCE,          digestinfo,                    2);
       LTC_SET_ASN1(siginfo,    1, LTC_ASN1_OCTET_STRING,      tmpbuf,                        siglen);
 
-      if ((err = der_decode_sequence(out, outlen, siginfo, 2)) != CRYPT_OK) {
+      if ((err = der_decode_sequence_strict(out, outlen, siginfo, 2)) != CRYPT_OK) {
          /* fallback to Legacy:missing NULL */
          LTC_SET_ASN1(siginfo, 0, LTC_ASN1_SEQUENCE,          digestinfo,                    1);
-         if ((err = der_decode_sequence(out, outlen, siginfo, 2)) != CRYPT_OK) {
+         if ((err = der_decode_sequence_strict(out, outlen, siginfo, 2)) != CRYPT_OK) {
            XFREE(out);
            goto bail_2;
          }

tests/der_test.c

@@ -1409,10 +1409,10 @@ static void _der_regression_test(void)
 
 static void der_toolong_test(void)
 {
-   int err, failed = 0;
+   int n, err, failed = 0;
    ltc_asn1_list *list;
-   unsigned long len;
-   unsigned char buf5[5], buf12[12];
+   unsigned long len, oid[16];
+   unsigned char buf5[5], buf12[12], buf32[32];
    static const unsigned char invalid1[] = {
          0x30,0x19, /* SEQUENCE len=25 bytes */
               0x30,0x0a, /* SEQUENCE len=10 bytes (which is wrong, should be 9) */
@@ -1432,8 +1432,35 @@ static void der_toolong_test(void)
               0x02,0x04, /* INTEGER len=4 */ 0x74,0x72,0x91,0xdd,
               0x00,0x00  /* garbage inside the sequence */
    };
+   static const unsigned char invalid4[] = {
+         0x30, 0x30,
+               0x30, 0x0d,
+                     0x06, 0x09, 0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x01,
+                     0x05, 0x00,
+               0x04, 0x20, 0x53, 0x2e, 0xaa, 0xbd, 0x95, 0x74, 0x88, 0x0d, 0xbf, 0x76, 0xb9, 0xb8, 0xcc, 0x00, 0x83, 0x2c,
+                           0x20, 0xa6, 0xec, 0x11, 0x3d, 0x68, 0x22, 0x99, 0x55, 0x0d, 0x7a, 0x6e, 0x0f, 0x34, 0x5e, 0x25
+
+   };
+   static const unsigned char invalid5[] = {
+          0x30, 0x31,
+                0x30, 0x0e,
+                      0x06, 0x09, 0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x01,
+                      0x05, 0x00,
+                0x04, 0x20, 0x53, 0x2e, 0xaa, 0xbd, 0x95,0x74, 0x88, 0x0d, 0xbf, 0x76, 0xb9, 0xb8, 0xcc,0x00, 0x83, 0x2c,
+                            0x20, 0xa6, 0xec, 0x11, 0x3d,0x68, 0x22, 0x99, 0x55, 0x0d, 0x7a, 0x6e, 0x0f,0x34, 0x5e, 0x25
+
+   };
+   static const unsigned char invalid6[] = {
+          0x30, 0x31,
+                0x30, 0x0c,
+                      0x06, 0x09, 0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x01,
+                      0x05, 0x00,
+                0x04, 0x20, 0x53, 0x2e, 0xaa, 0xbd, 0x95,0x74, 0x88, 0x0d, 0xbf, 0x76, 0xb9, 0xb8, 0xcc,0x00, 0x83, 0x2c,
+                            0x20, 0xa6, 0xec, 0x11, 0x3d,0x68, 0x22, 0x99, 0x55, 0x0d, 0x7a, 0x6e, 0x0f,0x34, 0x5e, 0x25
+
+   };
 
-   ltc_asn1_list seqsub[2], seqmain[2], seqint[2];
+   ltc_asn1_list seqsub[2], seqoid[2], seqmain[2], seqint[2];
    void *int1, *int2;
 
    LTC_SET_ASN1(seqsub,  0, LTC_ASN1_OCTET_STRING, buf5,   5);
@@ -1441,16 +1468,17 @@ static void der_toolong_test(void)
    LTC_SET_ASN1(seqmain, 0, LTC_ASN1_SEQUENCE,     seqsub, 2);
    LTC_SET_ASN1(seqmain, 1, LTC_ASN1_OCTET_STRING, buf12,  12);
 
+   n = 1;
    len = sizeof(invalid1);
-   err = der_decode_sequence(invalid1, len, seqmain, 2);
+   err = der_decode_sequence_strict(invalid1, len, seqmain, 2);
    if (err == CRYPT_OK) {
-      fprintf(stderr,"Sequence invalid%d accepted by der_decode_sequence\n", 1);
+      fprintf(stderr,"Sequence invalid%d accepted by der_decode_sequence\n", n);
       failed = 1;
    }
    len = sizeof(invalid1);
    err = der_decode_sequence_flexi(invalid1, &len, &list);
    if (err == CRYPT_OK) {
-      fprintf(stderr,"Sequence invalid%d accepted by der_decode_sequence_flexi\n", 1);
+      fprintf(stderr,"Sequence invalid%d accepted by der_decode_sequence_flexi\n", n);
       failed = 1;
       der_sequence_free(list);
    }
@@ -1459,10 +1487,11 @@ static void der_toolong_test(void)
    LTC_SET_ASN1(seqint,  0, LTC_ASN1_INTEGER,      int1,   1);
    LTC_SET_ASN1(seqint,  1, LTC_ASN1_INTEGER,      int2,   1);
 
+   n++;
    len = sizeof(invalid2);
-   err = der_decode_sequence(invalid2, len, seqint, 2);
+   err = der_decode_sequence_strict(invalid2, len, seqint, 2);
    if (err == CRYPT_OK) {
-      fprintf(stderr,"Sequence invalid%d accepted by der_decode_sequence\n", 2);
+      fprintf(stderr,"Sequence invalid%d accepted by der_decode_sequence\n", n);
       failed = 1;
    }
    len = sizeof(invalid2);
@@ -1475,21 +1504,72 @@ static void der_toolong_test(void)
    if (err == CRYPT_OK)
       der_sequence_free(list);
 
+   n++;
    len = sizeof(invalid3);
-   err = der_decode_sequence(invalid3, len, seqint, 2);
+   err = der_decode_sequence_strict(invalid3, len, seqint, 2);
    if (err == CRYPT_OK) {
-      fprintf(stderr,"Sequence invalid%d accepted by der_decode_sequence\n", 3);
+      fprintf(stderr,"Sequence invalid%d accepted by der_decode_sequence\n", n);
       failed = 1;
    }
    len = sizeof(invalid3);
    err = der_decode_sequence_flexi(invalid3, &len, &list);
    if (err == CRYPT_OK) {
-      fprintf(stderr,"Sequence invalid%d accepted by der_decode_sequence_flexi\n", 3);
+      fprintf(stderr,"Sequence invalid%d accepted by der_decode_sequence_flexi\n", n);
       failed = 1;
       der_sequence_free(list);
    }
 
    mp_clear_multi(int1, int2, NULL);
+
+   LTC_SET_ASN1(seqoid,  0, LTC_ASN1_OBJECT_IDENTIFIER, oid, sizeof(oid)/sizeof(oid[0]));
+   LTC_SET_ASN1(seqoid,  1, LTC_ASN1_NULL,              NULL,   0);
+   LTC_SET_ASN1(seqmain, 0, LTC_ASN1_SEQUENCE,          seqoid, 2);
+   LTC_SET_ASN1(seqmain, 1, LTC_ASN1_OCTET_STRING,      buf32,  32);
+
+   n++;
+   len = sizeof(invalid4);
+   err = der_decode_sequence_strict(invalid4, len, seqmain, 2);
+   if (err == CRYPT_OK) {
+      fprintf(stderr,"Sequence invalid%d accepted by der_decode_sequence\n", n);
+      failed = 1;
+   }
+   len = sizeof(invalid4);
+   err = der_decode_sequence_flexi(invalid4, &len, &list);
+   if (err == CRYPT_OK) {
+      fprintf(stderr,"Sequence invalid%d accepted by der_decode_sequence_flexi\n", n);
+      failed = 1;
+      der_sequence_free(list);
+   }
+
+   n++;
+   len = sizeof(invalid5);
+   err = der_decode_sequence_strict(invalid5, len, seqmain, 2);
+   if (err == CRYPT_OK) {
+      fprintf(stderr,"Sequence invalid%d accepted by der_decode_sequence\n", n);
+      failed = 1;
+   }
+   len = sizeof(invalid5);
+   err = der_decode_sequence_flexi(invalid5, &len, &list);
+   if (err == CRYPT_OK) {
+      fprintf(stderr,"Sequence invalid%d accepted by der_decode_sequence_flexi\n", n);
+      failed = 1;
+      der_sequence_free(list);
+   }
+   n++;
+   len = sizeof(invalid6);
+   err = der_decode_sequence_strict(invalid6, len, seqmain, 2);
+   if (err == CRYPT_OK) {
+      fprintf(stderr,"Sequence invalid%d accepted by der_decode_sequence\n", n);
+      failed = 1;
+   }
+   len = sizeof(invalid6);
+   err = der_decode_sequence_flexi(invalid6, &len, &list);
+   if (err == CRYPT_OK) {
+      fprintf(stderr,"Sequence invalid%d accepted by der_decode_sequence_flexi\n", n);
+      failed = 1;
+      der_sequence_free(list);
+   }
+
    if (failed) exit(EXIT_FAILURE);
 }
 

tests/dsa_test.c

@@ -306,7 +306,8 @@ static int _dsa_wycheproof_test(void)
    }
 
    stat = 666; /* intentionally not one, not zero */
-   DO(dsa_verify_hash(sig, sizeof(sig), hash, hashlen, &stat, &key));
+   DOX(dsa_verify_hash(sig, sizeof(sig), hash, hashlen, &stat, &key)
+               == CRYPT_INPUT_TOO_LONG ? CRYPT_OK:CRYPT_INVALID_PACKET, "should be too long");
    /* this should be invalid */
    if (stat != 0) {
       fprintf(stderr, "dsa_verify_hash did not reject invalid signature\n");

tests/rsa_test.c

@@ -733,11 +733,8 @@ print_hex("q", tmp, len);
 
      len3 = sizeof(tmp);
      /* (6) */
-     if (i < 8)
-       DOX(rsa_verify_hash_ex(p2, len2, p, 20, LTC_PKCS_1_V1_5, hash_idx, -1, &stat, &pubKey)
+     DOX(rsa_verify_hash_ex(p2, len2, p, 20, LTC_PKCS_1_V1_5, hash_idx, -1, &stat, &pubKey)
            == CRYPT_INVALID_PACKET ? CRYPT_OK:CRYPT_INVALID_PACKET, "should fail");
-     else
-       DOX(rsa_verify_hash_ex(p2, len2, p, 20, LTC_PKCS_1_V1_5, hash_idx, -1, &stat, &pubKey), "should succeed");
      DOX(stat == 0?CRYPT_OK:CRYPT_FAIL_TESTVECTOR, "should fail");
    }
    rsa_free(&key);