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

signature dictionary "Contents" must not be encrypted #794

Closed
fancycode opened this issue Jan 29, 2024 · 11 comments
Closed

signature dictionary "Contents" must not be encrypted #794

fancycode opened this issue Jan 29, 2024 · 11 comments
Assignees

Comments

@fancycode
Copy link
Contributor

For signature fields in encrypted documents, all entries in the /Sig dictionary are currently kept encrypted:

pdfcpu/pkg/pdfcpu/crypto.go

Lines 1199 to 1200 in 793c509

if ftv, ok := ft.(types.Name); ok && ftv == "Sig" {
return nil

According to the PDF specification, only the /Contents entry must be kept as-is. All other fields must be decrypted.

See "7.6.2 Application of encryption":

Encryption applies to all strings and streams in the document's PDF file, with the following exceptions:
• The values for the ID entry in the trailer
• Any strings in an Encrypt dictionary
• Any strings that are inside streams such as content streams and compressed object streams, which
themselves are encrypted
• Any hexadecimal strings representing the value of the Contents key in a Signature dictionary

Simple patch:

diff --git a/pkg/pdfcpu/crypto.go b/pkg/pdfcpu/crypto.go
index 7412bfd..8a56549 100644
--- a/pkg/pdfcpu/crypto.go
+++ b/pkg/pdfcpu/crypto.go
@@ -1191,16 +1191,20 @@ func encryptDeepObject(objIn types.Object, objNr, genNr int, key []byte, needAES
 }
 
 func decryptDict(d types.Dict, objNr, genNr int, key []byte, needAES bool, r int) error {
+       isSignature := false
        ft := d["FT"]
        if ft == nil {
                ft = d["Type"]
        }
        if ft != nil {
                if ftv, ok := ft.(types.Name); ok && ftv == "Sig" {
-                       return nil
+                       isSignature = true
                }
        }
        for k, v := range d {
+               if isSignature && k == "Contents" {
+                       continue
+               }
                s, err := decryptDeepObject(v, objNr, genNr, key, needAES, r)
                if err != nil {
                        return err

I'm happy to provide this as MR if interested.

@hhrutter
Copy link
Collaborator

What is your use case and how does the current behavior interfere negatively with your work,
other than being a PDF spec issue?

@fancycode
Copy link
Contributor Author

fancycode commented Jan 29, 2024

I'm looking into signature support and found an issue with an encrypted and signed document where the certificate of a signature (in /Cert) was invalid because it was not decrypted.

@hhrutter
Copy link
Collaborator

FYI Signature support is evolving and will be the main task before going Beta once we have better support for PDF 2.0.

@fancycode
Copy link
Contributor Author

FYI Signature support is evolving and will be the main task before going Beta once we have better support for PDF 2.0.

Great news! Is there a place where can I check the existing signature support? The sign branch here seems to be 4 years old.

Resolving this issue is trivial, so I figured to report it here and provide a patch if you're interested in fixing it.

@hhrutter
Copy link
Collaborator

Very much appreciated and I agree this fix needs to go in.

The sign branch was an initial stab but since we are in the process of integrating PDF 2.0 support
and that specification came with plenty of updates to encryption and signature handling implementing support for digital signatures was pushed back to the end of this.

So this will take some time I am afraid.
My focus on this topic is getting the design right first and then provide an as much complete as can be implementation.

@hhrutter
Copy link
Collaborator

This should be fixed with latest commit!

@hhrutter
Copy link
Collaborator

Nope, sorry my bad - still open.

@hhrutter hhrutter reopened this Feb 22, 2024
@fancycode
Copy link
Contributor Author

Should I provide the change as merge request?

@hhrutter
Copy link
Collaborator

No need thx, this goes in today - new release imminent.

@hhrutter
Copy link
Collaborator

This is fixed with the latest commit.

@hhrutter hhrutter changed the title Entries in signature dictionary must be decrypted signature dictionary "Contents" must not be encrypted Feb 28, 2024
@fancycode
Copy link
Contributor Author

This is fixed with the latest commit.

Perfect, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants