From 40f181207574827827d2bf1b4ad72d46fc8ff1fb Mon Sep 17 00:00:00 2001 From: Thorsten Behrens Date: Mon, 17 Jul 2017 02:17:16 +0200 Subject: 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 Tested-by: Thorsten Behrens --- .../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(-) 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 >()); 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( - 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( keyId, strlen(keyId)); } -uno::Sequence CertificateImpl::getSHA256Thumbprint() +Sequence 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( 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( 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(strKeyId)) == 0) { + if (!k.isInvalid() && strcmp(k.primaryFingerprint(), reinterpret_cast(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 -- cgit