From aa10345f171ad3d99e6a892ba9456043d436b7f8 Mon Sep 17 00:00:00 2001 From: HoundThe Date: Wed, 25 Aug 2021 14:01:27 +0200 Subject: [PATCH] Add signatureVerified flag for each signature (#994) * Add signatureVerified flag for each signature * Simplify condition * Remove checking for version == 1 when validating signatures * Modify plain signature presentation --- .../certificate_table/certificate_table.h | 1 + .../pe/authenticode/pkcs7_signature.cpp | 34 ++++++++----------- src/fileformat/file_format/pe/pe_format.cpp | 2 +- .../file_presentation/json_presentation.cpp | 2 ++ .../file_presentation/plain_presentation.cpp | 7 ++-- 5 files changed, 24 insertions(+), 22 deletions(-) diff --git a/include/retdec/fileformat/types/certificate_table/certificate_table.h b/include/retdec/fileformat/types/certificate_table/certificate_table.h index a6c279f36..99460dd9e 100644 --- a/include/retdec/fileformat/types/certificate_table/certificate_table.h +++ b/include/retdec/fileformat/types/certificate_table/certificate_table.h @@ -33,6 +33,7 @@ struct Signer /* naming - "Signature" was already taken by unpackers */ struct DigitalSignature { + bool isValid; std::string fileDigest; std::string signedDigest; std::string digestAlgorithm; diff --git a/src/fileformat/file_format/pe/authenticode/pkcs7_signature.cpp b/src/fileformat/file_format/pe/authenticode/pkcs7_signature.cpp index cea943168..791dc7743 100644 --- a/src/fileformat/file_format/pe/authenticode/pkcs7_signature.cpp +++ b/src/fileformat/file_format/pe/authenticode/pkcs7_signature.cpp @@ -106,7 +106,7 @@ Pkcs7Signature::Pkcs7Signature(const std::vector& input) noexcept { /* SignedData ::= SEQUENCE { - version Version, (Must be 1) + version Version, digestAlgorithms DigestAlgorithmIdentifiers, contentInfo ContentInfo, certificates @@ -203,7 +203,6 @@ Pkcs7Signature::SignerInfo::SignerInfo(const PKCS7* pkcs7, const PKCS7_SIGNER_IN si_info->enc_digest->data + si_info->enc_digest->length); ASN1_INTEGER_get_uint64(&version, si_info->version); - /* Version has to be equal to 1 */ serial = serialToString(si_info->issuer_and_serial->serial); issuer = X509NameToString(si_info->issuer_and_serial->issuer); @@ -360,7 +359,6 @@ const X509* Pkcs7Signature::SignerInfo::getSignerCert() const std::vector Pkcs7Signature::verify(const std::string& fileDigest) const { /* Check if signature is correctly parsed and complies with the spec: - - [x] Version is equal to 1 - [x] contentDigestAlgorithms contain single algorithm - [x] SignedData and SignerInfo digestAlgorithm match - [x] contentInfo contains PE hash, hashing algorithm and SpcIndirectDataOid @@ -384,10 +382,6 @@ std::vector Pkcs7Signature::verify(const std::string& fileDigest) c warnings.emplace_back("Invalid PKCS#7 type, expected SignedData"); } - if (version != 1) { - warnings.emplace_back("Signature version is: " + std::to_string(version) + ", expected 1"); - } - if (contentDigestAlgorithms.size() != 1) { warnings.emplace_back("Invalid number of DigestAlgorithmIdentifiers: " + std::to_string(contentDigestAlgorithms.size()) + " - expected 1"); } @@ -413,16 +407,12 @@ std::vector Pkcs7Signature::verify(const std::string& fileDigest) c if (!signerInfo->getSignerCert()) { warnings.emplace_back("Signing cert is missing"); } - if (signerInfo->version != 1) { - warnings.emplace_back("SignerInfo version is: " + std::to_string(signerInfo->version) + ", expected 1"); - } if (contentDigestAlgorithms.size() > 0 && signerInfo->digestAlgorithm != contentDigestAlgorithms[0]) { warnings.emplace_back("SignedData digest algorithm and signerInfo digest algorithm don't match"); } if (signerInfo->encryptDigest.empty()) { warnings.emplace_back("Encrypted digest is empty"); } - // verify auth attrs existence if (!signerInfo->spcInfo) { warnings.emplace_back("Couldn't get SpcSpOpusInfo"); @@ -534,6 +524,7 @@ std::vector Pkcs7Signature::getSignatures(const retdec::filefo /* No signer would mean, we have pretty much nothing */ if (!signerInfo.has_value()) { signatures.push_back(signature); + signature.isValid = false; return signatures; } @@ -556,6 +547,11 @@ std::vector Pkcs7Signature::getSignatures(const retdec::filefo signature.signer = signer; + // If there are warnings, then the signature is invalid + // We don't use the countersignature warnings here as + // previous implementation did when verifying + signature.isValid = signature.warnings.empty(); + for (auto&& counterSig : signInfo.counterSignatures) { CertificateProcessor processor; auto certChain = processor.getChain(counterSig.signerCert, certs); @@ -563,11 +559,11 @@ std::vector Pkcs7Signature::getSignatures(const retdec::filefo Signer counterSigner; counterSigner.chain = fileformatCertChain; - counterSigner.signingTime = counterSig.signTime, + counterSigner.signingTime = counterSig.signTime; counterSigner.digest = bytesToHexString(counterSig.messageDigest.data(), - counterSig.messageDigest.size()), - counterSigner.digestAlgorithm = OBJ_nid2ln(counterSig.digestAlgorithm), - counterSigner.warnings = counterSig.verify(signerInfo->encryptDigest), + counterSig.messageDigest.size()); + counterSigner.digestAlgorithm = OBJ_nid2ln(counterSig.digestAlgorithm); + counterSigner.warnings = counterSig.verify(signerInfo->encryptDigest); counterSigner.counterSigners = std::vector(); signature.signer.counterSigners.push_back(counterSigner); @@ -580,11 +576,11 @@ std::vector Pkcs7Signature::getSignatures(const retdec::filefo Signer counterSigner; counterSigner.chain = fileformatCertChain; - counterSigner.signingTime = counterSig.signTime, + counterSigner.signingTime = counterSig.signTime; counterSigner.digest = bytesToHexString(counterSig.messageDigest.data(), - counterSig.messageDigest.size()), - counterSigner.digestAlgorithm = OBJ_nid2ln(counterSig.digestAlgorithm), - counterSigner.warnings = counterSig.verify(signerInfo->encryptDigest), + counterSig.messageDigest.size()); + counterSigner.digestAlgorithm = OBJ_nid2ln(counterSig.digestAlgorithm); + counterSigner.warnings = counterSig.verify(signerInfo->encryptDigest); counterSigner.counterSigners = std::vector(); signature.signer.counterSigners.push_back(counterSigner); diff --git a/src/fileformat/file_format/pe/pe_format.cpp b/src/fileformat/file_format/pe/pe_format.cpp index b3de60710..78ba5538c 100644 --- a/src/fileformat/file_format/pe/pe_format.cpp +++ b/src/fileformat/file_format/pe/pe_format.cpp @@ -1806,7 +1806,7 @@ void PeFormat::loadCertificates() auto certBytes = securityDir.getCertificate(0); authenticode::Authenticode authenticode(certBytes); - + certificateTable = new CertificateTable(authenticode.getSignatures(this)); } diff --git a/src/fileinfo/file_presentation/json_presentation.cpp b/src/fileinfo/file_presentation/json_presentation.cpp index 8f00afc97..2ef5a9d25 100644 --- a/src/fileinfo/file_presentation/json_presentation.cpp +++ b/src/fileinfo/file_presentation/json_presentation.cpp @@ -487,6 +487,8 @@ void WriteSigner(JsonPresentation::Writer& writer, const Signer& signer) void WriteSignature(JsonPresentation::Writer& writer, const DigitalSignature& signature) { writer.StartObject(); + writer.String("signatureVerified"); + writer.Bool(signature.isValid); writer.String("warnings"); writer.StartArray(); for (auto&& warn : signature.warnings) { diff --git a/src/fileinfo/file_presentation/plain_presentation.cpp b/src/fileinfo/file_presentation/plain_presentation.cpp index f948cc0d6..ee512197d 100644 --- a/src/fileinfo/file_presentation/plain_presentation.cpp +++ b/src/fileinfo/file_presentation/plain_presentation.cpp @@ -724,7 +724,7 @@ static void printCertificate(const Certificate& cert, int indent) Log::info() << std::string(indent, ' ') << "Valid until: " << cert.getValidUntil() << "\n"; Log::info() << std::string(indent, ' ') << "Signature Algorithm: " << cert.getSignatureAlgorithm() << "\n"; Log::info() << std::string(indent, ' ') << "Public Key Algorithm: " << cert.getPublicKeyAlgorithm() << "\n"; - Log::info() << std::string(indent, ' ') << "Public key: " << cert.getPublicKey() << ":\n"; + Log::info() << std::string(indent, ' ') << "Public key: " << cert.getPublicKey() << "\n"; } static void printCertificateChain(const std::vector& certs, int indent) @@ -756,6 +756,7 @@ static void printSigner(const Signer& signer, int indent) static void printSignature(const DigitalSignature& signature, int indent) { + Log::info() << std::string(indent, ' ') << "Is Valid: " << signature.isValid << "\n"; Log::info() << std::string(indent, ' ') << "Digest Algorithm: " << signature.digestAlgorithm << "\n"; Log::info() << std::string(indent, ' ') << "Signed Digest: " << signature.signedDigest << "\n"; Log::info() << std::string(indent, ' ') << "File Digest: " << signature.fileDigest << "\n"; @@ -772,7 +773,9 @@ void PlainPresentation::presentSignatures() const if (!table) { return; } - Log::info() << "Digital Signatures:\n"; + Log::info() << "\n"; + Log::info() << "Digital Signatures\n"; + Log::info() << "------------------\n\n"; int indent = 4; Log::info() << std::string(indent, ' ') << "Signature count: " << table->signatureCount() << "\n";