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

External signature and partial PAdES support #57

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

ekzobrain
Copy link

Hi.

I've made some fixes and merged @moritzgloeckl's changes from #32 with sign() method for code cleanless. So external signature embding is currently supported.
Also added partial support for PAdES, it is currently validated as PKCS#7 because CAdES is backward compatible with PKCS#7, but validating additional CAdES attributes is up to the end user.
We needed it because we are signing files with external provider, which produces CAdES BES signatures.

@moritzgloeckl
Copy link

Very nice @netcitylife 👍

@gdelugre
Copy link
Owner

gdelugre commented Feb 2, 2019

Thanks for this PR, that would be a useful feature.

I would like to integrate it, however I will need to modify it because the current interface does not fully satisfy me. The problem is that the signature process is not atomic. I want the public API of the PDF object to let the document is a consistent state all the time. Here the method is split in two (prepare_signature / insert_signature), it is confusing and lets the object in an inconsistent state if just one method is called.

For the signature process, it needs to be done through a unique call to the sign method. If external code needs to be executed, it should be called through a callback or a block.

So I am proposing the following changes:

  • Keep the prepare_signature / insert_signature methods, but make them private.
  • Implement a callback mechanism that will call the right provider based on the signature filter used, allowing you to define your own providers.

In the case of adbe.*, the code would invoke the default Origami compute/verify methods.

Assuming you would need to had an extra signature scheme, you could do it like:

class CadesSignatureProvider
    def compute(method, data, certificate, key, ca)
        # your code here
    end

    def verify(method, data, signature, store, chain)
        # your code here
    end
end

Origami::PDF.add_signature_handler "ETSI.CAdES.detached", CadesSignatureProvider

# Then use PDF#sign/verify normally and your provider will be automatically invoked.
document.sign(cert, key, method: "ETSI.CAdES.detached")

This design would be compatible with the current API and would allow you to add your own custom handlers.

Would this work for you?

@ekzobrain
Copy link
Author

ekzobrain commented Feb 3, 2019

Hi, @gdelugre

prepare_signature / insert_signature methods are used in different user requests in our case, so we cannot use a callback or block. Our flow is:

  1. Generate document on server / upload document to server
  2. User makes a request to server to sign this document
  3. On server call read(), prepare_signature(), save() and send signable contents to users browser
  4. User signs recieved contents with his signature in the browser with CryptoAPI and sends computed signature back to server
  5. On server call read(), insert_signature(), save() - document is now signed.

I generally agree with you about atomicity, you just need to decide weather you want to officially support such use case or not. Of course in Ruby there is a way to overcome private method protection with send() method, so we could use it in this case instead of direct calls if methods just become private and their behavior would not change.

About signature providers - "ETSI.CAdES.detached" is not an extension, but an officially documented signature method, supported by Adode Acrobat 10+. The only difference is that it is much more complicated to compute/verify such signatures than adbe.*. Moving signature providers to separate registerable classes is a good idea, but I think, for consistency, it should be implemented the same way for all existing signature methods, not only for CAdES. So all available signature any method may be overriden if desired.

@ekzobrain
Copy link
Author

Hi, @gdelugre , did you decide something about this implementation?

@dvanderbeek
Copy link

@netcitylife I'm trying to use your code to insert a signature that I'm generating with a private key in Google Cloud KMS. My private key uses the following algorithm for asymmetric signatures: RSASSA-PSS 2048 bit key with a SHA-256 digest

Do you happen to know how to properly encode the signature that I'm getting back from KMS before inserting it into the PDF? Right now I'm getting an Invalid Signature error in Adobe with SigDict \Contents: illegal data. I also don't know if I'm sending the proper digest over to them. Right now I'm sending Digest::SHA256.digest(signable_content) based on your example file.

@moritzgloeckl
Copy link

Hey @dvanderbeek

First I'd suggest opening the PDF file in a hex viewer to check if the Signature Object is valid (especially ByteRange, and Contents).

In my fork I'm encoding the signature like this: OpenSSL::PKCS7.new(signature).to_der.

You might want to check out my repo: https://github.com/moritzgloeckl/origami

@dvanderbeek
Copy link

Thanks @moritzgloeckl. For some reason I still can't get it to work. It won't insert the signature that comes back from Google Cloud KMS. The error keeps alternating between Could not parse the PKCS7: header too long and Could not parse the PKCS7: not enough data without changing any of the code

@moritzgloeckl
Copy link

Hey @dvanderbeek unfortunately I haven't used Google Cloud KMS myself, so I can't really tell what's going on. But I'd guess that it wouldn't work much different than having the certificate on your computer locally and signing the PDF.

@dvanderbeek
Copy link

@moritzgloeckl Yeah I am sure there is just some sort of issue with the way they encode the signature. It seems like it's supposed to be base64 encoded, and your code looks like that's what it is expecting, but I am kind of learning on the fly and can't seem to figure out how to get it working. Thanks anyway for the responses.

@ekzobrain
Copy link
Author

Hi, @gdelugre.

I think I've found a way to leave both methods prepare_signature and insert_signature public and leave document in a correct state between these calls. First method may be modified a bit and serve a purpose of creating a signature placeholder, then a second method should insert signature to that placeholder. Creating a signature placeholer is almost the same as prepare_signature() currently does, except some flag values, as i think... The whole process is described here: https://github.com/vbuch/node-signpdf#append-a-signature-placeholder, may you look at it quickly to find the difference with current behavior?

@rngtng
Copy link

rngtng commented Jun 29, 2020

Hey @netcitylife thanks a lot for your PR, I was looking for that. What's the status of the PR? I unfortunately couldn't get it working, when I check the signed PDF in adobe acrobat, the signature tab is empty :(

UPDATE: fixed, content_size was to small

jtorrents added a commit to jtorrents/origami that referenced this pull request Oct 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants