Created attachment 124365 [details] Certificate used When I export as signed PDF/A with my personal certificate, the algorithm name save is SHA1WITHRSA. I suppose it should be only SHA1. One of the visible consequences is that, when opened in Adobe Reader, it shows an alert of altered pages. And worst, my signed documents are not being accepted on some systems
Where do you see the SBA1WITHRSA? And do you have some comparable PDF signed by some other application where you instead see SHA1?
Created attachment 125803 [details] java error messages displayed on our internal document system The java error messages displayed on our internal document system when I upload any LO exported PDF/A file with my signature.
Created attachment 125804 [details] A snapshot of Acrobat Reader warnings A snapshot of Acrobat Reader DC showing a page modification warning on the same PDF/A signed (and untouched) file.
Created attachment 125805 [details] The signed PDF/A file. The signed PDF/A file exported by LO that causes the problem.
Created attachment 125806 [details] The same PDF/A signed by our internal java signer. The same PDF/A file, exported unsigned and signed by our internal java signer.
Created attachment 125809 [details] Certificate The certification I used. This is a Brazilian official CA (www.serpro.gov.br) issued certificate, used on many other applications w/o a problem.
I attached some adsitional files; hope it will help to identify the problem.
Thanks. No promises when I (or somebody) will have time to take a look, but the new information seems useful. If that is your real certificate, are you sure you should make that publicly available?
Ok, thanx in advance. No problem it's only the pub key, to show that the algorithm field looks ok.
Hi, I am seeing the same problem here and the culprit seems to be this line in LibreOffice's source code: https://cgit.freedesktop.org/libreoffice/core/tree/vcl/source/gdi/pdfwriter_impl.cxx#n7188 The following line: aSignerInfo.HashAlgorithm.pszObjId = const_cast<LPSTR>(szOID_RSA_SHA1RSA); Should probably read: aSignerInfo.HashAlgorithm.pszObjId = const_cast<LPSTR>(szOID_OIWSEC_sha1); I have traced that path from the pdfexport.cxx code, which creates an aContext structure and creates a pPDFWriter object at: https://cgit.freedesktop.org/libreoffice/core/tree/filter/source/pdf/pdfexport.cxx#n787 The implementation of the code that does the actual signing is in pdfwriter_impl.cxx and creates the aSignerInfo structure of type CMSG_SIGNER_ENCODE_INFO, documented in https://msdn.microsoft.com/pt-br/library/aa925156.aspx The specific field that is set is aSignerInfo.HashAlgorithm.pszObjId, which is documented at: https://msdn.microsoft.com/pt-br/library/office/aa381133.aspx. The value currently in the code is: szOID_RSA_SHA1RSA "1.2.840.113549.1.1.5" which is NOT a digest algorithm, but an encryption and signing algorithn. The likely value for that should be: szOID_OIWSEC_sha1 "1.3.14.3.2.26" Or, while we are at that, upgrade the algorithm to a more modern one not relying on sha1. It should have minor performance impact: szOID_NIST_sha256 "2.16.840.1.101.3.4.2.1" or szOID_NIST_sha512 "2.16.840.1.101.3.4.2.3" Other parts of the file should also be updated: https://cgit.freedesktop.org/libreoffice/core/tree/vcl/source/gdi/pdfwriter_impl.cxx#n7159 (although it seems the aPara structure is not actually being used anywhere else) CAVEAT: 1. I am not familiar with LibreOffice's codebase, please check if these two lines are really all it takes. 2. The digest algorithm upgrade seems to be a bit more complex than a fix for this bug, it requires changes in other parts of the file. Nonetheless, I think this is a wonderful time to do it, since sha1 is showing it's age. Bruce Scheiner blogged in 2005 - 11 years ago - that it was no longer safe even then. See https://www.schneier.com/blog/archives/2005/02/cryptanalysis_o.html
By the way, it does not seem to be an issue specific do PDF/A, but to all signed PDF exports.
I've changed the pdf export code to use SHA256 on Linux/macOS (NSS case) in commit 3ab31ae5db001021069f25257454b7dee78e6dba. I plan to do the same on Windows soon.
Thanks for looking into this Miklos! I have taken some time to read the relevant bits in the source but I am far from being able to submit a proper pull request. I have got some feedback from my IT department and I would like to ask that the algorithm is cranked up to SHA512, if you are willing. Since there is no option presented to the user and the performance penalty should not be noticeable, they asked that we default to the highest practical security alternative. I have put together a diff of what I perceive to be the necessary changes in the next post, including the fix for the wrong szOID_RSA_SHA1RSA id.
--- a/vcl/source/gdi/pdfwriter_impl.cxx +++ b/vcl/source/gdi/pdfwriter_impl.cxx @@ -6278,7 +6278,7 @@ * Hash ::= OCTET STRING * * ESSCertIDv2 ::= SEQUENCE { - * hashAlgorithm AlgorithmIdentifier DEFAULT {algorithm id-sha256}, + * hashAlgorithm AlgorithmIdentifier DEFAULT {algorithm id-sha512}, * certHash Hash, * issuerSerial IssuerSerial OPTIONAL * } @@ -6662,7 +6662,7 @@ return nullptr; } - *cms_signer = NSS_CMSSignerInfo_Create(result, cert, SEC_OID_SHA256); + *cms_signer = NSS_CMSSignerInfo_Create(result, cert, SEC_OID_SHA512); if (!*cms_signer) { SAL_WARN("vcl.pdfwriter", "NSS_CMSSignerInfo_Create failed"); @@ -6703,7 +6703,7 @@ return nullptr; } - if (NSS_CMSSignedData_SetDigestValue(*cms_sd, SEC_OID_SHA256, digest) != SECSuccess) + if (NSS_CMSSignedData_SetDigestValue(*cms_sd, SEC_OID_SHA512, digest) != SECSuccess) { SAL_WARN("vcl.pdfwriter", "NSS_CMSSignedData_SetDigestValue failed"); NSS_CMSSignedData_Destroy(*cms_sd); @@ -6748,7 +6748,7 @@ return false; } - HashContextScope hc(HASH_Create(HASH_AlgSHA256)); + HashContextScope hc(HASH_Create(HASH_AlgSHA512)); if (!hc.get()) { SAL_WARN("vcl.pdfwriter", "HASH_Create failed"); @@ -6762,15 +6762,15 @@ HASH_Update(hc.get(), static_cast<const unsigned char*>(rContext.m_pByteRange2), rContext.m_nByteRange2); SECItem digest; - unsigned char hash[SHA256_LENGTH]; + unsigned char hash[SHA512_LENGTH]; digest.data = hash; - HASH_End(hc.get(), digest.data, &digest.len, SHA256_LENGTH); + HASH_End(hc.get(), digest.data, &digest.len, SHA512_LENGTH); hc.clear(); #ifdef DBG_UTIL { FILE *out = fopen("PDFWRITER.hash.data", "wb"); - fwrite(hash, SHA256_LENGTH, 1, out); + fwrite(hash, SHA512_LENGTH, 1, out); fclose(out); } #endif @@ -6836,7 +6836,7 @@ } #endif - HashContextScope ts_hc(HASH_Create(HASH_AlgSHA256)); + HashContextScope ts_hc(HASH_Create(HASH_AlgSHA512)); if (!ts_hc.get()) { SAL_WARN("vcl.pdfwriter", "HASH_Create failed"); @@ -6847,16 +6847,16 @@ HASH_Begin(ts_hc.get()); HASH_Update(ts_hc.get(), ts_cms_signer->encDigest.data, ts_cms_signer->encDigest.len); SECItem ts_digest; - unsigned char ts_hash[SHA256_LENGTH]; + unsigned char ts_hash[SHA512_LENGTH]; ts_digest.type = siBuffer; ts_digest.data = ts_hash; - HASH_End(ts_hc.get(), ts_digest.data, &ts_digest.len, SHA256_LENGTH); + HASH_End(ts_hc.get(), ts_digest.data, &ts_digest.len, SHA512_LENGTH); ts_hc.clear(); #ifdef DBG_UTIL { FILE *out = fopen("PDFWRITER.ts_hash.data", "wb"); - fwrite(ts_hash, SHA256_LENGTH, 1, out); + fwrite(ts_hash, SHA512_LENGTH, 1, out); fclose(out); } #endif @@ -6868,7 +6868,7 @@ src.messageImprint.hashAlgorithm.algorithm.data = nullptr; src.messageImprint.hashAlgorithm.parameters.data = nullptr; - SECOID_SetAlgorithmID(nullptr, &src.messageImprint.hashAlgorithm, SEC_OID_SHA256, nullptr); + SECOID_SetAlgorithmID(nullptr, &src.messageImprint.hashAlgorithm, SEC_OID_SHA512, nullptr); src.messageImprint.hashedMessage = ts_digest; src.reqPolicy.type = siBuffer; @@ -7097,11 +7097,11 @@ // Write ESSCertIDv2.hashAlgorithm. aCertID.hashAlgorithm.algorithm.data = nullptr; aCertID.hashAlgorithm.parameters.data = nullptr; - SECOID_SetAlgorithmID(nullptr, &aCertID.hashAlgorithm, SEC_OID_SHA256, nullptr); + SECOID_SetAlgorithmID(nullptr, &aCertID.hashAlgorithm, SEC_OID_SHA512, nullptr); // Write ESSCertIDv2.certHash. SECItem aCertHashItem; - unsigned char aCertHash[SHA256_LENGTH]; - HashContextScope aCertHashContext(HASH_Create(HASH_AlgSHA256)); + unsigned char aCertHash[SHA512_LENGTH]; + HashContextScope aCertHashContext(HASH_Create(HASH_AlgSHA512)); if (!aCertHashContext.get()) { SAL_WARN("vcl.pdfwriter", "HASH_Create() failed"); @@ -7111,7 +7111,7 @@ HASH_Update(aCertHashContext.get(), reinterpret_cast<const unsigned char *>(rContext.m_pDerEncoded), rContext.m_nDerEncoded); aCertHashItem.type = siBuffer; aCertHashItem.data = aCertHash; - HASH_End(aCertHashContext.get(), aCertHashItem.data, &aCertHashItem.len, SHA256_LENGTH); + HASH_End(aCertHashContext.get(), aCertHashItem.data, &aCertHashItem.len, SHA512_LENGTH); aCertID.certHash = aCertHashItem; // Write SigningCertificateV2.certs. aCertIDs[0] = &aCertID; @@ -7228,7 +7228,7 @@ aPara.cbSize = sizeof(aPara); aPara.dwMsgEncodingType = PKCS_7_ASN_ENCODING | X509_ASN_ENCODING; aPara.pSigningCert = pCertContext; - aPara.HashAlgorithm.pszObjId = const_cast<LPSTR>(szOID_RSA_SHA1RSA); + aPara.HashAlgorithm.pszObjId = const_cast<LPSTR>(szOID_NIST_sha512); aPara.HashAlgorithm.Parameters.cbData = 0; aPara.cMsgCert = 1; aPara.rgpMsgCert = &pCertContext; @@ -7257,7 +7257,7 @@ aSignerInfo.pCertInfo = pCertContext->pCertInfo; aSignerInfo.hCryptProv = hCryptProv; aSignerInfo.dwKeySpec = nKeySpec; - aSignerInfo.HashAlgorithm.pszObjId = const_cast<LPSTR>(szOID_RSA_SHA1RSA); + aSignerInfo.HashAlgorithm.pszObjId = const_cast<LPSTR>(szOID_NIST_sha512); aSignerInfo.HashAlgorithm.Parameters.cbData = 0; CMSG_SIGNED_ENCODE_INFO aSignedInfo; @@ -7392,7 +7392,7 @@ if (!(*crts)(rContext.m_aSignTSA.getStr(), 0, 10000, - szOID_NIST_sha256, + szOID_NIST_sha512, &aTsPara, pDecodedSignerInfo->EncryptedHash.pbData, pDecodedSignerInfo->EncryptedHash.cbData,
Oh, sorry, I set the state of this bug to "assigned" to signal that I plan to fix the problem, i.e. you should not spend your time on it to avoid duplicated work. :-) How about: 1) I'll fix the algo & upgrade to SHA-256, so that LO on all platforms uses the same hashing (current NSS is SHA-256, mscrypto is SHA-1, which is inconsistent). 2) The sha256 -> sha512 part of your above patch would be still useful, but we take patches by you pushing to gerrit (more info at <https://wiki.documentfoundation.org/Development/gerrit>), not here pasted as bugzilla comments. Given that the LO 5.3 feature freeze is ~today, I'll make sure that 1) is in for 5.3. I'm a bit worried doing one more algo change right before a release, so if you don't mind, I would recommend doing that in master only, i.e. have that enabled in LO 5.4 only. I'll update this bug when I'm done with 1) (planned later this week), then you're more than welcome to push your SHA-512 patch to gerrit. ;-)
Miklos Vajna committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=4234b099f1ced2c1929a98ef6133b5280697f00a tdf#99327 vcl PDF mscrypto sign: fix SHA-256 OID It will be available in 5.3.0. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
It's in -- please submit your sha256 -> sha512 changes to gerrit on top of this. Thanks! :-)
Hi, Miklos! Thank you very much for your help in sorting out this bug! I re-read my previous posts and I apologize for omitting some vital information. I failed to mention that the proposed diffs were not tested, or even verified to compile. I am unable to commit the time needed to set a build environment and do the basic tests - the codebase is too alien for me at this time and I am nearing deadlines for two other projects. My intention was to clearly state what I thought the necessary changes were and to somehow help in the process. If you could review the changes in your build environment and check that they work, I will submit a patch as you indicated. Thanks again! --Daniel
Sorry for double posting - just clarifying that my patch addresses not only the algorithm change, but also the actual bug, which is the incorrect use of szOID_RSA_SHA1RSA.
No problem, but why did you reopen this bug? :-) The above fixed problem is exactly the title of this bug, that szOID_RSA_SHAXXXRSA was used as the algo ID instead of szOID_NIST_shaXXX. I plan to mark this resolved again in the near future, unless I hear I missed something.
Awesome Niklos! Thank you for your patience. I apologize for the noise, you are correct in that your patch solved the bug. Best, Daniel