Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add signatureVerified flag for each signature #994

Merged
merged 4 commits into from
Aug 25, 2021

Conversation

HoundThe
Copy link
Member

Add a flag for each signature that represents if the signature and its signer were successfully verified (digest matches etc.) in the same manner as the previous implementation.

Copy link
Member

@metthal metthal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

General question, does warnings reflect also discrepancies like hash of the file does not match the hash in the signature, the signature of the hash is not the same as the one in authenticated attributes, ...? What about expiration? Previously we neglected the expiration dates as far as I remember and would consider the signature valid because if the hash matches and the signature is expired, we can't reliably tell that whether the signature is OK without verification in the whole chain and that's something we didn't want to do and we just stick with what we can verify statically. I would like to have a list of possible warnings and when can the warning occur before we proceed with this.

@HoundThe
Copy link
Member Author

HoundThe commented Aug 4, 2021

Expiration is not looked at, the only part regarding the certificate chain that is checked is the existence of the signing certificate.

Regarding the possible warnings, I'll split them into 2 parts - warnings regarding a signature and warnings regarding counter-signatures.

Signature warnings (PKCS7 structure):

  • Couldn't parse the Pkcs7 signature"- Situation where the openssl library fails to parse the signature
  • Wrong version
  • Invalid number of DigestAlgorithmIdentifiers
  • No contentInfo (structure where the actual signature digest is stored)
  • Wrong contentInfo type
  • Missing signature digest value
  • Signature digest doesn't match the file digest
  • No signerInfo
  • No signer certificate
  • Wrong signerInfo version
  • Mismatching signerInfo and signature digest algorithms
  • No encrypted digest
  • No message digest
  • Missing SpcSpOpusInfo structure
  • Failure to verify the signature (decrypting encrypted digest and matching the digest - done by openssl PKCS7_signatureVerify)

CounterSignature warnings:

  • Couldn't parse counter-signature.
  • Missing signing certificate
  • Missing content type (which again contains the digest)
  • Failed to verify the counter-signature - calculating hash and comparing with the decrypted encrypted hash
  • Unknown digest algorithm
  • Failed to decrypt the digest
  • Missing digest
  • Failed to verify the signature with counter-signature - when the hash in the countersignature doesn't match with the hash of the countersigned signature.

@HoundThe
Copy link
Member Author

HoundThe commented Aug 5, 2021

I have also removed version != 1 warning because there are samples with different versions that Windows accepts and it's not reflected by the old Authenticode specification.

@PeterMatula
Copy link
Collaborator

lets run TC tests

@PeterMatula PeterMatula merged commit aa10345 into avast:master Aug 25, 2021
PeterMatula added a commit that referenced this pull request Aug 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants