[poppler] poppler/Form.cc poppler/Form.h poppler/SignatureHandler.cc poppler/SignatureHandler.h qt5/src qt6/src

GitLab Mirror gitlab-mirror at kemper.freedesktop.org
Mon Mar 27 15:04:14 UTC 2023


 poppler/Form.cc             |    8 +-
 poppler/Form.h              |    4 -
 poppler/SignatureHandler.cc |  125 +++++++++++++++++++-------------------------
 poppler/SignatureHandler.h  |   64 ++++++++++++++--------
 qt5/src/poppler-form.cc     |    2 
 qt6/src/poppler-form.cc     |    2 
 6 files changed, 105 insertions(+), 100 deletions(-)

New commits:
commit 08c0766e9d7ab08cdc7ef380ce9c190ae87d789b
Author: Sune Vuorela <sune at vuorela.dk>
Date:   Fri Mar 3 08:22:13 2023 +0100

    Split signature handler for signing and verification
    
    Only a little bit of the class state was shared between the signing
    and verification functions, so split it in the shared hashing bits
    and a class for signing and one for verification
    
    Also clean up a little bit of the memory handling

diff --git a/poppler/Form.cc b/poppler/Form.cc
index 3f4b0181..2c0c35a4 100644
--- a/poppler/Form.cc
+++ b/poppler/Form.cc
@@ -575,7 +575,7 @@ SignatureInfo *FormWidgetSignature::validateSignature(bool doVerifyCert, bool fo
 
 #ifdef ENABLE_NSS3
 // update hash with the specified range of data from the file
-static bool hashFileRange(FILE *f, SignatureHandler *handler, Goffset start, Goffset end)
+static bool hashFileRange(FILE *f, SignatureSignHandler *handler, Goffset start, Goffset end)
 {
     const int BUF_SIZE = 65536;
 
@@ -611,7 +611,7 @@ bool FormWidgetSignature::signDocument(const std::string &saveFilename, const st
         return false;
     }
 
-    SignatureHandler sigHandler(certNickname, HashAlgorithm::Sha256);
+    SignatureSignHandler sigHandler(certNickname, HashAlgorithm::Sha256);
 
     FormFieldSignature *signatureField = static_cast<FormFieldSignature *>(field);
     std::unique_ptr<X509CertificateInfo> certInfo = sigHandler.getCertificateInfo();
@@ -2305,7 +2305,7 @@ void FormFieldSignature::parseInfo()
     }
 }
 
