Fix various Problems and add some features#2
Merged
Conversation
1. remove Cargo.lock from git repository, it was already in gitignore but still checked in. 2. fix all generated cerficiates were always ca certificates regardless of input. 3. fix all generated certificates always had the wrong signature algorithm nid used. They used the NID of the encryption algorithm not the signature algorithm. Most applications such as apache2 or rust-tls refused to load such certificates because they couldn't make sense of the signature. added some convinience features: 1. add easy way to generate a self signed certificate with a custom validity duration that is not 1 year from now on. 2. add easy way to output the private key from the KeyPair to a pcks8 pem file.
Owner
|
Thanks! I'll try to review soon |
Owner
|
Sorry, I was on vacation last week. I just merged and released on crates.io. Thanks for the fixes! |
Contributor
Author
|
@nacardin Thank you! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Good day to you,
this pr fixes #1
fix various problems:
=> I didn't check if the file is also present on crates.io release, but this file prevented using the crate as a git or path dependency during development with the latest rust compiler. It should be removed.
=> as discussed in the issue
=> This was a pain to find. You had a test, but the test did test it with the wrong NID. You checked for X9_62_ID_ECPUBLICKEY which is the NID for the public key itself, but the NID should have been ECDSA_WITH_SHA256 which is the actual signature algorithm. I only found this after side by side comparing a dearmored dump of the certificate with openssl
with one generated by openssl itself. I have no clue what application even accepted the certificates that were generated by 0.1.0 but neither rust-tls nor apache2 would accept certificates with a bogus signature algorithm.
Perhaps applications that do not check at all if the certificate signature is valid?
Since I do not know a web server that accepts such certificates I couldn't check with a browser,
but I doubt a browser would have accepted it.
added some convenience features:
=> Some of my systems have time desync with each other (yes I know skill issue on my end) but this caused the certificate to only become valid in 5 minutes because the client was 5 minutes behind in time, which caused ssl errors on the clients end. Providing a easy method that allows me to specify the validity allows me to just pass "yesterday" as start date.
=> Getting the private key either as DER or PEM from the KeyPair struct was a pain without this added function. You needed to add several dependencies to your crate with matching versions as well as do rather complex match statement with a lot of unreachable arms (depending on your choice of key algo). Adding this to certkit directly allows for very easy export of the private key.
Note:
I was unable to run the "botan" tests because botan(build-sys) did not compile on my computer.
So the "botan" tests may be failing. I have no idea what "botan" is.
I would appreciate it if this could be merged and if you could push a release after this.
I took the liberty of increasing the version for you already.
Sincerely
Alexander Schütz