[Libreoffice-commits] core.git: xmlsecurity/source

[Thorsten Behrens]

 xmlsecurity/source/component/documentdigitalsignatures.cxx |    3 +
 xmlsecurity/source/gpg/CertificateImpl.cxx                 |   24 ++++++-------
 xmlsecurity/source/gpg/SecurityEnvironment.cxx             |    2 -
 xmlsecurity/source/gpg/xmlsignature_gpgimpl.cxx            |    6 ++-
 4 files changed, 20 insertions(+), 15 deletions(-)

New commits:
commit 40f181207574827827d2bf1b4ad72d46fc8ff1fb
Author: Thorsten Behrens <Thorsten.Behrens at CIB.de>
Date:   Mon Jul 17 02:17:16 2017 +0200

    gpg4libre: use full SHA1 hash for key identification
    
    Read and write full 20 bytes/40 hex chars of SHA1 key hash,
    instead of some abridged versions. See also
    https://lists.debian.org/debian-devel/2016/08/msg00215.html
    
    Change-Id: I741afc94ac7cf559880fe55ff02420723e13310d
    Reviewed-on: https://gerrit.libreoffice.org/40027
    Reviewed-by: Thorsten Behrens <Thorsten.Behrens at CIB.de>
    Tested-by: Thorsten Behrens <Thorsten.Behrens at CIB.de>

diff --git a/xmlsecurity/source/component/documentdigitalsignatures.cxx b/xmlsecurity/source/component/documentdigitalsignatures.cxx
index 832d4d082034..9d94b1845fd9 100644
--- a/xmlsecurity/source/component/documentdigitalsignatures.cxx
+++ b/xmlsecurity/source/component/documentdigitalsignatures.cxx
@@ -353,6 +353,9 @@ DocumentDigitalSignatures::ImplVerifySignatures(
             }
             else // GPG
             {
+                // TODO not ideal to retrieve cert by keyID, might
+                // collide, or PGPKeyID format might change - can't we
+                // keep the xCert ifself in rInfo?
                 rSigInfo.Signer = xGpgSecEnv->getCertificate( rInfo.ouGpgKeyID, xmlsecurity::numericStringToBigInteger("") );
                 rSigInfo.CertificateStatus = xGpgSecEnv->verifyCertificate(rSigInfo.Signer,
                                                                            Sequence<Reference<css::security::XCertificate> >());
diff --git a/xmlsecurity/source/gpg/CertificateImpl.cxx b/xmlsecurity/source/gpg/CertificateImpl.cxx
index 03fa49cdef68..49674f877956 100644
--- a/xmlsecurity/source/gpg/CertificateImpl.cxx
+++ b/xmlsecurity/source/gpg/CertificateImpl.cxx
@@ -40,10 +40,9 @@ sal_Int16 SAL_CALL CertificateImpl::getVersion()
 
 Sequence< sal_Int8 > SAL_CALL CertificateImpl::getSerialNumber()
 {
-    // This is mapped to the fingerprint for gpg
-    const char* keyId = m_pKey.primaryFingerprint();
-    return comphelper::arrayToSequence<sal_Int8>(
-        keyId, strlen(keyId));
+    // TODO: perhaps map to subkey's cardSerialNumber - if you have
+    // one to test
+    return Sequence< sal_Int8 >();
 }
 
 OUString SAL_CALL CertificateImpl::getIssuerName()
@@ -153,24 +152,25 @@ OUString SAL_CALL CertificateImpl::getSignatureAlgorithm()
 
 Sequence< sal_Int8 > SAL_CALL CertificateImpl::getSHA1Thumbprint()
 {
-    // This is mapped to the short keyID for gpg
-    const char* keyId = m_pKey.shortKeyID();
+    // This is mapped to the fingerprint for gpg
+    const char* keyId = m_pKey.primaryFingerprint();
     return comphelper::arrayToSequence<sal_Int8>(
         keyId, strlen(keyId));
 }
 
-uno::Sequence<sal_Int8> CertificateImpl::getSHA256Thumbprint()
+Sequence<sal_Int8> CertificateImpl::getSHA256Thumbprint()
 {
-    // This is mapped to the long keyID for gpg
-    const char* keyId = m_pKey.keyID();
+    // This is mapped to the fingerprint for gpg (though that's only
+    // SHA1 actually)
+    const char* keyId = m_pKey.primaryFingerprint();
     return comphelper::arrayToSequence<sal_Int8>(
         keyId, strlen(keyId));
 }
 
 Sequence< sal_Int8 > SAL_CALL CertificateImpl::getMD5Thumbprint()
 {
-    // This is mapped to the short keyID for gpg
-    const char* keyId = m_pKey.shortKeyID();
+    // This is mapped to the shorter keyID for gpg
+    const char* keyId = m_pKey.keyID();
     return comphelper::arrayToSequence<sal_Int8>(
         keyId, strlen(keyId));
 }
@@ -212,7 +212,7 @@ void CertificateImpl::setCertificate(GpgME::Context* ctx, const GpgME::Key& key)
     // extract key data, store into m_aBits
     GpgME::Data data_out;
     ctx->setArmor(false); // caller will base64-encode anyway
-    GpgME::Error err = ctx->exportPublicKeys(key.keyID(), data_out);
+    GpgME::Error err = ctx->exportPublicKeys(key.primaryFingerprint(), data_out);
 
     if (err)
         throw RuntimeException("The GpgME library failed to retrieve the public key");
diff --git a/xmlsecurity/source/gpg/SecurityEnvironment.cxx b/xmlsecurity/source/gpg/SecurityEnvironment.cxx
index a6d736b0232f..367fa35e76a3 100644
--- a/xmlsecurity/source/gpg/SecurityEnvironment.cxx
+++ b/xmlsecurity/source/gpg/SecurityEnvironment.cxx
@@ -115,7 +115,7 @@ Reference< XCertificate > SecurityEnvironmentGpg::getCertificate( const OUString
         GpgME::Key k = m_ctx->nextKey(err);
         if (err)
             break;
-        if (!k.isInvalid() && strcmp(k.keyID(), reinterpret_cast<const char*>(strKeyId)) == 0) {
+        if (!k.isInvalid() && strcmp(k.primaryFingerprint(), reinterpret_cast<const char*>(strKeyId)) == 0) {
             xCert = new CertificateImpl();
             xCert->setCertificate(m_ctx.get(), k);
             m_ctx->endKeyListing();
diff --git a/xmlsecurity/source/gpg/xmlsignature_gpgimpl.cxx b/xmlsecurity/source/gpg/xmlsignature_gpgimpl.cxx
index 227c34d78d64..1e90ca7c0545 100644
--- a/xmlsecurity/source/gpg/xmlsignature_gpgimpl.cxx
+++ b/xmlsecurity/source/gpg/xmlsignature_gpgimpl.cxx
@@ -366,8 +366,10 @@ SAL_CALL XMLSignature_GpgImpl::validate(
         // TODO: needs some more error handling, needs checking _all_ signatures
         if( verify_res.isNull() || verify_res.numSignatures() == 0 )
         {
-            // let's try again, but this time import the public key payload
-            // (avoiding that in a first cut for being a bit speedier)
+            // let's try again, but this time import the public key
+            // payload (avoiding that in a first cut for being a bit
+            // speedier. also prevents all too easy poisoning/sha1
+            // fingerprint collision attacks)
 
             // walk xml tree to PGPData node - go to children, first is
             // SignedInfo, 2nd is signaturevalue, 3rd is KeyInfo