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

Verify method implemented #59

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

aravindarc
Copy link

@aravindarc aravindarc commented Jul 5, 2019

A verification method has been implemented, which is in the TODO list.

After I couldn't find the cause of the exception:
Exception in thread "main" java.lang.NullPointerException
at org.bouncycastle.cms.CMSSignedData$1.write(Unknown Source)

pointed by issue #42, implemented this verify method.

This is a shallow verify, if the exe file contains a chain of certificates, only the leaf certificates and their counter-signatures are verified.

Check this to understand the implementation.

@coveralls
Copy link

Coverage Status

Coverage decreased (-6.6%) to 81.727% when pulling 85cf0eb on aravindarc:feature/verification into d8729c6 on ebourg:master.

@ebourg
Copy link
Owner

ebourg commented Nov 29, 2019

Thank you for the PR. I understand that this compares the digest of the file and the one in the signatures, but does it check the validity of the certificate chain?

@aravindarc
Copy link
Author

aravindarc commented Dec 2, 2019

@ebourg No it does not but if you could tell me if this is the way to approach verification, I'll try and add checking the validity of the certificate chain

@ebourg
Copy link
Owner

ebourg commented Dec 16, 2019

@aravindarc The code has been refactored to support several file formats. Do you think you could rebase your PR on top of the latest changes? The verify() method could go in a new AuthenticodeVerifier class and take a Signable object as input.

@aravindarc
Copy link
Author

@ebourg Yes I'll try to change as you say and also validate the certificate chain.

@mduft
Copy link

mduft commented Dec 2, 2020

Hey. I copied the proposed code and use it to verify that our binaries are properly signed before releasing them. This works fine, the only thing I changed is to add a check for whether the used certificate is self-signed. I just copied the isSelfSigned() method from AuthenticodeSigner and verify the X509Certificate using this...

@ebourg ebourg force-pushed the master branch 3 times, most recently from c6795d2 to 51bb816 Compare June 30, 2021 10:17
@strager
Copy link

strager commented Mar 12, 2022

Is the only task for this PR resolving merge conflicts?

If this feature is added to Jsign, I can switch from osslsigncode to Jsign. Then, I can implement APPX signing (#81).

Context: I use osslsigncode for signing and verification of .exe files. I want to also sign APPX files, but this is not implemented in osslsigncode (or in Jsign). I'd rather implement APPX signing in Jsign than in osslsigncode, but that would only make sense if I use Jsign for signing and verifying .exe files. (I don't want to use both Jsign and osslsigncode.)

@strager
Copy link

strager commented Mar 12, 2022

If this feature is added to Jsign, I can switch from osslsigncode to Jsign. Then, I can implement APPX signing (#81).

I discovered Relic which has all the features I need. Its interface isn't as nice as Jsign's for my use case, but I'll manage.

I no longer plan to work on #81.

@ebourg
Copy link
Owner

ebourg commented Mar 12, 2022

Is the only task for this PR resolving merge conflicts?

No I don't think I'll merge the PR, I'm working on another design that provides a detailed verification report.

@kushalagrawal
Copy link

kushalagrawal commented May 4, 2022

Hi,
I am looking for this windows executable implementation including the MSI file and I have been waiting for this feature to be available. I have tested this on a few executable (Exe and MSI) files. It's most of the part is working for the MSI files as well. other than the part:

if(!Arrays.equals(computeDigest(signedDataAlgorithm), digestInfo.getDigest()))
                    throw new VerificationException("The embedded hash in the SignedData is not equal to the computed hash");

Is there anything that could be done to validate the MSI files as well with this?

@GitHubb3R
Copy link

Is the only task for this PR resolving merge conflicts?

No I don't think I'll merge the PR, I'm working on another design that provides a detailed verification report.

I'd like to ask about the status, progress of said that design, and whether its completely stalled.

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.

7 participants