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

NOVERIFY flag #10

Closed
dirkx opened this issue Jul 31, 2021 · 17 comments
Closed

NOVERIFY flag #10

dirkx opened this issue Jul 31, 2021 · 17 comments

Comments

@dirkx
Copy link
Contributor

dirkx commented Jul 31, 2021

Note that by simplifying the chain validation code - we are now no longer checken the chain properly. It is no longer anchored.

Unfortunately solving this in pyopenss is not easy -- see pyca/pyopenssl#1031

@panzi
Copy link
Owner

panzi commented Jul 31, 2021

we are now no longer checken the chain properly

Are you talking of my version of verify_detached_signature()? This flag is also used in the library you originally used, see:
https://github.com/jnewbigin/pkcs7_detached/blob/1681424f903bdf22aa11d7430a605dc65f758f8b/pkcs7_detached/__init__.py#L86

I just implemented it myself so:

  • I don't need to decode/encode/decode everything for no reason.
  • It handles binary data correctly. (Gladly there are no non-ASCII characters in the JSON we had to verify, so there was no problem, but still. That seemed a bit iffy in that lib.)
  • I added missing ffi error handling!
  • I don't need yet another library this way.

@dirkx
Copy link
Contributor Author

dirkx commented Jul 31, 2021 via email

@panzi
Copy link
Owner

panzi commented Jul 31, 2021

What purpose flag would be the right one?

@panzi
Copy link
Owner

panzi commented Jul 31, 2021

@dirkx I've made a pull request to pyca/cryptography that adds the functions and related constants you've mentioned, but since I don't know about this cryptography stuff I don't know how to write proper tests for it and as such the coverage tests fail. Would you know how to write proper tests that include the PKCS7_get0_signers() and X509_STORE_set_purpose() functions? See also: pyca/cryptography#6187

@dirkx
Copy link
Contributor Author

dirkx commented Jul 31, 2021 via email

@dirkx
Copy link
Contributor Author

dirkx commented Jul 31, 2021 via email

@panzi
Copy link
Owner

panzi commented Aug 1, 2021

I'm currently in the process of trying to implement this verification by hand (using asn1crypto to parse the ASN.1 data) and in doing so found out that it was the wrong root certificate anyway! The correct one is: http://cert.pkioverheid.nl/EVRootCA.cer

@panzi
Copy link
Owner

panzi commented Aug 1, 2021

I managed to verify the trust chain manually... but currently failing at the actual signature.

@dirkx
Copy link
Contributor Author

dirkx commented Aug 1, 2021 via email

@dirkx
Copy link
Contributor Author

dirkx commented Aug 1, 2021 via email

@panzi
Copy link
Owner

panzi commented Aug 1, 2021

This is what I currently have:

The trust chain verification works, but then the verification of the payload fails somehow. Adding debug prints to Python cryptography library just tells me the OpenSSL error: error:04091068:rsa routines:int_rsa_verify:bad signature

@panzi
Copy link
Owner

panzi commented Aug 1, 2021

There are a lot of debug prints in there right now.

@panzi
Copy link
Owner

panzi commented Aug 1, 2021

Also verified: The digest I calculate is the message digest inside the signed data. Really only the last verification step fails. Really not sure how else I could call the public_key.verify() function.

@dirkx
Copy link
Contributor Author

dirkx commented Aug 1, 2021 via email

@dirkx
Copy link
Contributor Author

dirkx commented Aug 1, 2021 via email

@panzi
Copy link
Owner

panzi commented Aug 1, 2021

Thank you for your help! With this information and a little bit of trying around I finally managed it! See:

def verify_pkcs7_detached_signature(payload: bytes, signature: bytes, root_cert: x509.Certificate) -> bool:

So I'm closing this issue now. 😄

@panzi panzi closed this as completed Aug 1, 2021
@dirkx
Copy link
Contributor Author

dirkx commented Aug 1, 2021 via email

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

No branches or pull requests

2 participants