-void FormFieldSignature::hashSignedDataBlock(SignatureHandler *handler, Goffset block_len)
+void FormFieldSignature::hashSignedDataBlock(SignatureVerificationHandler *handler, Goffset block_len)
 {
 #ifdef ENABLE_NSS3
     const int BLOCK_SIZE = 4096;
@@ -2363,7 +2363,7 @@ SignatureInfo *FormFieldSignature::validateSignature(bool doVerifyCert, bool for
     const int signature_len = signature->getLength();
     std::vector<unsigned char> signatureData(signature_len);
     memcpy(signatureData.data(), signature->c_str(), signature_len);
-    SignatureHandler signature_handler(std::move(signatureData));
+    SignatureVerificationHandler signature_handler(std::move(signatureData));
 
     Goffset fileLength = doc->getBaseStream()->getLength();
     for (int i = 0; i < arrayLen / 2; i++) {
diff --git a/poppler/Form.h b/poppler/Form.h
index d9bf7a45..80be38e6 100644
--- a/poppler/Form.h
+++ b/poppler/Form.h
@@ -56,7 +56,7 @@ class GfxResources;
 class PDFDoc;
 class SignatureInfo;
 class X509CertificateInfo;
-class SignatureHandler;
+class SignatureVerificationHandler;
 
 enum FormFieldType
 {
@@ -640,7 +640,7 @@ public:
 
 private:
     void parseInfo();
-    void hashSignedDataBlock(SignatureHandler *handler, Goffset block_len);
+    void hashSignedDataBlock(SignatureVerificationHandler *handler, Goffset block_len);
 
     FormSignatureType signature_type;
     Object byte_range;
diff --git a/poppler/SignatureHandler.cc b/poppler/SignatureHandler.cc
index 17d7cb17..64ac6594 100644
--- a/poppler/SignatureHandler.cc
+++ b/poppler/SignatureHandler.cc
@@ -498,30 +498,21 @@ static unsigned int digestLength(HashAlgorithm digestAlgId)
     }
 }
 
-std::string SignatureHandler::getSignerName() const
+std::string SignatureVerificationHandler::getSignerName() const
 {
     if (!NSS_IsInitialized()) {
         return {};
     }
-
-    if (!signing_cert && !CMSSignerInfo) {
+    if (!CMSSignerInfo) {
         return {};
     }
-    CERTCertificate *activeCert = nullptr;
 
+    auto signing_cert = NSS_CMSSignerInfo_GetSigningCertificate(CMSSignerInfo, CERT_GetDefaultCertDB());
     if (!signing_cert) {
-        // we are signing, and thus getting the name of the SignerInfo, not of the signing cert. Data owned by CMSSignerInfo
-        activeCert = NSS_CMSSignerInfo_GetSigningCertificate(CMSSignerInfo, CERT_GetDefaultCertDB());
-    } else {
-        // We are verifying. data owned by us.
-        activeCert = signing_cert;
-    }
-
-    if (!activeCert) {
         return {};
     }
 
-    char *commonName = CERT_GetCommonName(&activeCert->subject);
+    char *commonName = CERT_GetCommonName(&signing_cert->subject);
     if (!commonName) {
         return {};
     }
@@ -531,38 +522,23 @@ std::string SignatureHandler::getSignerName() const
     return name;
 }
 
-std::string SignatureHandler::getSignerSubjectDN() const
+std::string SignatureVerificationHandler::getSignerSubjectDN() const
 {
-    if (!signing_cert && !CMSSignerInfo) {
+    if (!CMSSignerInfo) {
         return {};
     }
-    CERTCertificate *activeCert = nullptr;
-
+    auto signing_cert = NSS_CMSSignerInfo_GetSigningCertificate(CMSSignerInfo, CERT_GetDefaultCertDB());
     if (!signing_cert) {
-        // we are signing, and thus getting the name of the SignerInfo, not of the signing cert. Data owned by CMSSignerInfo
-        activeCert = NSS_CMSSignerInfo_GetSigningCertificate(CMSSignerInfo, CERT_GetDefaultCertDB());
-    } else {
-        // We are verifying. data owned by us.
-        activeCert = signing_cert;
-    }
-
-    if (!activeCert) {
         return {};
     }
-
-    return activeCert->subjectName;
+    return std::string { signing_cert->subjectName };
 }
 
-HashAlgorithm SignatureHandler::getHashAlgorithm() const
+time_t SignatureVerificationHandler::getSigningTime() const
 {
-    if (hashContext) {
-        return hashContext->getHashAlgorithm();
+    if (!CMSSignerInfo) {
+        return {};
     }
-    return HashAlgorithm::Unknown;
-}
-
-time_t SignatureHandler::getSigningTime() const
-{
     PRTime sTime; // time in microseconds since the epoch
 
     if (NSS_CMSSignerInfo_GetSigningTime(CMSSignerInfo, &sTime) != SECSuccess) {
@@ -670,21 +646,24 @@ static std::unique_ptr<X509CertificateInfo> getCertificateInfoFromCERT(CERTCerti
     return certInfo;
 }
 
-std::unique_ptr<X509CertificateInfo> SignatureHandler::getCertificateInfo() const
+std::unique_ptr<X509CertificateInfo> SignatureVerificationHandler::getCertificateInfo() const
 {
-    if (CMSSignerInfo) {
-        CERTCertificate *cert = NSS_CMSSignerInfo_GetSigningCertificate(CMSSignerInfo, CERT_GetDefaultCertDB());
-        if (!cert) {
-            return nullptr;
-        }
-        return getCertificateInfoFromCERT(cert);
-    } else {
-        if (!signing_cert) {
-            return nullptr;
-        }
+    if (!CMSSignerInfo) {
+        return nullptr;
+    }
+    CERTCertificate *cert = NSS_CMSSignerInfo_GetSigningCertificate(CMSSignerInfo, CERT_GetDefaultCertDB());
+    if (!cert) {
+        return nullptr;
+    }
+    return getCertificateInfoFromCERT(cert);
+}
 
-        return getCertificateInfoFromCERT(signing_cert);
+std::unique_ptr<X509CertificateInfo> SignatureSignHandler::getCertificateInfo() const
+{
+    if (!signing_cert) {
+        return nullptr;
     }
+    return getCertificateInfoFromCERT(signing_cert);
 }
 
 static std::optional<std::string> getDefaultFirefoxCertDB()
@@ -777,16 +756,15 @@ void SignatureHandler::setNSSPasswordCallback(const std::function<char *(const c
     PasswordFunction = f;
 }
 
-SignatureHandler::SignatureHandler(std::vector<unsigned char> &&p7data) : p7(std::move(p7data)), hashContext(nullptr), CMSMessage(nullptr), CMSSignedData(nullptr), CMSSignerInfo(nullptr), signing_cert(nullptr)
+SignatureVerificationHandler::SignatureVerificationHandler(std::vector<unsigned char> &&p7data) : p7(std::move(p7data)), CMSMessage(nullptr), CMSSignedData(nullptr), CMSSignerInfo(nullptr)
 {
-    setNSSDir({});
+    SignatureHandler::setNSSDir({});
     CMSitem.data = p7.data();
     CMSitem.len = p7.size();
     CMSMessage = CMS_MessageCreate(&CMSitem);
     CMSSignedData = CMS_SignedDataCreate(CMSMessage);
     if (CMSSignedData) {
         CMSSignerInfo = CMS_SignerInfoCreate(CMSSignedData);
-
         SECItem usedAlgorithm = NSS_CMSSignedData_GetDigestAlgs(CMSSignedData)[0]->algorithm;
         auto hashAlgorithm = SECOID_FindOIDTag(&usedAlgorithm);
         HASH_HashType hashType = HASH_GetHashTypeByOidTag(hashAlgorithm);
@@ -794,22 +772,34 @@ SignatureHandler::SignatureHandler(std::vector<unsigned char> &&p7data) : p7(std
     }
 }
 
-SignatureHandler::SignatureHandler(const std::string &certNickname, HashAlgorithm digestAlgTag)
-    : CMSitem(), hashContext(std::make_unique<HashContext>(digestAlgTag)), CMSMessage(nullptr), CMSSignedData(nullptr), CMSSignerInfo(nullptr), signing_cert(nullptr)
+SignatureSignHandler::SignatureSignHandler(const std::string &certNickname, HashAlgorithm digestAlgTag) : hashContext(std::make_unique<HashContext>(digestAlgTag)), signing_cert(nullptr)
 {
-    setNSSDir({});
-    CMSMessage = NSS_CMSMessage_Create(nullptr);
+    SignatureHandler::setNSSDir({});
     signing_cert = CERT_FindCertByNickname(CERT_GetDefaultCertDB(), certNickname.c_str());
 }
 
-void SignatureHandler::updateHash(unsigned char *data_block, int data_len)
+HashAlgorithm SignatureVerificationHandler::getHashAlgorithm() const
 {
-    if (hashContext) {
-        hashContext->updateHash(data_block, data_len);
-    }
+    return hashContext->getHashAlgorithm();
+}
+
+void SignatureVerificationHandler::updateHash(unsigned char *data_block, int data_len)
+{
+    hashContext->updateHash(data_block, data_len);
+}
+
+void SignatureSignHandler::updateHash(unsigned char *data_block, int data_len)
+{
+    hashContext->updateHash(data_block, data_len);
 }
 
-SignatureHandler::~SignatureHandler()
+SignatureSignHandler::~SignatureSignHandler()
+{
+    if (signing_cert) {
+        CERT_DestroyCertificate(signing_cert);
+    }
+}
+SignatureVerificationHandler::~SignatureVerificationHandler()
 {
     if (CMSMessage) {
         // in the CMS_SignedDataCreate, we malloc some memory
@@ -826,10 +816,6 @@ SignatureHandler::~SignatureHandler()
         NSS_CMSMessage_Destroy(CMSMessage);
         free(toFree);
     }
-
-    if (signing_cert) {
-        CERT_DestroyCertificate(signing_cert);
-    }
 }
 
 static NSSCMSMessage *CMS_MessageCreate(SECItem *cms_item)
@@ -881,7 +867,7 @@ static NSSCMSSignedData *CMS_SignedDataCreate(NSSCMSMessage *cms_msg)
     }
 }
 
-NSSCMSSignerInfo *CMS_SignerInfoCreate(NSSCMSSignedData *cms_sig_data)
+static NSSCMSSignerInfo *CMS_SignerInfoCreate(NSSCMSSignedData *cms_sig_data)
 {
     NSSCMSSignerInfo *signerInfo = NSS_CMSSignedData_GetSignerInfo(cms_sig_data, 0);
     if (!signerInfo) {
@@ -912,7 +898,7 @@ static SignatureValidationStatus NSS_SigTranslate(NSSCMSVerificationStatus nss_c
     }
 }
 
-SignatureValidationStatus SignatureHandler::validateSignature()
+SignatureValidationStatus SignatureVerificationHandler::validateSignature()
 {
     if (!CMSSignedData) {
         return SIGNATURE_GENERIC_ERROR;
@@ -955,7 +941,7 @@ SignatureValidationStatus SignatureHandler::validateSignature()
     }
 }
 
-CertificateValidationStatus SignatureHandler::validateCertificate(time_t validation_time, bool ocspRevocationCheck, bool useAIACertFetch)
+CertificateValidationStatus SignatureVerificationHandler::validateCertificate(time_t validation_time, bool ocspRevocationCheck, bool useAIACertFetch)
 {
     CERTCertificate *cert;
 
@@ -1011,7 +997,7 @@ CertificateValidationStatus SignatureHandler::validateCertificate(time_t validat
     return CERTIFICATE_GENERIC_ERROR;
 }
 
-std::unique_ptr<GooString> SignatureHandler::signDetached(const std::string &password) const
+std::unique_ptr<GooString> SignatureSignHandler::signDetached(const std::string &password)
 {
     if (!hashContext) {
         return nullptr;
@@ -1024,7 +1010,6 @@ std::unique_ptr<GooString> SignatureHandler::signDetached(const std::string &pas
     /////////////////////////////////////
     /// Code from LibreOffice under MPLv2
     /////////////////////////////////////
-
     struct NSSCMSMessageDestroyer
     {
         void operator()(NSSCMSMessage *message) { NSS_CMSMessage_Destroy(message); }
@@ -1166,11 +1151,11 @@ std::unique_ptr<GooString> SignatureHandler::signDetached(const std::string &pas
         return nullptr;
     }
 
-    GooString *signature = new GooString(reinterpret_cast<const char *>(cms_output.data), cms_output.len);
+    auto signature = std::make_unique<GooString>(reinterpret_cast<const char *>(cms_output.data), cms_output.len);
 
     SECITEM_FreeItem(pEncodedCertificate, PR_TRUE);
 
-    return std::unique_ptr<GooString>(signature);
+    return signature;
 }
 
 static char *GetPasswordFunction(PK11SlotInfo *slot, PRBool /*retry*/, void * /*arg*/)
diff --git a/poppler/SignatureHandler.h b/poppler/SignatureHandler.h
index fb3aac2e..c9fb575e 100644
--- a/poppler/SignatureHandler.h
+++ b/poppler/SignatureHandler.h
@@ -29,6 +29,7 @@
 
 #include <vector>
 #include <functional>
+#include <memory>
 
 /* NSPR Headers */
 #include <nspr.h>
@@ -66,23 +67,54 @@ private:
     HashAlgorithm digest_alg_tag;
 };
 
-class POPPLER_PRIVATE_EXPORT SignatureHandler
+class POPPLER_PRIVATE_EXPORT SignatureVerificationHandler
 {
 public:
-    explicit SignatureHandler(std::vector<unsigned char> &&p7data);
-    SignatureHandler(const std::string &certNickName, HashAlgorithm digestAlgTag);
-    ~SignatureHandler();
+    explicit SignatureVerificationHandler(std::vector<unsigned char> &&p7data);
+    ~SignatureVerificationHandler();
+    SignatureValidationStatus validateSignature();
     time_t getSigningTime() const;
     std::string getSignerName() const;
     std::string getSignerSubjectDN() const;
-    HashAlgorithm getHashAlgorithm() const;
-    void updateHash(unsigned char *data_block, int data_len);
-    SignatureValidationStatus validateSignature();
     // Use -1 as validation_time for now
     CertificateValidationStatus validateCertificate(time_t validation_time, bool ocspRevocationCheck, bool useAIACertFetch);
     std::unique_ptr<X509CertificateInfo> getCertificateInfo() const;
+    void updateHash(unsigned char *data_block, int data_len);
+    HashAlgorithm getHashAlgorithm() const;
+
+    SignatureVerificationHandler(const SignatureVerificationHandler &) = delete;
+    SignatureVerificationHandler &operator=(const SignatureVerificationHandler &) = delete;
+
+private:
+    std::vector<unsigned char> p7;
+    NSSCMSMessage *CMSMessage;
+    NSSCMSSignedData *CMSSignedData;
+    NSSCMSSignerInfo *CMSSignerInfo;
+    SECItem CMSitem;
+    std::unique_ptr<HashContext> hashContext;
+};
+
+class POPPLER_PRIVATE_EXPORT SignatureSignHandler
+{
+public:
+    SignatureSignHandler(const std::string &certNickname, HashAlgorithm digestAlgTag);
+    ~SignatureSignHandler();
+    std::unique_ptr<X509CertificateInfo> getCertificateInfo() const;
+    void updateHash(unsigned char *data_block, int data_len);
+    std::unique_ptr<GooString> signDetached(const std::string &password);
+
+    SignatureSignHandler(const SignatureSignHandler &) = delete;
+    SignatureSignHandler &operator=(const SignatureSignHandler &) = delete;
+
+private:
+    std::unique_ptr<HashContext> hashContext;
+    CERTCertificate *signing_cert;
+};
+
+class POPPLER_PRIVATE_EXPORT SignatureHandler
+{
+public:
     static std::vector<std::unique_ptr<X509CertificateInfo>> getAvailableSigningCertificates();
-    std::unique_ptr<GooString> signDetached(const std::string &password) const;
 
     // Initializes the NSS dir with the custom given directory
     // calling it with an empty string means use the default firefox db, /etc/pki/nssdb, ~/.pki/nssdb
@@ -95,21 +127,9 @@ public:
 
     static void setNSSPasswordCallback(const std::function<char *(const char *)> &f);
 
-private:
-    SignatureHandler(const SignatureHandler &);
-    SignatureHandler &operator=(const SignatureHandler &);
-
-    static void outputCallback(void *arg, const char *buf, unsigned long len);
-
-    std::vector<unsigned char> p7;
-    HashAlgorithm digest_alg_tag;
-    SECItem CMSitem;
-    std::unique_ptr<HashContext> hashContext;
-    NSSCMSMessage *CMSMessage;
-    NSSCMSSignedData *CMSSignedData;
-    NSSCMSSignerInfo *CMSSignerInfo;
-    CERTCertificate *signing_cert;
+    SignatureHandler() = delete;
 
+private:
     static std::string sNssDir;
 };
 
diff --git a/qt5/src/poppler-form.cc b/qt5/src/poppler-form.cc
index 1bead08b..dfdcd391 100644
--- a/qt5/src/poppler-form.cc
+++ b/qt5/src/poppler-form.cc
@@ -781,7 +781,7 @@ bool CertificateInfo::checkPassword(const QString &password) const
 {
 #ifdef ENABLE_NSS3
     Q_D(const CertificateInfo);
-    SignatureHandler sigHandler(d->nick_name.toStdString(), HashAlgorithm::Sha256);
+    SignatureSignHandler sigHandler(d->nick_name.toStdString(), HashAlgorithm::Sha256);
     unsigned char buffer[5];
     memcpy(buffer, "test", 5);
     sigHandler.updateHash(buffer, 5);
diff --git a/qt6/src/poppler-form.cc b/qt6/src/poppler-form.cc
index ac138261..b415c08e 100644
--- a/qt6/src/poppler-form.cc
+++ b/qt6/src/poppler-form.cc
@@ -781,7 +781,7 @@ bool CertificateInfo::checkPassword(const QString &password) const
 {
 #ifdef ENABLE_NSS3
     Q_D(const CertificateInfo);
-    SignatureHandler sigHandler(d->nick_name.toStdString(), HashAlgorithm::Sha256);
+    SignatureSignHandler sigHandler(d->nick_name.toStdString(), HashAlgorithm::Sha256);
     unsigned char buffer[5];
     memcpy(buffer, "test", 5);
     sigHandler.updateHash(buffer, 5);


More information about the poppler mailing list