Trusted SHA256-based Authenticode hashes can now be loaded in
from .cat files. In addition:
- Files that are covered by Authenticode hashes loaded in from
.cat files will now be treated as VERIFIED like executables
where the embedded Authenticode sig is deemed to be trusted
based on .crb rules. This fixes a regression introduced in
0.102 (I think).
- The Authenticode hashes for signed EXEs without .crb coverage
will no longer be computed in cli_check_auth_header unless
hashes from .cat rules have been loaded. This fixes a slight
performance regression introduced in 0.102 (I think).
Improvements to use modern block list and allow list verbiage.
blacklist -> block list
whitelist -> allow listed
blacklisted -> blocked
whitelisted -> allowed
In the case of certificate verification, use "trust" or "verify" when
something is allowed.
Also changed domainlist -> domain list (or DomainList) to match.
Certs can omit the boolean field in the Basic Constraints section,
since the RFC specifies a default value for this field. This fixes
the following error:
LibClamAV debug: asn1_expect_objtype: expected type 01, got 02
LibClamAV debug: asn1_get_x509: An error occurred when parsing x509 extensions
LibClamAV debug: asn1_parse_mscat: skipping x509 certificate with errors
Ex: 05de45fd6a406dc147a4c8040a54eee947cd6eba02f28c0279ffd1a229e17130
Allow UTCDate fields in x509 certs to omit the seconds. Technically
this is disallowed by RFC5280, but Windows Authenticode verification
routines don't seem to mind it, so we'll allow it too. This fixes
the following error:
LibClamAV debug: asn1_getnum: expecting digits, found 'Z'
LibClamAV debug: asn1_get_time: invalid second 4294967295
LibClamAV debug: asn1_get_x509: unable to extract the notBefore time
LibClamAV debug: asn1_parse_mscat: skipping x509 certificate with errors
Ex: d577010638f208ad8b6dab1a33dc583b2ec6b0c719021fb9f083dd595ede27e8
Also, add a check on CRT_RAWMAXLEN, since if it's > 256 problems
will arise
Fixes:
==123806== Conditional jump or move depends on uninitialised value(s)
==123806== at 0x50C4A65: crtmgr_whitelist_lookup (crtmgr.c:107)
==123806== by 0x50C4F36: crtmgr_lookup (crtmgr.c:161)
==123806== by 0x50CC003: asn1_get_x509 (asn1.c:1053)
...
==123806== Uninitialised value was created by a stack allocation
==123806== at 0x50CA335: asn1_get_x509 (asn1.c:723)
hashtype and issuer were not getting set prior to the check
for duplicates when processing embedded certs, which means
some certs that were actually duplicates could have been added
multiple times to the list of trusted certs based on the
contents of the unitialized memory backing those (harmless,
but not as efficient).
Also, move the cert-related DCONF cfg checks to more
appropriate locations. One change in behavior:
PE_CONF_CATALOG will disable loading trusted hashes from
.cat files, but won't disable Authenticode hash checking
completely (PE_CONF_CERTS does this).
Instead of checking the Authenticode header as an FP prevention
mechanism, we now check it in the beginning if it exists. Also,
we can now do actual blacklisting with .crb rules (previously, a
blacklist rule just let you override a whitelist rule).
Consolidate the PE parsing code into one function. I tried to preserve all existing functionality from the previous, distinct implementations to a large extent (with the exceptions mentioned below). If I noticed potential bugs/improvements, I added a TODO statement about those so that they can be fixed in a smaller commit later. Also, there are more TODOs in places where I'm not entirely sure why certain actions are performed - more research is needed for these.
I'm submitting a pull request now so that regression testing can be done, and because merging what I have thus far now will likely have fewer conflicts than if I try to merge later
PE parsing code improvements:
- PEs without all 16 data directories are parsed more appropriately now
- Added lots more debug statements
Also:
- Allow MAX_BC and MAX_TRACKED_PCRE to be specified via CFLAGS
When doing performance testing with the latest CVD, MAX_BC and
MAX_TRACKED_PCRE need to be raised to track all the events.
Allow these to be specified via CFLAGS by not redefining them
if they are already defined
- Fix an issue preventing wildcard sizes in .MDB/.MSB rules
I'm not sure what the original intent of the check I removed was,
but it prevents using wildcard sizes in .MDB/.MSB rules. AFAICT
these wildcard sizes should be handled appropriately by the MD5
section hash computation code, so I don't think a check on that
is needed.
- Fix several issues related to db loading
- .imp files will now get loaded if they exist in a directory passed
via clamscan's '-d' flag
- .pwdb files will now get loaded if they exist in a directory passed
via clamscan's '-d' flag even when compiling without yara support
- Changes to .imp, .ign, and .ign2 files will now be reflected in calls
to cl_statinidir and cl_statchkdir (and also .pwdb files, even when
compiling without yara support)
- The contents of .sfp files won't be included in some of the signature
counts, and the contents of .cud files will be
- Any local.gdb files will no longer be loaded twice
- For .imp files, you are no longer required to specify a minimum flevel for wildcard rules, since this isn't needed
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)
This commit adds back in support for whitelisting files based on
signatures from .cat files loaded in via a '-d' flag to clamscan.
This also makes it so that a .crb blacklist rule match can't be
overruled by a signature in a .cat file
This commit makes the following changes:
- --dumpcerts will print certificates even if they already exist
in any .crb files loaded
- --dumpcerts will print certificates only once
- Having a whitelist CRB rule on a leaf certificate should no longer
prevent signature verification from happening. NOTE, this doesn't
mean that you can have whitelist rules for leaf certificates and have
that result in a trusted signature - that doesn't work yet
- Determining whether a certificate is blacklisted now includes comparing
the public key data (modulus and exponent) in addition to the subject
and serial hashes
- If a blacklisted certificate is detected, the code will return
immediately instead of continuing on to parse the rest of the signature
There are some Windows binaries that have certificates with version 1
TBSCertificate sections. This technically isn't allowed by the spec,
but the Windows API still seems to report these are being OK
In an earlier commit, I mistakenly check for whether a nested signature has
been seen when determining whether a countersignature is present instead of
checking that the countersignature has been seen
We used to get a pointer to file data without locking and for some samples
this pointer would be invalidated by the time we used it. Now, we just
store the offset for the sections that should be hashed as part of the
Authenticode hash computation and get the file data pointer right before
it's needed.
The diff is confusing, but basically I moved the countersignature
verification code into it's own function and then in asn1_parse_mscat
we now loop through the unauthenticatedAttributes to find the
counterSignature attribute (instead of assuming it's the first
attribute in the list.)
We also now do time-validation in the case where an unauthAttrs
section exists but doesn't include a counterSignature
If no unauthenticatedAttributes sections exist, the code will now judge
validity based on whether the code signing certificate is valid at the
time of the scan.
In my sample set of 2,000 signed binaries, there were 69 with x509
certificates included that didn't seem to comply with the spec. These
weren't in the actual certificate chain used to verify the binary,
though, and the Windows verification API had no problems with it, so
we shouldn't either. The specific errors varied. Specifically:
- 54 - expected NULL following RSA OID - For some
binaries this was due to an old "DUMMY CERTIFICATE" included
for some reason.
- 8 - module has got an unsupported length (392) - Binaries from
one company include 392-bit RSA keys for some reason
- 7 - expected [0] version container in TBSCertificate - Some
really older certificates don't seem to include the version
number (maybe the RFC didn't include one at the time?)
This doesn't add support to actually verify whitelisting rules
against SHA384 signatures, but makes it so that verification
doesn't fail completely if there is a SHA384 certificate somewhere
in the signature.
In the case where nested signatures are present, we still don't parse out
the nested signatures, but now signature verification based on the
non-nested signatures can continue
Everything should be working, but I'm having a hard time finding a binary
to test with that doesn't encounter other parsing issues (no countersignature,
extra data in the unauthenticatedAttributes section, etc.)
Andrew