tdf#84881: Try to fix "The signature includes an embedded timestamp but ...

... it could not be verified"

I got some insight reading this question and reply on stackoverflow:
http://stackoverflow.com/questions/18761993/steps-to-include-timestamp-in-pdf-signature

I had been doing the timestamping wrong in the same way: I had timestamped the
hash of the PDF document, not of the signature. That is wrong. If you think
hard, it is obvious: It is the (rest of the) signature that needs an
authenticated timestamp, not the PDF document contents. After all, if the
document contents is timestamped, but not the signature, that doesn't prevent
tampering with the signature after the timestamping. When you timestamp the
signature, that proves the date of the signature. (And the signature proves
the authenticity of the document contents.)

So I had to re-engineer the code a bit. I create two originally identical NSS
CMS messages with signatures, encode one signature into DER, take the hash of
the signature, get a timestamp from the TSA for that hash. Then I add that
timestamp to the other CMS message as an unsigned attribute of its signature,
sign it, encode it, convert to hex, and store it the document.

(I first tried to use just one CMS message, but NSS stopped with an assertion
when I tried to encode the signature of the same message a second time, after
adding the timestamp attribute to the signature. Go figure.)

(I did verify the the encoded signatures, taken from what should be identical
but separate CMS messages, was in fact identical. So I am fairly sure the idea
described above is sound.)

But, it doesn't help. Adobe Reader still complains "The signature includes an
embedded timestamp but it could not be verified".

Change-Id: I4e4cd0443005e82f597586942badc7145ef64160
(cherry picked from commit 86796f127b15fd33374f2a18344dc944b7b8314d)

1 files changed, 131 insertions, 16 deletions

