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

OopenSSL 1.1.0 compatibility #107

Merged
merged 9 commits into from Nov 27, 2017
Merged

OopenSSL 1.1.0 compatibility #107

merged 9 commits into from Nov 27, 2017

Conversation

Jakuje
Copy link
Contributor

@Jakuje Jakuje commented Feb 20, 2017

Update 2017-11-14: Resolves #104. Tested with latest Yubikey 4 with hardware tests, provisioning new key and selfsigning certificates.

/* XXX: this should probably use X509_REQ_digest() but that's buggy */
if(!ASN1_item_digest(ASN1_ITEM_rptr(X509_REQ_INFO), md, req->req_info,
digest + oid_len, &digest_len)) {
if (!X509_REQ_digest(req, md, digest + oid_len, &digest_len)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you tested if this works? The problem with X509_REQ_digest() is (used to be?) that it hashes the entire X509_REQ, ideally I guess there would be a X509_REQ_INFO_digest().

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't test this part, but I am not sure if there is an interface in the new OpenSSL to get the req_info from the structure. I will remove this chunk for now.
I planned to do all the changes, but some of the certificate handling was more complex than I was familiar with. Currently dumping what is available for others to have possibility to continue or I will finish that later.

@klali
Copy link
Member

klali commented Feb 20, 2017

The approach of doing this with a compat layer is probably the only way to go ahead. I haven't looked it over to see how many of those would need to be added.
There is no direct plan or schedule for this work.

@Jakuje
Copy link
Contributor Author

Jakuje commented Feb 20, 2017

Thank you for prompt comment. There are really only few more changes, but they are probably harder to address than this part in this commit.
If there is no plan yet from Yubico, I will be hopefully able to continue on that (or somebody else), since many distributions are moving to openssl 1.1.0

@Jakuje
Copy link
Contributor Author

Jakuje commented Feb 23, 2017

I added few more changes I was able to handle. The embedded tests work except the hardware ones (failing even before the change).

What is missing is working with {X509,X509_REQ}_digest() and tweaking of {x509,req}->signature->flags and {req,x509}->{cert_info,sig_alg}->{parameter,algorithm} on several places in the code.

I believe that is a good start, but I don't have enough internal knowledge to test further changes to make sure they will not break some use cases. I will build the package against old OpenSSL so far, but having this fixed in future would be very nice.

@Jakuje
Copy link
Contributor Author

Jakuje commented Oct 20, 2017

I was tweaking with it little bit more added the straight-forward parts, but the certificates are still little bit more complicated.

From what I see, with opaque structures, it is not possible to do exactly what you were doing in functions request_certificate() and selfsign_certificate() and do_create_empty_cert(), such as modifying algorithms, writing signatures directly into X509_REQ and X509 structures and so on.

On the other hand, the X509_REQ_sign() and X509_sign() should get used to achieve the same using high-level interface, while the signature should be provided by using RSA_method and EC_KEY_METHOD structures to pass the signature request to the card itself.

I am pushing a WIP status, that builds with OpenSSL 1.1.0, but for older is still using the same approach. Unfortunately, currently I do not have any capable yubikey at hand to test the actual functionality, so the last commit is for further review/testing, but the idea should be clear.

@Jakuje
Copy link
Contributor Author

Jakuje commented Nov 14, 2017

Finally my new Yubikey arrived so before setting it up for myself I finished with this PR and made even the HW tests pass with OpenSSL 1.1.0 and hopefully also with the old version (including the rebase to the current master).

@Jakuje Jakuje changed the title Initial idea of openssl-1.1.0 compatibility OopenSSL 1.1.0 compatibility Nov 14, 2017
@mrmekon
Copy link
Contributor

mrmekon commented Nov 27, 2017

@Jakuje I updated this to work with the recent refactoring: PR #131

@mrmekon mrmekon merged commit 77c51a7 into Yubico:master Nov 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants