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

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;

src/fileformat/file_format/pe/authenticode/pkcs7_signature.cpp

@@ -106,7 +106,7 @@ Pkcs7Signature::Pkcs7Signature(const std::vector<std::uint8_t>& 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<std::string> 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<std::string> 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<std::string> 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<DigitalSignature> 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,18 +547,23 @@ std::vector<DigitalSignature> 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);
 		auto fileformatCertChain = convertToFileformatCertChain(certChain);
 
 		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<Signer>();
 
 		signature.signer.counterSigners.push_back(counterSigner);
@@ -580,11 +576,11 @@ std::vector<DigitalSignature> 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<Signer>();
 
 		signature.signer.counterSigners.push_back(counterSigner);

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));
 }
 

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) {

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<Certificate>& 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";