From f5092717cd2ca2752cbd5e00075e17c92b2ae895 Mon Sep 17 00:00:00 2001 From: aCaB Date: Thu, 12 Jan 2012 19:26:37 +0100 Subject: [PATCH] few fixmes on cat manager --- libclamav/asn1.c | 28 +++++++++++++--------------- libclamav/crtmgr.c | 35 ++++++++++++++++++++--------------- libclamav/crtmgr.h | 2 +- 3 files changed, 34 insertions(+), 31 deletions(-) diff --git a/libclamav/asn1.c b/libclamav/asn1.c index 9482b65f5..e29f105ce 100644 --- a/libclamav/asn1.c +++ b/libclamav/asn1.c @@ -747,6 +747,7 @@ static int asn1_parse_mscat(fmap_t *map, size_t offset, unsigned int size, crtmg unsigned int dsize, message_size, attrs_size; cli_crt_hashtype hashtype; SHA1Context ctx; + cli_crt *x509; int result; cli_dbgmsg("in asn1_parse_mscat\n"); @@ -835,22 +836,23 @@ static int asn1_parse_mscat(fmap_t *map, size_t offset, unsigned int size, crtmg if(dsize) break; if(newcerts.crts) { - cli_crt *x509 = newcerts.crts; + x509 = newcerts.crts; cli_dbgmsg("asn1_parse_mscat: %u new certificates collected\n", newcerts.items); while(x509) { cli_crt *parent = crtmgr_verify_crt(cmgr, x509); if(parent) { x509->codeSign &= parent->codeSign; x509->timeSign &= parent->timeSign; - if(crtmgr_add(cmgr, x509)) { - /* FIXME handle error */ - } + if(crtmgr_add(cmgr, x509)) + break; crtmgr_del(&newcerts, x509); x509 = newcerts.crts; continue; } x509 = x509->next; } + if(x509) + break; if(newcerts.items) cli_dbgmsg("asn1_parse_mscat: %u certificates did not verify\n", newcerts.items); crtmgr_free(&newcerts); @@ -1020,7 +1022,7 @@ static int asn1_parse_mscat(fmap_t *map, size_t offset, unsigned int size, crtmg cli_dbgmsg("asn1_parse_mscat: failed to read encryptedDigest\n"); break; } - if(crtmgr_verify_pkcs7(cmgr, issuer, serial, asn1.content, asn1.size, CLI_SHA1RSA, sha1, VRFY_CODE)) { + if(!(x509 = crtmgr_verify_pkcs7(cmgr, issuer, serial, asn1.content, asn1.size, CLI_SHA1RSA, sha1, VRFY_CODE))) { cli_dbgmsg("asn1_parse_mscat: pkcs7 signature verification failed\n"); break; } @@ -1193,6 +1195,10 @@ static int asn1_parse_mscat(fmap_t *map, size_t offset, unsigned int size, crtmg deep.size = 1; else if(deep.size) cli_dbgmsg("asn1_parse_mscat: extra data in countersignature signing-time\n"); + else if(sigdate < x509->not_before || sigdate > x509->not_after) { + cli_dbgmsg("asn1_parse_mscat: countersignature timestamp outside cert validity\n"); + deep.size = 1; + } break; } } @@ -1264,7 +1270,7 @@ static int asn1_parse_mscat(fmap_t *map, size_t offset, unsigned int size, crtmg cli_dbgmsg("asn1_parse_mscat: failed to read countersignature encryptedDigest\n"); break; } - if(crtmgr_verify_pkcs7(cmgr, issuer, serial, asn1.content, asn1.size, hashtype, sha1, VRFY_TIME)) { + if(!crtmgr_verify_pkcs7(cmgr, issuer, serial, asn1.content, asn1.size, hashtype, sha1, VRFY_TIME)) { cli_dbgmsg("asn1_parse_mscat: pkcs7 countersignature verification failed\n"); break; } @@ -1279,7 +1285,6 @@ static int asn1_parse_mscat(fmap_t *map, size_t offset, unsigned int size, crtmg int asn1_load_mscat(fmap_t *map, struct cl_engine *engine) { struct cli_asn1 c; - char *virname; unsigned int size; struct cli_matcher *db; int i; @@ -1406,11 +1411,6 @@ int asn1_load_mscat(fmap_t *map, struct cl_engine *engine) { sprintf(&sha1[i*2], "%02x", ((uint8_t *)(tagval3.content))[i]); cli_dbgmsg("asn1_load_mscat: got hash %s (%s)\n", sha1, (hashtype == 2) ? "PE" : "CAB"); } - /* FIXME might as well use a static buf */ - virname = cli_mpool_virname(engine->mempool, "CAT", 1); - if(!virname) - return 1; - if(!engine->hm_fp) { if(!(engine->hm_fp = mpool_calloc(engine->mempool, 1, sizeof(*db)))) { tag.size = 1;; @@ -1420,10 +1420,8 @@ int asn1_load_mscat(fmap_t *map, struct cl_engine *engine) { engine->hm_fp->mempool = engine->mempool; #endif } - - if(hm_addhash_bin(engine->hm_fp, tagval3.content, CLI_HASH_SHA1, hashtype, virname)) { + if(hm_addhash_bin(engine->hm_fp, tagval3.content, CLI_HASH_SHA1, hashtype, NULL)) { cli_warnmsg("asn1_load_mscat: failed to add hash\n"); - mpool_free(engine->mempool, (void *)virname); return 1; } } diff --git a/libclamav/crtmgr.c b/libclamav/crtmgr.c index 0f205fad1..94a5a0174 100644 --- a/libclamav/crtmgr.c +++ b/libclamav/crtmgr.c @@ -87,6 +87,7 @@ int crtmgr_add(crtmgr *m, cli_crt *x509) { i->certSign |= x509->certSign; i->codeSign |= x509->codeSign; i->timeSign |= x509->timeSign; + return 0; } } @@ -260,39 +261,45 @@ static int crtmgr_rsa_verify(cli_crt *x509, mp_int *sig, cli_crt_hashtype hashty cli_crt *crtmgr_verify_crt(crtmgr *m, cli_crt *x509) { - cli_crt *i = m->crts; + cli_crt *i = m->crts, *best = NULL; + int score = 0; for(i = m->crts; i; i = i->next) { if(i->certSign && - (x509->codeSign & i->codeSign) == x509->codeSign && - (x509->timeSign & i->timeSign) == x509->timeSign && !memcmp(i->subject, x509->issuer, sizeof(i->subject)) && - !crtmgr_rsa_verify(i, &x509->sig, x509->hashtype, x509->tbshash)) - return i; + !crtmgr_rsa_verify(i, &x509->sig, x509->hashtype, x509->tbshash)) { + int curscore; + if((x509->codeSign & i->codeSign) == x509->codeSign && (x509->timeSign & i->timeSign) == x509->timeSign) + return i; + curscore = (x509->codeSign & i->codeSign) + (x509->timeSign & i->timeSign); + if(curscore > score) { + best = i; + score = curscore; + } + } } - return NULL; + return best; } -int crtmgr_verify_pkcs7(crtmgr *m, const uint8_t *issuer, const uint8_t *serial, const void *signature, unsigned int signature_len, cli_crt_hashtype hashtype, const uint8_t *refhash, cli_vrfy_type vrfytype) { +cli_crt *crtmgr_verify_pkcs7(crtmgr *m, const uint8_t *issuer, const uint8_t *serial, const void *signature, unsigned int signature_len, cli_crt_hashtype hashtype, const uint8_t *refhash, cli_vrfy_type vrfytype) { cli_crt *i; mp_int sig; int ret; if(signature_len < 1024/8 || signature_len > 4096/8+1) { cli_dbgmsg("crtmgr_verify_pkcs7: unsupported sig len: %u\n", signature_len); - return 1; + return NULL; } if((ret = mp_init(&sig))) { cli_errmsg("crtmgr_verify_pkcs7: mp_init failed with %d\n", ret); - return 1; + return NULL; } if((ret=mp_read_unsigned_bin(&sig, signature, signature_len))) { cli_warnmsg("crtmgr_verify_pkcs7: mp_read_unsigned_bin failed with %d\n", ret); - return 1; + return NULL; } - ret = 1; for(i = m->crts; i; i = i->next) { if(vrfytype == VRFY_CODE && !i->codeSign) continue; @@ -300,13 +307,11 @@ int crtmgr_verify_pkcs7(crtmgr *m, const uint8_t *issuer, const uint8_t *serial, continue; if(!memcmp(i->issuer, issuer, sizeof(i->issuer)) && !memcmp(i->serial, serial, sizeof(i->serial)) && - !crtmgr_rsa_verify(i, &sig, hashtype, refhash)) { - ret = 0; + !crtmgr_rsa_verify(i, &sig, hashtype, refhash)) break; - } } mp_clear(&sig); - return ret; + return i; } /* DC=com, DC=microsoft, CN=Microsoft Root Certificate Authority */ diff --git a/libclamav/crtmgr.h b/libclamav/crtmgr.h index e4407a8cd..a8d9d9e6c 100644 --- a/libclamav/crtmgr.h +++ b/libclamav/crtmgr.h @@ -61,7 +61,7 @@ int crtmgr_add(crtmgr *m, cli_crt *x509); cli_crt *crtmgr_lookup(crtmgr *m, cli_crt *x509); void crtmgr_del(crtmgr *m, cli_crt *x509); cli_crt *crtmgr_verify_crt(crtmgr *m, cli_crt *x509); -int crtmgr_verify_pkcs7(crtmgr *m, const uint8_t *issuer, const uint8_t *serial, const void *signature, unsigned int signature_len, cli_crt_hashtype hashtype, const uint8_t *refhash, cli_vrfy_type vrfytype); +cli_crt *crtmgr_verify_pkcs7(crtmgr *m, const uint8_t *issuer, const uint8_t *serial, const void *signature, unsigned int signature_len, cli_crt_hashtype hashtype, const uint8_t *refhash, cli_vrfy_type vrfytype); int crtmgr_add_roots(crtmgr *m);