svl/source/crypto/cryptosign.cxx |  153 +++++++++++++++++++--------------------
 1 file changed, 78 insertions(+), 75 deletions(-)

New commits:
commit cc1353f41427a202a98337618bd07a6cdcc8c963
Author:     Eloi Montañés <e...@ilsrv.com>
AuthorDate: Thu Aug 1 20:41:13 2024 +0200
Commit:     Michael Stahl <michael.st...@allotropia.de>
CommitDate: Wed Sep 4 18:25:57 2024 +0200

    Fix signature hash on NSS backend
    
    When using the NSS backend, the SignedAttributes were added after the hash
    computation for the timestamp request, altering the final signature and
    rendering the timestamp unverifiable.
    This patch changes the order in which the attributes are added and uses a 
copy
    of the signer for the hash computation in order to generate a correct hash.
    
    Change-Id: I14cecf1819e80d7a2e470e0f1dee2d09bcbe852c
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/171384
    Tested-by: Jenkins
    Reviewed-by: Michael Stahl <michael.st...@allotropia.de>
    (cherry picked from commit 16a9df36380a24380d4eee77bee3c36223661907)
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/172816

diff --git a/svl/source/crypto/cryptosign.cxx b/svl/source/crypto/cryptosign.cxx
index 52f4e4e021b2..7b410b38b32d 100644
--- a/svl/source/crypto/cryptosign.cxx
+++ b/svl/source/crypto/cryptosign.cxx
@@ -988,6 +988,81 @@ bool Signing::Sign(OStringBuffer& rCMSHexBuffer)
 
     OString pass(OUStringToOString( m_aSignPassword, RTL_TEXTENCODING_UTF8 ));
 
+    // Add the signing certificate as a signed attribute.
+    ESSCertIDv2* aCertIDs[2];
+    ESSCertIDv2 aCertID;
+    // Write ESSCertIDv2.hashAlgorithm.
+    aCertID.hashAlgorithm.algorithm.data = nullptr;
+    aCertID.hashAlgorithm.parameters.data = nullptr;
+    SECOID_SetAlgorithmID(nullptr, &aCertID.hashAlgorithm, SEC_OID_SHA256, 
nullptr);
+    comphelper::ScopeGuard aAlgoGuard(
+        [&aCertID] () { SECOID_DestroyAlgorithmID(&aCertID.hashAlgorithm, 
false); } );
+    // Write ESSCertIDv2.certHash.
+    SECItem aCertHashItem;
+    auto pDerEncoded = reinterpret_cast<const unsigned char 
*>(aDerEncoded.getArray());
+    std::vector<unsigned char> aCertHashResult = 
comphelper::Hash::calculateHash(pDerEncoded, aDerEncoded.getLength(), 
comphelper::HashType::SHA256);
+    aCertHashItem.type = siBuffer;
+    aCertHashItem.data = aCertHashResult.data();
+    aCertHashItem.len = aCertHashResult.size();
+    aCertID.certHash = aCertHashItem;
+    // Write ESSCertIDv2.issuerSerial.
+    IssuerSerial aSerial;
+    GeneralName aName;
+    aName.name = cert->issuer;
+    aSerial.issuer.names = aName;
+    aSerial.serialNumber = cert->serialNumber;
+    aCertID.issuerSerial = aSerial;
+    // Write SigningCertificateV2.certs.
+    aCertIDs[0] = &aCertID;
+    aCertIDs[1] = nullptr;
+    SigningCertificateV2 aCertificate;
+    aCertificate.certs = &aCertIDs[0];
+    SECItem* pEncodedCertificate = SEC_ASN1EncodeItem(nullptr, nullptr, 
&aCertificate, SigningCertificateV2Template);
+    if (!pEncodedCertificate)
+    {
+        SAL_WARN("svl.crypto", "SEC_ASN1EncodeItem() failed");
+        return false;
+    }
+
+    NSSCMSAttribute aAttribute;
+    SECItem aAttributeValues[2];
+    SECItem* pAttributeValues[2];
+    pAttributeValues[0] = aAttributeValues;
+    pAttributeValues[1] = nullptr;
+    aAttributeValues[0] = *pEncodedCertificate;
+    aAttributeValues[1].type = siBuffer;
+    aAttributeValues[1].data = nullptr;
+    aAttributeValues[1].len = 0;
+    aAttribute.values = pAttributeValues;
+
+    SECOidData aOidData;
+    aOidData.oid.data = nullptr;
+    /*
+     * id-aa-signingCertificateV2 OBJECT IDENTIFIER ::=
+     * { iso(1) member-body(2) us(840) rsadsi(113549) pkcs(1) pkcs9(9)
+     *   smime(16) id-aa(2) 47 }
+     */
+    if (my_SEC_StringToOID(&aOidData.oid, "1.2.840.113549.1.9.16.2.47", 0) != 
SECSuccess)
+    {
+        SAL_WARN("svl.crypto", "my_SEC_StringToOID() failed");
+        return false;
+    }
+    comphelper::ScopeGuard aGuard(
+        [&aOidData] () { SECITEM_FreeItem(&aOidData.oid, false); } );
+    aOidData.offset = SEC_OID_UNKNOWN;
+    aOidData.desc = "id-aa-signingCertificateV2";
+    aOidData.mechanism = CKM_SHA_1;
+    aOidData.supportedExtension = UNSUPPORTED_CERT_EXTENSION;
+    aAttribute.typeTag = &aOidData;
+    aAttribute.type = aOidData.oid;
+    aAttribute.encoded = PR_TRUE;
+
+    if (my_NSS_CMSSignerInfo_AddAuthAttr(cms_signer, &aAttribute) != 
SECSuccess)
+    {
+        SAL_WARN("svl.crypto", "my_NSS_CMSSignerInfo_AddAuthAttr() failed");
+        return false;
+    }
+
     TimeStampReq src;
     OStringBuffer response_buffer;
     TimeStampResp response;
@@ -999,6 +1074,7 @@ bool Signing::Sign(OStringBuffer& rCMSHexBuffer)
     valuesp[1] = nullptr;
     SECOidData typetag;
 
+    //NOTE: All signed/authenticated attributes are to be added before the 
following hash computation
     if( !m_aSignTSA.isEmpty() )
     {
         // Create another CMS message with the same contents as cms_msg, 
because it doesn't seem
@@ -1013,6 +1089,8 @@ bool Signing::Sign(OStringBuffer& rCMSHexBuffer)
             return false;
         }
 
+        PORT_Memcpy(ts_cms_signer, cms_signer, sizeof(NSSCMSSignerInfo));
+
         SECItem ts_cms_output;
         ts_cms_output.data = nullptr;
         ts_cms_output.len = 0;
@@ -1233,81 +1311,6 @@ bool Signing::Sign(OStringBuffer& rCMSHexBuffer)
         }
     }
 
-    // Add the signing certificate as a signed attribute.
-    ESSCertIDv2* aCertIDs[2];
-    ESSCertIDv2 aCertID;
-    // Write ESSCertIDv2.hashAlgorithm.
-    aCertID.hashAlgorithm.algorithm.data = nullptr;
-    aCertID.hashAlgorithm.parameters.data = nullptr;
-    SECOID_SetAlgorithmID(nullptr, &aCertID.hashAlgorithm, SEC_OID_SHA256, 
nullptr);
-    comphelper::ScopeGuard aAlgoGuard(
-        [&aCertID] () { SECOID_DestroyAlgorithmID(&aCertID.hashAlgorithm, 
false); } );
-    // Write ESSCertIDv2.certHash.
-    SECItem aCertHashItem;
-    auto pDerEncoded = reinterpret_cast<const unsigned char 
*>(aDerEncoded.getArray());
-    std::vector<unsigned char> aCertHashResult = 
comphelper::Hash::calculateHash(pDerEncoded, aDerEncoded.getLength(), 
comphelper::HashType::SHA256);
-    aCertHashItem.type = siBuffer;
-    aCertHashItem.data = aCertHashResult.data();
-    aCertHashItem.len = aCertHashResult.size();
-    aCertID.certHash = aCertHashItem;
-    // Write ESSCertIDv2.issuerSerial.
-    IssuerSerial aSerial;
-    GeneralName aName;
-    aName.name = cert->issuer;
-    aSerial.issuer.names = aName;
-    aSerial.serialNumber = cert->serialNumber;
-    aCertID.issuerSerial = aSerial;
-    // Write SigningCertificateV2.certs.
-    aCertIDs[0] = &aCertID;
-    aCertIDs[1] = nullptr;
-    SigningCertificateV2 aCertificate;
-    aCertificate.certs = &aCertIDs[0];
-    SECItem* pEncodedCertificate = SEC_ASN1EncodeItem(nullptr, nullptr, 
&aCertificate, SigningCertificateV2Template);
-    if (!pEncodedCertificate)
-    {
-        SAL_WARN("svl.crypto", "SEC_ASN1EncodeItem() failed");
-        return false;
-    }
-
-    NSSCMSAttribute aAttribute;
-    SECItem aAttributeValues[2];
-    SECItem* pAttributeValues[2];
-    pAttributeValues[0] = aAttributeValues;
-    pAttributeValues[1] = nullptr;
-    aAttributeValues[0] = *pEncodedCertificate;
-    aAttributeValues[1].type = siBuffer;
-    aAttributeValues[1].data = nullptr;
-    aAttributeValues[1].len = 0;
-    aAttribute.values = pAttributeValues;
-
-    SECOidData aOidData;
-    aOidData.oid.data = nullptr;
-    /*
-     * id-aa-signingCertificateV2 OBJECT IDENTIFIER ::=
-     * { iso(1) member-body(2) us(840) rsadsi(113549) pkcs(1) pkcs9(9)
-     *   smime(16) id-aa(2) 47 }
-     */
-    if (my_SEC_StringToOID(&aOidData.oid, "1.2.840.113549.1.9.16.2.47", 0) != 
SECSuccess)
-    {
-        SAL_WARN("svl.crypto", "my_SEC_StringToOID() failed");
-        return false;
-    }
-    comphelper::ScopeGuard aGuard(
-        [&aOidData] () { SECITEM_FreeItem(&aOidData.oid, false); } );
-    aOidData.offset = SEC_OID_UNKNOWN;
-    aOidData.desc = "id-aa-signingCertificateV2";
-    aOidData.mechanism = CKM_SHA_1;
-    aOidData.supportedExtension = UNSUPPORTED_CERT_EXTENSION;
-    aAttribute.typeTag = &aOidData;
-    aAttribute.type = aOidData.oid;
-    aAttribute.encoded = PR_TRUE;
-
-    if (my_NSS_CMSSignerInfo_AddAuthAttr(cms_signer, &aAttribute) != 
SECSuccess)
-    {
-        SAL_WARN("svl.crypto", "my_NSS_CMSSignerInfo_AddAuthAttr() failed");
-        return false;
-    }
-
     SECItem cms_output;
     cms_output.data = nullptr;
     cms_output.len = 0;

Reply via email to