Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extend Signature Handling with Timestamp Support #3

Open
wants to merge 3 commits into
base: signature-feature
Choose a base branch
from

Conversation

quecot
Copy link

@quecot quecot commented Feb 13, 2024

This PR introduces timestamp support in PDFSharp-extended's signature handling.

Key Changes:

  • Added GetSignedCms method in DefaultSigner and BouncySigner (TODO) to support timestamp tokens.
  • Updated PdfSignatureHandler to process signatures with optional timestamp tokens.
  • Modified ISigner interface to accommodate the new method signature.

Benefits:
These enhancements enable the creation of verifiably timestamped signatures in PDF documents, meeting requirements for document integrity and authenticity in sensitive use cases.

Motivation:
Identified the need for timestamped signatures in a project and implemented this feature to fill the gap, hoping to contribute a valuable addition to the PDFSharp community.

I'm new to C# and open-source contributions and look forward to feedback and suggestions to refine this feature.

Thank you.

@julienrffr julienrffr added the enhancement New feature or request label Feb 13, 2024
if (timestampToken != null)
{
AsnEncodedData timestampTokenAsnEncodedData = new AsnEncodedData(new Oid("1.2.840.113549.1.9.16.2.14"), timestampToken);
signer.UnsignedAttributes.Add(new CryptographicAttributeObject(new Oid("1.2.840.113549.1.9.16.2.14"), new AsnEncodedDataCollection(timestampTokenAsnEncodedData)));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I read from https://datatracker.ietf.org/doc/html/rfc3126#section-3.12.4 that timestamp must be a signed attribute, but you put it in unsigned attributes. Is it an error or is there a reason?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, this need to be fixed.

The document you have referenced is obsoleted by RFC 5126 (CMS Advanced Electronic Signatures - CAdES), and Section 5.11.4 states that "The content-time-stamp attribute shall be a signed attribute".

The PAdES standard references CAdES in regards of timestamp attributes (Sections 5.2.2 and 5.3.3 of this document).

It says that signatures can optionally include timestamps, which are recomended.

@julienrffr
Copy link
Collaborator

Hi @quecot

Thank you for your contribution, it's an interesting feature that we would be interested in as well.

I've done an initial code review and left some remarks.

I have additional questions though:

  • What are you trying to achieve? PADES signatures?
  • Do you have a code sample? How do you generate the timestamp bytes?

@quecot
Copy link
Author

quecot commented Feb 13, 2024

Thank you for reviewing my contribution and for your feedback, @julienrffr. I will take into account the comments in the code review and update you on that soon.

Regarding your questions, my goal is to integrate timestamping into PDF signatures, utilizing a Time Stamp Authority (TSA). While I am not entirely familiar with all the nuances of PDF signature standards, I am eager to develop a solution that adheres to established protocols, potentially aligning with PAdES requirements.

For generating the timestamp, I have implemented a custom TimestampService that leverages the BouncyCastle.Cryptography library along with HTTP requests to interact with a TSA. You can find the code and an example of its use in this gist.

I am more than willing to adapt and integrate this code into the project if it aligns with the project's goals and standards.

@julienrffr
Copy link
Collaborator

@quecot

OK nice, PADES is a goal of ours too.

We could consider embedding the code from the TimestampService as well, as it uses the same lib (BouncyCastle) that is already used in this branch. It would have to be netstandard2.0 compatible though (I did not check).

What is the data 'dataToTimestamp' that is passed to this service exactly?

@quecot
Copy link
Author

quecot commented Feb 13, 2024

@julienrffr

As I said, I'm very new to C# and .NET so I don't know the answer to the netstandard2.0 compatibility question.
As per dataToTimestamp, you can pass the entire PDF as a byte[] but a more sensible choice is to simply pass a hash to it or to the relevant parts of the file.

This could be improved in the TimestampService or be left for users to decide.

As reference, the hash is obtained like this:

var digest = new Org.BouncyCastle.Crypto.Digests.Sha256Digest();
byte[] hash = new byte[digest.GetDigestSize()];
digest.BlockUpdate(dataToTimestamp, 0, dataToTimestamp.Length);
digest.DoFinal(hash, 0);

@julienrffr
Copy link
Collaborator

As per dataToTimestamp, you can pass the entire PDF as a byte[] but a more sensible choice is to simply pass a hash to it or to the relevant parts of the file.

This could be improved in the TimestampService or be left for users to decide.

We should dig the PADES specs to know what is the data to timestamp.
If you timestamp the entire document bytes before affixing the signature, it looks weird to me because hash of the document would change after affixing the signature.
Maybe the data to timespan would be the same as the data to sign: this means, the entire document minus the reserved space for signature. If this is the way to go, dataToTimestamp is actually the ByteRange we sign internally, so this service would fit in PDFsharp-extended.

Let me know if you find some answers for this.

@quecot quecot marked this pull request as draft February 14, 2024 09:41
@julienrffr
Copy link
Collaborator

By the way, please set branch https://github.com/KDS/PDFsharp/tree/signature-feature as the target for this PR.
This wat, if merged, it will be added to the PR we've done to original PDFsharp repo.

@quecot quecot changed the base branch from pdfsharp-extended to signature-feature February 14, 2024 11:05
@quecot
Copy link
Author

quecot commented Feb 14, 2024

Hello, as per your last question about dataToTimestamp, I've looked up the latest PAdES Standard publication, which states that:

The ByteRange shall cover the entire document, including the Document Time-stamp dictionary but excluding the TimeStampToken itself (the entry with key Contents). - Section 5.4.3.

This ensures the document's hash remains consistent before and after the signature is added. This approach aligns with your suggestion that the data to timestamp is essentially the document minus the reserved space for the signature.

@quecot quecot marked this pull request as ready for review February 14, 2024 12:17
@julienrffr
Copy link
Collaborator

julienrffr commented Feb 16, 2024

By the way, please set branch https://github.com/KDS/PDFsharp/tree/signature-feature as the target for this PR. This wat, if merged, it will be added to the PR we've done to original PDFsharp repo.

@quecot please rebase your branch on KDS:signature-feature
Your current branch is based on pdfsharp-extended branch so there are more changes than necessary.

image

@quecot
Copy link
Author

quecot commented Feb 19, 2024

Hi @julienrffr, I'm not very experienced with rebasing and merge conflict resolving.

I've tried to perform the rebase, but found many conflicts that need manual resolving and I don't want to mess anything up as I don't fully know the project.

Could you lend me a hand? Thanks

@julienrffr
Copy link
Collaborator

@quecot
It may be simplier to do the following commands on your branch:

  1. reset to state of signature-feature branch
    git reset --hard 13119693af3e67b98e215ca496ed06a164664130
  2. cherry pick your 3 commits in chronological order
    git cherry-pick 82b25c22adcf5b78ac0aee0dfde734ac49a4d55a
    git cherry-pick 3b9802653cf4fb8ab9d655040965e6ce700d09fe
    git cherry-pick 2164c05e8509213e1e928ad4eddffd722d0f9787
  3. push your new branch version with force
    git push --force

@quecot quecot force-pushed the signature-timestamping branch from 2164c05 to ab35a86 Compare February 19, 2024 11:05
@quecot
Copy link
Author

quecot commented Feb 19, 2024

@julienrffr
Done! Thanks for the instructions

@julienrffr
Copy link
Collaborator

Hi @quecot,
Could you provide a sample PDF document signed with your implementation so I can look at the signature timestamp?
By the way, are you interested by communicating by IM like Teams?

@julienrffr
Copy link
Collaborator

julienrffr commented Feb 22, 2024

I've made some reading and it appears tha tthe timestamp should actually be:

  • added to the signature as an unsigned attribute
  • it can be a hash of the signature value itself. TSA will sign this hash.

Space needed to host this timestamp token should be taken into account when we reserve some space in the PDF. This is already done because the timestamp is added to the signature in the GetSignedCms method.

Interesting reads to accomplish this:
https://github.com/itext/itext-java/blob/develop/sign/src/main/java/com/itextpdf/signatures/PdfPKCS7.java
https://stackoverflow.com/questions/35058723/how-to-timestamp-signature-create

Free TSA to test:
https://www.freetsa.org/index_en.php

Note for steps beyond: in order to achieve PADES compliant signing, we will also be missing the /Certs element in the DSS dictionary.
This /Certs element is supposed to embed all certificates implied (from OCSP responder, from signing certificate, from CRL, from TSA).

@julienrffr
Copy link
Collaborator

Hi @quecot

I've made some more tries, and thanks to this article I was able to implement a working timestamp feature for the DefaultSigner.
Please try the code I pushed on the parent branch: https://github.com/KDS/PDFsharp/tree/signature-feature

Just pass the TSA uri as a 2nd parameter to DefaultSigner, and it will retrieve and attach the timestamp to the signature.

Note: it seems that TSA http response content varies randomly between calls. I used "https://freetsa.org/tsr" (free TSA) for my tests, and response varies 1 byte from a call to another. It has to be fixed.

@quecot
Copy link
Author

quecot commented Feb 23, 2024

Hi, @julienrffr

I tried the code in the signature-feature branch and was able to verify it does work with my personal certificate and the TSA timestamp.

From Adobe Reader, I can see the signature is valid and includes the timestamp:
Screenshot 2024-02-23 at 19 45 41

Also the TSA certificate is embedded, too:
Screenshot 2024-02-23 at 19 47 48

However, when I run the signed PDF through this service, the signature fails validation, which does not happen with documents signed with iText7 with timestamp or with PDFsharp-extended without the timestamp.

Screenshot 2024-02-23 at 19 52 10

I don't fully know how the validation is performed in this website, but maybe this is something we can look into!

As per your request of PM communication and document sharing you can email me at [email protected], if that works for you.

Thanks for the shown interest and the hard work!

@quecot
Copy link
Author

quecot commented Feb 23, 2024

Hi again, I found this other service that provides a detailed report on signed documents, which I'm sure will be useful for our purposes:
https://ec.europa.eu/digital-building-blocks/DSS/webapp-demo/validation

@julienrffr
Copy link
Collaborator

Hi @quecot
What TSA do you use for the timestamp?
Maybe it is not recognized/trusted by fineid.fi.
Same as the screenshot from your PDF reader you provided, is the TSa certificate part of the trusted CAs of your computer?

@julienrffr
Copy link
Collaborator

Note: it seems that TSA http response content varies randomly between calls. I used "https://freetsa.org/tsr" (free TSA) for my tests, and response varies 1 byte from a call to another. It has to be fixed.

Fixed with last commit 8a90349

@quecot
Copy link
Author

quecot commented Mar 5, 2024

Hi @quecot What TSA do you use for the timestamp? Maybe it is not recognized/trusted by fineid.fi. Same as the screenshot from your PDF reader you provided, is the TSa certificate part of the trusted CAs of your computer?

Hi @julienrffr, I tried with both freetsa and globalsign. I believe the issue is not with the TSA certificate. It's fair if a warning appears if the TSA certificate is not trusted by my computer.

My concern is that validation tools like the ones I shared show more issues with the signature integrity in their reports.

@julienrffr
Copy link
Collaborator

@quecot
Does the https://ec.europa.eu/digital-building-blocks/DSS/webapp-demo/validation service shows the same message? Or more details?
Can you provide your signed PDF that is raising those messages?

@quecot
Copy link
Author

quecot commented Mar 5, 2024

Sure thing, please provide an email address or email me at [email protected]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants