Browse code

Allow the NULL to be missing on all AlgorithmIds

Some of the MS samples previously covered by ClamAV have
AlgorithmIdentifiers that omit the (required) NULL byte, and I
had changed the code to make this a hard requirement in some
places. Now we allow this is in all cases.

Also, I simplified the countersignature parsing code so that
any valid RSA OID is supported in the digestEncryptionAlgorithm
field... This makes the code cleaner and should avoid any
future variations from the specification (if SHA1RSA is an
acceptable value to pass, SHA256RSA probably is too)

Andrew authored on 2018/10/10 03:00:28
Showing 1 changed files
... ...
@@ -333,6 +333,10 @@ static int asn1_expect_algo(fmap_t *map, const void **asn1data, unsigned int *as
333 333
 
334 334
     if((ret = asn1_expect_obj(map, &obj.content, &avail, ASN1_TYPE_OBJECT_ID, algo_size, algo))) /* ALGO */
335 335
         return ret;
336
+
337
+    // The specification says that the NULL is a required parameter for this
338
+    // data type, but in practice it doesn't always exist in the ASN1. If
339
+    // there is something after the ALGO OID, assume it's the NULL
336 340
     if(avail && (ret = asn1_expect_obj(map, &obj.content, &avail, ASN1_TYPE_NULL, 0, NULL))) { /* NULL */
337 341
         cli_dbgmsg("asn1_expect_algo: expected NULL after AlgorithmIdentifier OID\n");
338 342
         return ret;
... ...
@@ -406,7 +410,10 @@ static int asn1_expect_hash_algo(fmap_t *map, const void **asn1data, unsigned in
406 406
         cli_dbgmsg("asn1_expect_hash_algo: unknown digest OID in AlgorithmIdentifier\n");
407 407
         return 1;
408 408
     }
409
-    if(ret = asn1_expect_obj(map, &obj.next, &avail, ASN1_TYPE_NULL, 0, NULL)) {
409
+    // The specification says that the NULL is a required parameter for this
410
+    // data type, but in practice it doesn't always exist in the ASN1. If
411
+    // there is something after the ALGO OID, assume it's the NULL
412
+    if(avail && (ret = asn1_expect_obj(map, &obj.next, &avail, ASN1_TYPE_NULL, 0, NULL))) {
410 413
         cli_dbgmsg("asn1_expect_hash_algo: expected NULL after AlgorithmIdentifier OID\n");
411 414
         return ret;
412 415
     }
... ...
@@ -496,7 +503,10 @@ static int asn1_expect_rsa(fmap_t *map, const void **asn1data, unsigned int *asn
496 496
         cli_dbgmsg("asn1_expect_rsa: OID mismatch (size %u)\n", obj.size);
497 497
         return 1;
498 498
     }
499
-    if((ret = asn1_expect_obj(map, &obj.next, &avail, ASN1_TYPE_NULL, 0, NULL))) { /* NULL */
499
+    // The specification says that the NULL is a required parameter for this
500
+    // data type, but in practice it doesn't always exist in the ASN1. If
501
+    // there is something after the ALGO OID, assume it's the NULL
502
+    if(avail && (ret = asn1_expect_obj(map, &obj.next, &avail, ASN1_TYPE_NULL, 0, NULL))) { /* NULL */
500 503
         cli_dbgmsg("asn1_expect_rsa: expected NULL following RSA OID\n");
501 504
         return ret;
502 505
     }
... ...
@@ -621,8 +631,10 @@ static int asn1_get_rsa_pubkey(fmap_t *map, const void **asn1data, unsigned int
621 621
     *asn1data = obj.next;
622 622
 
623 623
     avail = obj.size;
624
-    if(asn1_expect_algo(map, &obj.content, &avail, lenof(OID_rsaEncryption), OID_rsaEncryption)) /* rsaEncryption */
624
+    if(asn1_expect_algo(map, &obj.content, &avail, lenof(OID_rsaEncryption), OID_rsaEncryption)) { /* rsaEncryption */
625
+       cli_dbgmsg("asn1_get_rsa_pubkey: AlgorithmIdentifier other than RSA not yet supported\n");
625 626
        return 1;
627
+    }
626 628
 
627 629
     if(asn1_expect_objtype(map, obj.content, &avail, &obj, ASN1_TYPE_BIT_STRING)) /* BIT STRING - subjectPublicKey */
628 630
         return 1;
... ...
@@ -828,8 +840,10 @@ static int asn1_get_x509(fmap_t *map, const void **asn1data, unsigned int *size,
828 828
             break;
829 829
         if(map_sha1(map, obj.content, obj.size, x509.subject))
830 830
             break;
831
-        if(asn1_get_rsa_pubkey(map, &obj.next, &tbs.size, &x509)) /* subjectPublicKeyInfo */
831
+        if(asn1_get_rsa_pubkey(map, &obj.next, &tbs.size, &x509)) { /* subjectPublicKeyInfo */
832
+            cli_dbgmsg("asn1_get_x509: failed to get RSA public key\n");
832 833
             break;
834
+        }
833 835
 
834 836
         if (1 == version && tbs.size) {
835 837
             cli_dbgmsg("asn1_get_x509: TBSCertificate should not contain fields beyond subjectPublicKeyInfo if version == 1\n");
... ...
@@ -1094,6 +1108,7 @@ static int asn1_parse_countersignature(fmap_t *map, const void **asn1data, unsig
1094 1094
     unsigned int avail;
1095 1095
     uint8_t hash[MAX_HASH_SIZE];
1096 1096
     cli_crt_hashtype hashtype;
1097
+    cli_crt_hashtype hashtype2;
1097 1098
     unsigned int hashsize;
1098 1099
     uint8_t md[MAX_HASH_SIZE];
1099 1100
     int result;
... ...
@@ -1287,33 +1302,18 @@ static int asn1_parse_countersignature(fmap_t *map, const void **asn1data, unsig
1287 1287
             break;
1288 1288
         }
1289 1289
 
1290
-        if(asn1_expect_objtype(map, asn1.next, &avail, &asn1, ASN1_TYPE_SEQUENCE)) { /* digestEncryptionAlgorithm == sha1 */
1291
-            cli_dbgmsg("asn1_parse_countersignature: expected to parse SEQUENCE after counterSignature attributes\n");
1292
-            break;
1293
-        }
1294
-        if(asn1_expect_objtype(map, asn1.content, &asn1.size, &deep, ASN1_TYPE_OBJECT_ID)) {/* digestEncryptionAlgorithm == sha1 */
1295
-            cli_dbgmsg("asn1_parse_countersignature: unexpected value when parsing counterSignature digestEncryptionAlgorithm\n");
1296
-            break;
1297
-        }
1298
-        if(deep.size != lenof(OID_rsaEncryption)) { /* lenof(OID_rsaEncryption) = lenof(OID_sha1WithRSAEncryption) = 9 */
1299
-            cli_dbgmsg("asn1_parse_countersignature: wrong digestEncryptionAlgorithm size in countersignature\n");
1300
-            break;
1301
-        }
1302
-        if(!fmap_need_ptr_once(map, deep.content, lenof(OID_rsaEncryption))) {
1303
-            cli_dbgmsg("asn1_parse_countersignature: cannot read digestEncryptionAlgorithm in countersignature\n");
1304
-            break;
1305
-        }
1306
-        /* rsaEncryption or sha1withRSAEncryption */
1307
-        if(memcmp(deep.content, OID_rsaEncryption, lenof(OID_rsaEncryption)) && memcmp(deep.content, OID_sha1WithRSAEncryption, lenof(OID_sha1WithRSAEncryption))) {
1308
-            cli_dbgmsg("asn1_parse_countersignature: digestEncryptionAlgorithm in countersignature is not sha1\n");
1309
-            break;
1310
-        }
1311
-        if(asn1.size && asn1_expect_obj(map, &deep.next, &asn1.size, ASN1_TYPE_NULL, 0, NULL)) {
1312
-            cli_dbgmsg("asn1_parse_countersignature: expected NULL after counterSignature digestEncryptionAlgorithm OID\n");
1290
+        // TODO For some reason there tends to be more variability here than
1291
+        // when parsing the regular signature - we have to support at least
1292
+        // szOID_RSA_RSA and szOID_RSA_SHA1RSA based on samples seen in the
1293
+        // wild.  The spec says this should only be the RSA and DSA OIDs,
1294
+        // though.
1295
+        if (asn1_expect_rsa(map, &asn1.next, &avail, &hashtype2)) {
1296
+            cli_dbgmsg("asn1_parse_countersignature: unable to parse the digestEncryptionAlgorithm\n");
1313 1297
             break;
1314 1298
         }
1315
-        if(asn1.size) {
1316
-            cli_dbgmsg("asn1_parse_countersignature: extra data in digestEncryptionAlgorithm in countersignature\n");
1299
+
1300
+        if (hashtype2 != CLI_RSA && hashtype2 != hashtype) {
1301
+            cli_dbgmsg("asn1_parse_countersignature: digestEncryptionAlgorithm conflicts with digestAlgorithm\n");
1317 1302
             break;
1318 1303
         }
1319 1304
 
... ...
@@ -2142,8 +2142,10 @@ int asn1_load_mscat(fmap_t *map, struct cl_engine *engine) {
2142 2142
                 return 1;
2143 2143
             }
2144 2144
 
2145
-            if(asn1_expect_algo(map, &tagval2.content, &tagval2.size, lenof(OID_sha1), OID_sha1)) /* objid 1.3.14.3.2.26 - sha1 */
2145
+            if(asn1_expect_algo(map, &tagval2.content, &tagval2.size, lenof(OID_sha1), OID_sha1)) { /* objid 1.3.14.3.2.26 - sha1 */
2146
+                cli_dbgmsg("asn1_load_mscat: currently only SHA1 hashes are supported for .cat file signatures\n");
2146 2147
                 return 1;
2148
+            }
2147 2149
 
2148 2150
             if(asn1_expect_objtype(map, tagval2.content, &tagval2.size, &tagval3, ASN1_TYPE_OCTET_STRING))
2149 2151
                 return 1;