[Libreoffice-commits] core.git: Branch 'libreoffice-5-3' - vcl/source xmlsecurity/qa xmlsecurity/source
Miklos Vajna
vmiklos at collabora.co.uk
Wed Nov 30 17:52:14 UTC 2016
vcl/source/gdi/pdfwriter_impl.cxx | 133 ++++++++++++++++-----
xmlsecurity/qa/unit/pdfsigning/data/good-pades.pdf |binary
xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx | 8 +
xmlsecurity/qa/unit/signing/data/good-xades.odt |binary
xmlsecurity/qa/unit/signing/data/no.odt |binary
xmlsecurity/qa/unit/signing/signing.cxx | 33 +++++
xmlsecurity/source/pdfio/pdfdocument.cxx | 22 +++
7 files changed, 168 insertions(+), 28 deletions(-)
New commits:
commit d8f50337fcafc52a12ad87f29233e5650b7bfca3
Author: Miklos Vajna <vmiklos at collabora.co.uk>
Date: Mon Nov 28 15:38:39 2016 +0100
vcl mscrypto PDF sign: bring it up to date with NSS, part 2
This is a combination of 6 commits:
1) CppunitTest_xmlsecurity_signing: fix this on Windows with non-empty cert store
The NSS code earlier started to save the hash algo ID of the signature
into the signature structure and I also added a unit test for this. This
failed on Windows when the system had at least one signing certificate
installed, as the mscrypto part of the patch was missing.
(cherry picked from commit fd3db1cf77c86cd787f912b7bb2ba3ad894203f3)
2) vcl mscrypto PDF sign: don't assume that header length is always 2 bytes
For now just assert that the short form doesn't try to handle larger
values than expected, the long form has to be implemented once we hit
the assert.
(cherry picked from commit 5bf32e4e78ffbe34f3b2840a9677ded34e5b4da7)
3) vcl mscrypto PDF sign: write IssuerSerial sequence
It fixes a problem detected by the PAdES validator from
<https://github.com/esig/dss>, and with this the Windows output is in
sync with NSS.
(cherry picked from commit e1446e9e25f784a730c0399ba64b52b36a01a91c)
4) vcl mscrypto PDF sign: fix typo in GetDERLengthOfLength()
When id-aa-signingCertificateV2 had a value that was larger than 255
bytes, then the header size is expected to be 4 bytes, but it was only
3. The length part of the header is 3 bytes: one byte declaring the
length-of-length, and 3 bytes for the length. We added this additional
byte to the result too early, that way we counted that e.g. 278 (the
number) fits into a single uint8_t, which is not the case.
Also introduce named constants for some of the hardwired numbers in the
code for better readability.
(cherry picked from commit 7339a3d39035ccc7541fbbddc858121ce464dc68)
5) CppunitTest_xmlsecurity_signing: add 2 more ODF / XAdES tests
Make sure we handle the case when the document has a signature
stream, but it's empty.
Make sure we find a given XAdES-enabled ODF document valid.
Previously this was tested only dynamically, i.e. breaking both the
import and the export at the same time went unnoticed.
(cherry picked from commit deaa4701e609f698999c3e05ce79b15f4cb94670)
6) CppunitTest_xmlsecurity_pdfsigning: add first PAdES test
As a start just make sure we accept "ETSI.CAdES.detached" as a valid
SubFilter value.
(cherry picked from commit 568e0394868114457c9dbf7cc1af5bc863ae2a4d)
Change-Id: I19f480a5a24df0f451261d6d9a0dd9bd72ff6cc1
Reviewed-on: https://gerrit.libreoffice.org/31435
Reviewed-by: Miklos Vajna <vmiklos at collabora.co.uk>
Tested-by: Miklos Vajna <vmiklos at collabora.co.uk>
diff --git a/vcl/source/gdi/pdfwriter_impl.cxx b/vcl/source/gdi/pdfwriter_impl.cxx
index 372f08e..1697cfd 100644
--- a/vcl/source/gdi/pdfwriter_impl.cxx
+++ b/vcl/source/gdi/pdfwriter_impl.cxx
@@ -6807,8 +6807,52 @@ typedef BOOL (WINAPI *PointerTo_CryptRetrieveTimeStamp)(LPCWSTR wszUrl,
namespace
{
+
+/// Counts how many bytes are needed to encode a given length.
+size_t GetDERLengthOfLength(size_t nLength)
+{
+ size_t nRet = 1;
+
+ if(nLength > 127)
+ {
+ while (nLength >> (nRet * 8))
+ ++nRet;
+ // Long form means one additional byte: the length of the length and
+ // the length itself.
+ ++nRet;
+ }
+ return nRet;
+}
+
+/// Writes the length part of the header.
+void WriteDERLength(SvStream& rStream, size_t nLength)
+{
+ size_t nLengthOfLength = GetDERLengthOfLength(nLength);
+ if (nLengthOfLength == 1)
+ {
+ // We can use the short form.
+ rStream.WriteUInt8(nLength);
+ return;
+ }
+
+ // 0x80 means that the we use the long form: the first byte is the length
+ // of length with the highest bit set to 1, not the actual length.
+ rStream.WriteUInt8(0x80 | (nLengthOfLength - 1));
+ for (size_t i = 1; i < nLengthOfLength; ++i)
+ rStream.WriteUInt8(nLength >> ((nLengthOfLength - i - 1) * 8));
+}
+
+const unsigned nASN1_INTEGER = 0x02;
+const unsigned nASN1_OCTET_STRING = 0x04;
+const unsigned nASN1_NULL = 0x05;
+const unsigned nASN1_OBJECT_IDENTIFIER = 0x06;
+const unsigned nASN1_SEQUENCE = 0x10;
+/// An explicit tag on a constructed value.
+const unsigned nASN1_TAGGED_CONSTRUCTED = 0xa0;
+const unsigned nASN1_CONSTRUCTED = 0x20;
+
/// Create payload for the 'signing-certificate' signed attribute.
-bool CreateSigningCertificateAttribute(vcl::PDFWriter::PDFSignContext& rContext, SvStream& rEncodedCertificate)
+bool CreateSigningCertificateAttribute(vcl::PDFWriter::PDFSignContext& rContext, PCCERT_CONTEXT pCertContext, SvStream& rEncodedCertificate)
{
// CryptEncodeObjectEx() does not support encoding arbitrary ASN.1
// structures, like SigningCertificateV2 from RFC 5035, so let's build it
@@ -6855,46 +6899,79 @@ bool CreateSigningCertificateAttribute(vcl::PDFWriter::PDFSignContext& rContext,
CryptDestroyHash(hHash);
CryptReleaseContext(hProv, 0);
+ // Collect info for IssuerSerial.
+ BYTE* pIssuer = pCertContext->pCertInfo->Issuer.pbData;
+ DWORD nIssuer = pCertContext->pCertInfo->Issuer.cbData;
+ BYTE* pSerial = pCertContext->pCertInfo->SerialNumber.pbData;
+ DWORD nSerial = pCertContext->pCertInfo->SerialNumber.cbData;
+ // pSerial is LE, aSerial is BE.
+ std::vector<BYTE> aSerial(nSerial);
+ for (size_t i = 0; i < nSerial; ++i)
+ aSerial[i] = *(pSerial + nSerial - i - 1);
+
// We now have all the info to count the lengths.
// The layout of the payload is:
// SEQUENCE: SigningCertificateV2
// SEQUENCE: SEQUENCE OF ESSCertIDv2
- // SEQUENCE: ESSCertIDv2
- // SEQUENCE: AlgorithmIdentifier
- // OBJECT: algorithm
- // NULL: parameters
- // OCTET STRING: certHash
-
- size_t nAlgorithm = aSHA256.size() + 2;
- size_t nParameters = 2;
- size_t nAlgorithmIdentifier = nAlgorithm + nParameters + 2;
- size_t nCertHash = aHash.size() + 2;
- size_t nESSCertIDv2 = nAlgorithmIdentifier + nCertHash + 2;
- size_t nESSCertIDv2s = nESSCertIDv2 + 2;
+ // SEQUENCE: ESSCertIDv2
+ // SEQUENCE: AlgorithmIdentifier
+ // OBJECT: algorithm
+ // NULL: parameters
+ // OCTET STRING: certHash
+ // SEQUENCE: IssuerSerial
+ // SEQUENCE: GeneralNames
+ // cont [ 4 ]: Name
+ // SEQUENCE: Issuer blob
+ // INTEGER: CertificateSerialNumber
+
+ size_t nAlgorithm = 1 + GetDERLengthOfLength(aSHA256.size()) + aSHA256.size();
+ size_t nParameters = 1 + GetDERLengthOfLength(1);
+ size_t nAlgorithmIdentifier = 1 + GetDERLengthOfLength(nAlgorithm + nParameters) + nAlgorithm + nParameters;
+ size_t nCertHash = 1 + GetDERLengthOfLength(aHash.size()) + aHash.size();
+ size_t nName = 1 + GetDERLengthOfLength(nIssuer) + nIssuer;
+ size_t nGeneralNames = 1 + GetDERLengthOfLength(nName) + nName;
+ size_t nCertificateSerialNumber = 1 + GetDERLengthOfLength(nSerial) + nSerial;
+ size_t nIssuerSerial = 1 + GetDERLengthOfLength(nGeneralNames + nCertificateSerialNumber) + nGeneralNames + nCertificateSerialNumber;
+ size_t nESSCertIDv2 = 1 + GetDERLengthOfLength(nAlgorithmIdentifier + nCertHash + nIssuerSerial) + nAlgorithmIdentifier + nCertHash + nIssuerSerial;
+ size_t nESSCertIDv2s = 1 + GetDERLengthOfLength(nESSCertIDv2) + nESSCertIDv2;
// Write SigningCertificateV2.
- rEncodedCertificate.WriteUInt8(0x30);
- rEncodedCertificate.WriteUInt8(nESSCertIDv2s);
+ rEncodedCertificate.WriteUInt8(nASN1_SEQUENCE | nASN1_CONSTRUCTED);
+ WriteDERLength(rEncodedCertificate, nESSCertIDv2s);
// Write SEQUENCE OF ESSCertIDv2.
- rEncodedCertificate.WriteUInt8(0x30);
- rEncodedCertificate.WriteUInt8(nESSCertIDv2);
+ rEncodedCertificate.WriteUInt8(nASN1_SEQUENCE | nASN1_CONSTRUCTED);
+ WriteDERLength(rEncodedCertificate, nESSCertIDv2);
// Write ESSCertIDv2.
- rEncodedCertificate.WriteUInt8(0x30);
- rEncodedCertificate.WriteUInt8(nAlgorithmIdentifier + nCertHash);
+ rEncodedCertificate.WriteUInt8(nASN1_SEQUENCE | nASN1_CONSTRUCTED);
+ WriteDERLength(rEncodedCertificate, nAlgorithmIdentifier + nCertHash + nIssuerSerial);
// Write AlgorithmIdentifier.
- rEncodedCertificate.WriteUInt8(0x30);
- rEncodedCertificate.WriteUInt8(nAlgorithm + nParameters);
+ rEncodedCertificate.WriteUInt8(nASN1_SEQUENCE | nASN1_CONSTRUCTED);
+ WriteDERLength(rEncodedCertificate, nAlgorithm + nParameters);
// Write algorithm.
- rEncodedCertificate.WriteUInt8(0x06);
- rEncodedCertificate.WriteUInt8(aSHA256.size());
+ rEncodedCertificate.WriteUInt8(nASN1_OBJECT_IDENTIFIER);
+ WriteDERLength(rEncodedCertificate, aSHA256.size());
rEncodedCertificate.WriteBytes(aSHA256.data(), aSHA256.size());
// Write parameters.
- rEncodedCertificate.WriteUInt8(0x05);
- rEncodedCertificate.WriteUInt8(0x00);
+ rEncodedCertificate.WriteUInt8(nASN1_NULL);
+ rEncodedCertificate.WriteUInt8(0);
// Write certHash.
- rEncodedCertificate.WriteUInt8(0x04);
- rEncodedCertificate.WriteUInt8(aHash.size());
+ rEncodedCertificate.WriteUInt8(nASN1_OCTET_STRING);
+ WriteDERLength(rEncodedCertificate, aHash.size());
rEncodedCertificate.WriteBytes(aHash.data(), aHash.size());
+ // Write IssuerSerial.
+ rEncodedCertificate.WriteUInt8(nASN1_SEQUENCE | nASN1_CONSTRUCTED);
+ WriteDERLength(rEncodedCertificate, nGeneralNames + nCertificateSerialNumber);
+ // Write GeneralNames.
+ rEncodedCertificate.WriteUInt8(nASN1_SEQUENCE | nASN1_CONSTRUCTED);
+ WriteDERLength(rEncodedCertificate, nName);
+ // Write Name.
+ rEncodedCertificate.WriteUInt8(nASN1_TAGGED_CONSTRUCTED | 4);
+ WriteDERLength(rEncodedCertificate, nIssuer);
+ rEncodedCertificate.WriteBytes(pIssuer, nIssuer);
+ // Write CertificateSerialNumber.
+ rEncodedCertificate.WriteUInt8(nASN1_INTEGER);
+ WriteDERLength(rEncodedCertificate, nSerial);
+ rEncodedCertificate.WriteBytes(aSerial.data(), aSerial.size());
return true;
}
@@ -7436,7 +7513,7 @@ bool PDFWriter::Sign(PDFSignContext& rContext)
// Add the signing certificate as a signed attribute.
CRYPT_INTEGER_BLOB aCertificateBlob;
SvMemoryStream aEncodedCertificate;
- if (!CreateSigningCertificateAttribute(rContext, aEncodedCertificate))
+ if (!CreateSigningCertificateAttribute(rContext, pCertContext, aEncodedCertificate))
{
SAL_WARN("vcl.pdfwriter", "CreateSigningCertificateAttribute() failed");
return false;
diff --git a/xmlsecurity/qa/unit/pdfsigning/data/good-pades.pdf b/xmlsecurity/qa/unit/pdfsigning/data/good-pades.pdf
new file mode 100644
index 0000000..987169e
Binary files /dev/null and b/xmlsecurity/qa/unit/pdfsigning/data/good-pades.pdf differ
diff --git a/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx b/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx
index 4d0ce52..dbe3319 100644
--- a/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx
+++ b/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx
@@ -62,6 +62,8 @@ public:
void testPDF16Add();
/// Test a PDF 1.4 document, signed by LO on Windows.
void testPDF14LOWin();
+ /// Test a PAdES document, signed by LO on Linux.
+ void testPDFPAdESGood();
CPPUNIT_TEST_SUITE(PDFSigningTest);
CPPUNIT_TEST(testPDFAdd);
@@ -72,6 +74,7 @@ public:
CPPUNIT_TEST(testPDF16Adobe);
CPPUNIT_TEST(testPDF16Add);
CPPUNIT_TEST(testPDF14LOWin);
+ CPPUNIT_TEST(testPDFPAdESGood);
CPPUNIT_TEST_SUITE_END();
};
@@ -314,6 +317,11 @@ void PDFSigningTest::testPDF14LOWin()
verify(m_directories.getURLFromSrc(DATA_DIRECTORY) + "pdf14lowin.pdf", 1, /*rExpectedSubFilter=*/OString());
}
+void PDFSigningTest::testPDFPAdESGood()
+{
+ verify(m_directories.getURLFromSrc(DATA_DIRECTORY) + "good-pades.pdf", 1, "ETSI.CAdES.detached");
+}
+
CPPUNIT_TEST_SUITE_REGISTRATION(PDFSigningTest);
CPPUNIT_PLUGIN_IMPLEMENT();
diff --git a/xmlsecurity/qa/unit/signing/data/good-xades.odt b/xmlsecurity/qa/unit/signing/data/good-xades.odt
new file mode 100644
index 0000000..4f96d4b
Binary files /dev/null and b/xmlsecurity/qa/unit/signing/data/good-xades.odt differ
diff --git a/xmlsecurity/qa/unit/signing/data/no.odt b/xmlsecurity/qa/unit/signing/data/no.odt
new file mode 100644
index 0000000..22cf768
Binary files /dev/null and b/xmlsecurity/qa/unit/signing/data/no.odt differ
diff --git a/xmlsecurity/qa/unit/signing/signing.cxx b/xmlsecurity/qa/unit/signing/signing.cxx
index 5bcf0c2..8ddc39b 100644
--- a/xmlsecurity/qa/unit/signing/signing.cxx
+++ b/xmlsecurity/qa/unit/signing/signing.cxx
@@ -69,6 +69,8 @@ public:
void testODFGood();
/// Test a typical broken ODF signature where one stream is corrupted.
void testODFBroken();
+ /// Document has a signature stream, but no actual signatures.
+ void testODFNo();
/// Test a typical OOXML where a number of (but not all) streams are signed.
void testOOXMLPartial();
/// Test a typical broken OOXML signature where one stream is corrupted.
@@ -90,12 +92,16 @@ public:
#endif
void test96097Calc();
void test96097Doc();
+ /// Creates a XAdES signature from scratch.
void testXAdES();
+ /// Works with an existing good XAdES signature.
+ void testXAdESGood();
CPPUNIT_TEST_SUITE(SigningTest);
CPPUNIT_TEST(testDescription);
CPPUNIT_TEST(testODFGood);
CPPUNIT_TEST(testODFBroken);
+ CPPUNIT_TEST(testODFNo);
CPPUNIT_TEST(testODFBroken);
CPPUNIT_TEST(testOOXMLPartial);
CPPUNIT_TEST(testOOXMLBroken);
@@ -111,6 +117,7 @@ public:
CPPUNIT_TEST(test96097Calc);
CPPUNIT_TEST(test96097Doc);
CPPUNIT_TEST(testXAdES);
+ CPPUNIT_TEST(testXAdESGood);
CPPUNIT_TEST_SUITE_END();
private:
@@ -380,6 +387,16 @@ void SigningTest::testODFBroken()
CPPUNIT_ASSERT_EQUAL(static_cast<int>(SignatureState::BROKEN), static_cast<int>(pObjectShell->GetDocumentSignatureState()));
}
+void SigningTest::testODFNo()
+{
+ createDoc(m_directories.getURLFromSrc(DATA_DIRECTORY) + "no.odt");
+ SfxBaseModel* pBaseModel = dynamic_cast<SfxBaseModel*>(mxComponent.get());
+ CPPUNIT_ASSERT(pBaseModel);
+ SfxObjectShell* pObjectShell = pBaseModel->GetObjectShell();
+ CPPUNIT_ASSERT(pObjectShell);
+ CPPUNIT_ASSERT_EQUAL(static_cast<int>(SignatureState::NOSIGNATURES), static_cast<int>(pObjectShell->GetDocumentSignatureState()));
+}
+
void SigningTest::testOOXMLPartial()
{
createDoc(m_directories.getURLFromSrc(DATA_DIRECTORY) + "partial.docx");
@@ -591,6 +608,22 @@ void SigningTest::testXAdES()
assertXPath(pXmlDoc, "//xd:CertDigest", 1);
}
+void SigningTest::testXAdESGood()
+{
+ createDoc(m_directories.getURLFromSrc(DATA_DIRECTORY) + "good-xades.odt");
+ SfxBaseModel* pBaseModel = dynamic_cast<SfxBaseModel*>(mxComponent.get());
+ CPPUNIT_ASSERT(pBaseModel);
+ SfxObjectShell* pObjectShell = pBaseModel->GetObjectShell();
+ CPPUNIT_ASSERT(pObjectShell);
+ // We expect NOTVALIDATED in case the root CA is not imported on the system, and OK otherwise, so accept both.
+ SignatureState nActual = pObjectShell->GetDocumentSignatureState();
+ CPPUNIT_ASSERT_MESSAGE(
+ (OString::number(
+ static_cast<std::underlying_type<SignatureState>::type>(nActual))
+ .getStr()),
+ (nActual == SignatureState::NOTVALIDATED
+ || nActual == SignatureState::OK));
+}
void SigningTest::registerNamespaces(xmlXPathContextPtr& pXmlXpathCtx)
{
xmlXPathRegisterNs(pXmlXpathCtx, BAD_CAST("odfds"), BAD_CAST("urn:oasis:names:tc:opendocument:xmlns:digitalsignature:1.0"));
diff --git a/xmlsecurity/source/pdfio/pdfdocument.cxx b/xmlsecurity/source/pdfio/pdfdocument.cxx
index 29b4a02..aeea58d 100644
--- a/xmlsecurity/source/pdfio/pdfdocument.cxx
+++ b/xmlsecurity/source/pdfio/pdfdocument.cxx
@@ -2315,6 +2315,28 @@ bool PDFDocument::ValidateSignature(SvStream& rStream, PDFObjectElement* pSignat
return false;
}
+ // Get the CRYPT_ALGORITHM_IDENTIFIER from the message.
+ DWORD nDigestID = 0;
+ if (!CryptMsgGetParam(hMsg, CMSG_SIGNER_HASH_ALGORITHM_PARAM, 0, nullptr, &nDigestID))
+ {
+ SAL_WARN("xmlsecurity.pdfio", "PDFDocument::ValidateSignature: CryptMsgGetParam() failed: " << WindowsErrorString(GetLastError()));
+ return false;
+ }
+ std::unique_ptr<BYTE[]> pDigestBytes(new BYTE[nDigestID]);
+ if (!CryptMsgGetParam(hMsg, CMSG_SIGNER_HASH_ALGORITHM_PARAM, 0, pDigestBytes.get(), &nDigestID))
+ {
+ SAL_WARN("xmlsecurity.pdfio", "PDFDocument::ValidateSignature: CryptMsgGetParam() failed: " << WindowsErrorString(GetLastError()));
+ return false;
+ }
+ auto pDigestID = reinterpret_cast<CRYPT_ALGORITHM_IDENTIFIER*>(pDigestBytes.get());
+ if (OString(szOID_NIST_sha256) == pDigestID->pszObjId)
+ rInformation.nDigestID = xml::crypto::DigestID::SHA256;
+ else if (OString(szOID_RSA_SHA1RSA) == pDigestID->pszObjId)
+ rInformation.nDigestID = xml::crypto::DigestID::SHA1;
+ else
+ // Don't error out here, we can still verify the message digest correctly, just the digest ID won't be set.
+ SAL_WARN("xmlsecurity.pdfio", "PDFDocument::ValidateSignature: unhandled algorithm identifier '"<<pDigestID->pszObjId<<"'");
+
// Get the signer CERT_INFO from the message.
DWORD nSignerCertInfo = 0;
if (!CryptMsgGetParam(hMsg, CMSG_SIGNER_CERT_INFO_PARAM, 0, nullptr, &nSignerCertInfo))
More information about the Libreoffice-commits
mailing list