Replace incoming certs with newer versions from trust store.#149
Replace incoming certs with newer versions from trust store.#149
Conversation
|
The logic here is derived from pieces of logic in OpenSSL's x509_vfy.c build_chain function, specifically this logic removing remaining untrusted certs and this logic replacing a wire cert with one from the trust store. |
|
Also relevant, the discussion about why this change was made in OpenSSL: http://openssl.6102.n7.nabble.com/openssl-dev-openssl-org-2634-Cross-signed-certs-rejected-by-OpenSSL-because-root-cert-not-base-of-chn-td56649.html |
kares
left a comment
kares
left a comment
There was a problem hiding this comment.
💚 for digging deep (into OpenSSL code)
| if (ok == X509Utils.X509_LU_X509) { | ||
| // replace old with new and clear rest of untrusted | ||
| Certificate certificate = (Certificate) objTmp[0]; | ||
| if (certificate.x509.equals(skCert)) { |
There was a problem hiding this comment.
I am sure I am missing bits and pieces. the question for me is what cert-chain do I need to send to get it trusted. so I need the skCert to match the subject-principal of a trusted certificate and then I need to satisfy this equals method.
from the supplied threads I figured that we basically want to replace self-signed certificates in the middle of the chain by some trusted certificate from local trust-store.
There was a problem hiding this comment.
@mkristian Yes, that's exactly it. I'm working on getting the right things to compare to allow an untrusted cert from the wire to properly match a trusted cert in the local store. Need to read more of the OpenSSL code to understand exactly how much needs to be the same.
There was a problem hiding this comment.
@headius I was pounding a bit more on this. maybe this is just OK as is when there new trusted certificate chain gets verified after this. after all that is what we want to achieve: a trusted verified chain from the server certificate to root certificate where root comes from our root certificate store.
|
@headius can I do anything to help move this along? => If you have any concrete steps in mind for resolving this situation, I'm happy to offer my time for that :) |
|
@original-brownbear this was an attempt to resolve a (system) CA cert issue. |
|
@kares I was able to reproduce the failure with Oracle Maybe we should try upping Travis to |
|
@original-brownbear thanks, still Java 8 is the primary target. recall trying latest at some point wout luck. |
enebo
left a comment
enebo
left a comment
There was a problem hiding this comment.
I think I never replied to this because I never understood how we knew we could trust the change in certificates. I don't have confidence I can say this is right or wrong. Is what we get by getBySubject guaranteed to be trusted?
|
Anyone checked with a more recent Java 8? If it got fixed in 9.0.4 it may have been backported too. |
|
Hi, this is still happening in alpine as of version: |
|
Is this PR useful for the current issues around the expired Let's Encrypt cert? #236 |
|
@ThomasKoppensteiner It may be but the reason it was never merged is because I was not confident that it was the correct (i.e. safe and secure) change to make. @kares Ping... this is the change I mentioned from back when I was trying to fix this revoked cert issue. Perhaps it helps? |
|
yeah, I recalled and revisited this. |
|
as seen with #236 this logic isn't enough (also a similar 'minimal' attempt at #240 wasn't sufficient) -> with jruby-openssl >= 0.11.0 this issue is expected to be resolved due porting of the alt-chain verification logic from OpenSSL 1.1.1 |
7 participants
Fixes jruby/jruby#4802
This tries to emulate what OpenSSL does in X509_vfy.c when building the untrusted certificate chain, replacing incoming certs with newer versions from the local store.