diff --git a/vcl/source/gdi/pdfwriter_impl.cxx b/vcl/source/gdi/pdfwriter_impl.cxx
index c0fdbf74b830..4b884bf8e5b6 100644
--- a/vcl/source/gdi/pdfwriter_impl.cxx
+++ b/vcl/source/gdi/pdfwriter_impl.cxx
@@ -6198,6 +6198,31 @@ RevocationInfoChoice ::= CHOICE {
 
 RevocationInfoChoices ::= SET OF RevocationInfoChoice
 
+SignerIdentifier ::= CHOICE {
+    issuerAndSerialNumber IssuerAndSerialNumber,
+    subjectKeyIdentifier [0] SubjectKeyIdentifier }
+
+AttributeValue ::= ANY
+
+Attribute ::= SEQUENCE {
+    attrType OBJECT IDENTIFIER,
+    attrValues SET OF AttributeValue }
+
+SignedAttributes ::= SET SIZE (1..MAX) OF Attribute
+
+SignatureValue ::= OCTET STRING
+
+UnsignedAttributes ::= SET SIZE (1..MAX) OF Attribute
+
+SignerInfo ::= SEQUENCE {
+    version CMSVersion,
+    sid SignerIdentifier,
+    digestAlgorithm DigestAlgorithmIdentifier,
+    signedAttrs [0] IMPLICIT SignedAttributes OPTIONAL,
+    signatureAlgorithm SignatureAlgorithmIdentifier,
+    signature SignatureValue,
+    unsignedAttrs [1] IMPLICIT UnsignedAttributes OPTIONAL }
+
 SignerInfos ::= SET OF SignerInfo
 
 SignedData ::= SEQUENCE {
@@ -6592,7 +6617,8 @@ my_NSS_CMSSignerInfo_AddUnauthAttr(NSSCMSSignerInfo *signerinfo, NSSCMSAttribute
     return my_NSS_CMSAttributeArray_AddAttr(signerinfo->cmsg->poolp, &(signerinfo->unAuthAttr), attr);
 }
 
-NSSCMSMessage *CreateCMSMessage(NSSCMSSignedData **cms_sd,
+NSSCMSMessage *CreateCMSMessage(PRTime time,
+                                NSSCMSSignedData **cms_sd,
                                 NSSCMSSignerInfo **cms_signer,
                                 CERTCertificate *cert,
                                 SECItem *digest)
@@ -6641,7 +6667,7 @@ NSSCMSMessage *CreateCMSMessage(NSSCMSSignedData **cms_sd,
         return NULL;
     }
 
-    if (NSS_CMSSignerInfo_AddSigningTime(*cms_signer, PR_Now()) != SECSuccess)
+    if (NSS_CMSSignerInfo_AddSigningTime(*cms_signer, time) != SECSuccess)
     {
         SAL_WARN("vcl.pdfwriter", "NSS_CMSSignerInfo_AddSigningTime failed");
         NSS_CMSSignedData_Destroy(*cms_sd);
@@ -6803,20 +6829,93 @@ bool PDFWriterImpl::finalizeSignature()
     HASH_End(hc.get(), digest.data, &digest.len, SHA1_LENGTH);
     hc.clear();
 
+#ifdef DBG_UTIL
+    {
+        FILE *out = fopen("PDFWRITER.hash.data", "wb");
+        fwrite(hash, SHA1_LENGTH, 1, out);
+        fclose(out);
+    }
+#endif
+
+    PRTime now = PR_Now();
     NSSCMSSignedData *cms_sd;
     NSSCMSSignerInfo *cms_signer;
-    NSSCMSMessage *cms_msg = CreateCMSMessage(&cms_sd, &cms_signer, cert, &digest);
+    NSSCMSMessage *cms_msg = CreateCMSMessage(now, &cms_sd, &cms_signer, cert, &digest);
     if (!cms_msg)
         return false;
 
+    char *pass(strdup(OUStringToOString( m_aContext.SignPassword, RTL_TEXTENCODING_UTF8 ).getStr()));
+
     NSSCMSAttribute timestamp;
     SECItem values[2];
     SECItem *valuesp = values;
     SECOidData typetag;
 
-    // Now we have the hash algorithm as a SECItem available in cms_siger->digestAlg
     if( !m_aContext.SignTSA.isEmpty() )
     {
+        // Create another CMS message with the same contents as cms_msg, because it doesn't seem
+        // possible to encode a message twice (once to get something to timestamp, and then after
+        // adding the timestamp attribute).
+
+        NSSCMSSignedData *ts_cms_sd;
+        NSSCMSSignerInfo *ts_cms_signer;
+        NSSCMSMessage *ts_cms_msg = CreateCMSMessage(now, &ts_cms_sd, &ts_cms_signer, cert, &digest);
+        if (!ts_cms_msg)
+        {
+            free(pass);
+            return false;
+        }
+
+        SECItem ts_cms_output;
+        ts_cms_output.data = 0;
+        ts_cms_output.len = 0;
+        PLArenaPool *ts_arena = PORT_NewArena(MAX_SIGNATURE_CONTENT_LENGTH);
+        NSSCMSEncoderContext *ts_cms_ecx;
+        ts_cms_ecx = NSS_CMSEncoder_Start(ts_cms_msg, NULL, NULL, &ts_cms_output, ts_arena, PDFSigningPKCS7PasswordCallback, pass, NULL, NULL, NULL, NULL);
+
+        if (NSS_CMSEncoder_Finish(ts_cms_ecx) != SECSuccess)
+        {
+            SAL_WARN("vcl.pdfwriter", "NSS_CMSEncoder_Finish failed");
+            free(pass);
+            return false;
+        }
+
+        // I have compared the ts_cms_output produced here with the cms_output produced below, with
+        // the DONTCALLADDUNAUTHATTR env var set (i.e. without actually calling
+        // my_NSS_CMSSignerInfo_AddUnauthAttr()), and they are identical.
+
+#ifdef DBG_UTIL
+        {
+            FILE *out = fopen("PDFWRITER.ts_cms.data", "wb");
+            fwrite(ts_cms_output.data, ts_cms_output.len, 1, out);
+            fclose(out);
+        }
+#endif
+
+        HashContextScope ts_hc(HASH_Create(HASH_AlgSHA1));
+        if (!ts_hc.get())
+        {
+            SAL_WARN("vcl.pdfwriter", "HASH_Create failed");
+            free(pass);
+            return false;
+        }
+
+        HASH_Begin(ts_hc.get());
+        HASH_Update(ts_hc.get(), reinterpret_cast<const unsigned char*>(ts_cms_output.data), ts_cms_output.len);
+        SECItem ts_digest;
+        unsigned char ts_hash[SHA1_LENGTH];
+        ts_digest.type = siBuffer;
+        ts_digest.data = ts_hash;
+        HASH_End(ts_hc.get(), ts_digest.data, &ts_digest.len, SHA1_LENGTH);
+        ts_hc.clear();
+
+#ifdef DBG_UTIL
+        {
+            FILE *out = fopen("PDFWRITER.ts_hash.data", "wb");
+            fwrite(ts_hash, SHA1_LENGTH, 1, out);
+            fclose(out);
+        }
+#endif
         TimeStampReq src;
 
         unsigned char cOne = 1;
@@ -6824,8 +6923,8 @@ bool PDFWriterImpl::finalizeSignature()
         src.version.data = &cOne;
         src.version.len = sizeof(cOne);
 
-        src.messageImprint.hashAlgorithm = cms_signer->digestAlg;
-        src.messageImprint.hashedMessage = digest;
+        src.messageImprint.hashAlgorithm = ts_cms_signer->digestAlg;
+        src.messageImprint.hashedMessage = ts_digest;
 
         src.reqPolicy.type = siBuffer;
         src.reqPolicy.data = NULL;
@@ -6846,12 +6945,14 @@ bool PDFWriterImpl::finalizeSignature()
         if (timestamp_request == NULL)
         {
             SAL_WARN("vcl.pdfwriter", "SEC_ASN1EncodeItem failed");
+            free(pass);
             return false;
         }
 
         if (timestamp_request->data == NULL)
         {
             SAL_WARN("vcl.pdfwriter", "SEC_ASN1EncodeItem succeeded but got NULL data");
+            free(pass);
             SECITEM_FreeItem(timestamp_request, PR_TRUE);
             return false;
         }
@@ -6875,6 +6976,7 @@ bool PDFWriterImpl::finalizeSignature()
         if (!curl)
         {
             SAL_WARN("vcl.pdfwriter", "curl_easy_init failed");
+            free(pass);
             SECITEM_FreeItem(timestamp_request, PR_TRUE);
             return false;
         }
@@ -6884,6 +6986,7 @@ bool PDFWriterImpl::finalizeSignature()
         if ((rc = curl_easy_setopt(curl, CURLOPT_URL, OUStringToOString(m_aContext.SignTSA, RTL_TEXTENCODING_UTF8).getStr())) != CURLE_OK)
         {
             SAL_WARN("vcl.pdfwriter", "curl_easy_setopt(CURLOPT_URL) failed: " << curl_easy_strerror(rc));
+            free(pass);
             curl_easy_cleanup(curl);
             SECITEM_FreeItem(timestamp_request, PR_TRUE);
             return false;
@@ -6895,6 +6998,7 @@ bool PDFWriterImpl::finalizeSignature()
         if ((rc = curl_easy_setopt(curl, CURLOPT_HTTPHEADER, slist)) != CURLE_OK)
         {
             SAL_WARN("vcl.pdfwriter", "curl_easy_setopt(CURLOPT_HTTPHEADER) failed: " << curl_easy_strerror(rc));
+            free(pass);
             curl_slist_free_all(slist);
             curl_easy_cleanup(curl);
             SECITEM_FreeItem(timestamp_request, PR_TRUE);
@@ -6905,6 +7009,7 @@ bool PDFWriterImpl::finalizeSignature()
             (rc = curl_easy_setopt(curl, CURLOPT_POSTFIELDS, timestamp_request->data)) != CURLE_OK)
         {
             SAL_WARN("vcl.pdfwriter", "curl_easy_setopt(CURLOPT_POSTFIELDSIZE or CURLOPT_POSTFIELDS) failed: " << curl_easy_strerror(rc));
+            free(pass);
             curl_easy_cleanup(curl);
             SECITEM_FreeItem(timestamp_request, PR_TRUE);
             return false;
@@ -6916,6 +7021,7 @@ bool PDFWriterImpl::finalizeSignature()
             (rc = curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, AppendToBuffer)) != CURLE_OK)
         {
             SAL_WARN("vcl.pdfwriter", "curl_easy_setopt(CURLOPT_WRITEDATA or CURLOPT_WRITEFUNCTION) failed: " << curl_easy_strerror(rc));
+            free(pass);
             curl_easy_cleanup(curl);
             SECITEM_FreeItem(timestamp_request, PR_TRUE);
             return false;
@@ -6924,6 +7030,7 @@ bool PDFWriterImpl::finalizeSignature()
         if ((rc = curl_easy_setopt(curl, CURLOPT_POST, 1l)) != CURLE_OK)
         {
             SAL_WARN("vcl.pdfwriter", "curl_easy_setopt(CURLOPT_POST) failed: " << curl_easy_strerror(rc));
+            free(pass);
             curl_easy_cleanup(curl);
             SECITEM_FreeItem(timestamp_request, PR_TRUE);
             return false;
@@ -6933,6 +7040,7 @@ bool PDFWriterImpl::finalizeSignature()
         if ((rc = curl_easy_setopt(curl, CURLOPT_ERRORBUFFER, error_buffer)) != CURLE_OK)
         {
             SAL_WARN("vcl.pdfwriter", "curl_easy_setopt(CURLOPT_ERRORBUFFER) failed: " << curl_easy_strerror(rc));
+            free(pass);
             curl_easy_cleanup(curl);
             SECITEM_FreeItem(timestamp_request, PR_TRUE);
             return false;
@@ -6943,6 +7051,7 @@ bool PDFWriterImpl::finalizeSignature()
             (rc = curl_easy_setopt(curl, CURLOPT_CONNECTTIMEOUT, 10l)) != CURLE_OK)
         {
             SAL_WARN("vcl.pdfwriter", "curl_easy_setopt(CURLOPT_TIMEOUT or CURLOPT_CONNECTTIMEOUT) failed: " << curl_easy_strerror(rc));
+            free(pass);
             curl_easy_cleanup(curl);
             SECITEM_FreeItem(timestamp_request, PR_TRUE);
             return false;
@@ -6951,6 +7060,7 @@ bool PDFWriterImpl::finalizeSignature()
         if (curl_easy_perform(curl) != CURLE_OK)
         {
             SAL_WARN("vcl.pdfwriter", "curl_easy_perform failed: " << error_buffer);
+            free(pass);
             curl_easy_cleanup(curl);
             SECITEM_FreeItem(timestamp_request, PR_TRUE);
             return false;
@@ -6981,6 +7091,7 @@ bool PDFWriterImpl::finalizeSignature()
         if (SEC_ASN1DecodeItem(NULL, &response, TimeStampResp_Template, &response_item) != SECSuccess)
         {
             SAL_WARN("vcl.pdfwriter", "SEC_ASN1DecodeItem failed");
+            free(pass);
             return false;
         }
 
@@ -6990,6 +7101,7 @@ bool PDFWriterImpl::finalizeSignature()
             (response.status.status.data[0] != 0 && response.status.status.data[0] != 1))
         {
             SAL_WARN("vcl.pdfwriter", "Timestamp request was not granted");
+            free(pass);
             return false;
         }
 
@@ -7009,11 +7121,12 @@ bool PDFWriterImpl::finalizeSignature()
         if (my_SEC_StringToOID(NULL, &typetag.oid, "1.2.840.113549.1.9.16.2.14", 0) != SECSuccess)
         {
             SAL_WARN("vcl.pdfwriter", "SEC_StringToOID failed");
+            free(pass);
             return false;
         }
         typetag.offset = SEC_OID_UNKNOWN; // ???
         typetag.desc = "id-aa-timeStampToken";
-        typetag.mechanism = CKM_SHA256; // ???
+        typetag.mechanism = CKM_SHA_1; // ???
         typetag.supportedExtension = UNSUPPORTED_CERT_EXTENSION; // ???
         timestamp.typeTag = &typetag;
 
@@ -7021,28 +7134,30 @@ bool PDFWriterImpl::finalizeSignature()
 
         timestamp.encoded = PR_TRUE; // ???
 
+#ifdef DBG_UTIL
+        if (getenv("DONTCALLADDUNAUTHATTR"))
+            ;
+        else
+#endif
         if (my_NSS_CMSSignerInfo_AddUnauthAttr(cms_signer, &timestamp) != SECSuccess)
         {
             SAL_WARN("vcl.pdfwriter", "NSS_CMSSignerInfo_AddUnauthAttr failed");
+            free(pass);
             return false;
         }
     }
+
     SECItem cms_output;
     cms_output.data = 0;
     cms_output.len = 0;
     PLArenaPool *arena = PORT_NewArena(MAX_SIGNATURE_CONTENT_LENGTH);
     NSSCMSEncoderContext *cms_ecx;
 
-    //FIXME: Check if password is passed correctly to SEC_PKCS7CreateSignedData function
-
-    // Inded, it was not, I think, and that caused a crash as described in fdo#83937.
-    // Unfortunately I could not test this fix fully before my hardware token decided to
-    // block itself thanks to too many wrong PIN attempts. Possibly it would work to
-    // even just pass NULL for the password callback function and its argument here.
-    // After all, at least with the hardware token and associated software I tested
-    // with, the software itself pops up a dialog asking for the PIN (password).
+    // Possibly it would work to even just pass NULL for the password callback function and its
+    // argument here. After all, at least with the hardware token and associated software I tested
+    // with, the software itself pops up a dialog asking for the PIN (password). But I am not going
+    // to test it and risk locking up my token...
 
-    char *pass(strdup(OUStringToOString( m_aContext.SignPassword, RTL_TEXTENCODING_UTF8 ).getStr()));
     cms_ecx = NSS_CMSEncoder_Start(cms_msg, NULL, NULL, &cms_output, arena, PDFSigningPKCS7PasswordCallback, pass, NULL, NULL, NULL, NULL);
 
     if (!cms_ecx)