On 2 August 2017 at 17:01, Rob Godfrey <rob.j.godf...@gmail.com> wrote: > On 2 August 2017 at 17:43, Lorenz Quack <quack.lor...@gmail.com> wrote: > >> Hi all, >> >> tl;dr >> ===== >> I think overall if it would come to a vote right now I would vote like >> this: >> a) -1 >> b.1) -1 >> b.2) +0 >> c) +1 >> >> > I think I'd vote for implementing option b.2), or option a) but only for > "peers only" truststores (since if you have enabled peers-only you are > explicitly saying that your certificate chain will only ever have one > entry). > I'm not in favour of option c) since it seems like if you are doing > peers-only and you have checked the validity of the certificate when it was > added, then you would expect the trust store to enforce that validity. > > -- Rob >
I'd say a) for "peers-only", or b.2) though I share Lorenz dislike of having to maintain more non-trivial code around this one. Perhaps c) with "use [self-signed] CA's" guidance would also work for me. > >> reasoning follows inline: >> >> On Wed, 2017-08-02 at 15:13 +0100, Keith W wrote: >> > If we were to add a feature to help the use-case, we'd need to decide >> > on the scope. >> > >> > The alternatives I see: >> > >> > (a) validate the expiration of self-signed certificates used for >> > authentication purposes only >> > >> >> I don't like this. It creates a false sense of security. >> My understanding is that this option means that we would validate >> incoming self-signed peer certificates explicitly. >> However, as a malicious attacker I think this would be easily >> circumvented. >> Imagine the following scenario: Eve has a self-signed certificate "A" >> which is in the broker's truststore. Unfortunately, "A" has expired >> and the broker (thanks to this new feature) refuses the connection. >> Eve now creates a new valid certificate "B" signed by "A". When the >> broker receives "B" it will check that it is valid (which it is) and >> that its certificate chain is trusted. It will immediately find the >> trust anchor (the expired certificate "A") in its trust store and >> accept the connection. >> So I am -1 on this. >> >> > (b) broaden the feature. Disallow all expired trust anchors.This >> > which would include (a) but would also catch cases where, say a, a >> > private CA is used carelessly: the root-CA certificate is allowed to >> > expire before certificates issued from it. >> > >> >> I am okay with this. It is not required by RFC5280 but it is >> permitted. It would provide some additional security for people with >> poorly maintained truststores (e.g., old JVM). >> I am +0 on this. >> >> > (c) do nothing more. QPID-7869 has already added altering for out of >> > date certificates within the trust stores. perhaps this is enough. >> > >> >> /s/altering/alerting/ >> IMHO, this is acceptable. >> I am +0 on this. >> >> > I think any new feature would be optional and turned off by default. >> > This is for the sake of backward compatibility. >> > >> > Turning to implementation: >> > >> > For (a) we would simply change the >> > X509TrustManagers#checkClientTrusted to call >> > X509Certificate#checkValidity() on all certificates in the peer's >> > chain. The CertificateExpiredException would halt the connection. >> > >> > For (b), I see two implementation options: >> > >> > 1) Exclude expired certificates from the truststore. Currently the >> > TrustManagers are generated once and reused by the Ports.This would >> > need to change: either TrustManagers could be generated on-the-fly for >> > each connection (not sure about the cost), or the TrustManagers could >> > be periodically refreshed (there would be a race, of course). >> > >> >> I don't like the race condition. I think if we say we don't allow >> expired certificates this should be exact. I also don't like that >> this would result in "certificate_unknown" exceptions but as it turns >> out we don't seem to have an alternative to this execption :( >> >> > 2) Have X509TrustManagers#checkClientTrusted check the TrustAnchor's >> > validity. Unfortunately, the JCA doesn't expose the full >> > certification path to the application until after the TLS handshake is >> > completed. To have X509TrustManagers#checkClientTrusted check the >> > TrustAnchor's validity, it is necessary to recompute the certification >> > path. I have checked this approach is feasible (proof of concept >> > patch attached to QPID-7868). >> > >> >> Sounds like it could work (I have not looked at the PoC which is on >> QPID-7869 and not QPID-7868). >> The down side of this would be more (non-trivial) code which can contain >> security relevant bugs and needs to be understood and maintained. >> >> Kind regards, >> Lorenz >> >> > I want to mention what the client would see,. With the CPP Broker/NSS >> > a TLS alert "certificate_expired" goes over the wire. Unfortunately, >> > Oracle's coding of the JCA means that if a >> > X509TrustManager#checkClientTrusted throws an CertificateException or >> > sub-class, the TLS alert that goes over the wire is always >> > "certificate_unknown" regardless of underlying cause. The code in >> > question is sun.security.ssl.ServerHandshaker#clientCertificate(). >> > The class is final and the whole piece seems to be unpluggable. I >> > don't see a reasonable way around this. >> > >> > I am in favour of feature (b) as this feels more generally useful. I >> > prefer implementation option 2). >> > >> > Thoughts? Alternatives? >> > Keith. >> > >> > >> > On 2 August 2017 at 11:50, Keith W <keith.w...@gmail.com> wrote: >> > > >> > > Hello >> > > >> > > Martin Krasa raised JIRA QPID-7867 [1] on 21st July. As the JIRA >> > > possibly eluded to a potential security issue, the initial discussion >> > > was held in private on the Qpid private / Apache security lists. We >> > > have now reached a point where there is a agreement that there is no >> > > security issue as such. We might however added a feature to the Java >> > > Broker to benefit this use-case. >> > > >> > > I don't wish to cross-post the private conversation verbatim. Instead >> > > I will try and summerise the issue and the background as I understand >> > > it. >> > > >> > > use-case/issue: >> > > >> > > The use-case is a development one, It involves a mutually >> > > authenticated TLS connection for messaging from the Qpid JMS Client to >> > > the Java Broker. Both client and server were using self signed >> > > certificates and those certificates had expired The complaint was >> > > that connection succeeded even though the certificates had expired. >> > > The user was comparing behaviour with the CPP Broker which disallowed >> > > such connections. >> > > >> > > background: >> > > >> > > Both the Java Broker and Qpid JMS Client rely on the inbuilt features >> > > of Java JCA for the purpose of establishing trust during a TLS >> > > handshake. Java treats certificates in the truststore store as >> > > trust anchors. During an TLS handshake, the Java builds a certificate >> > > chain from the certificate(s) provided by the peer over the wire to a >> > > trust anchor in the store. If the chain can be formed, the >> > > certificates that comprise the chain are subject to checks (such as an >> > > expiration), but these checks are *not* applied to the trust anchor >> > > itself. This behaviour seems to me to consistent with RFC5280 [4]. >> > > >> > > In use-cases involving self-signed certificates, the certificate is >> > > installed in the peer's truststore. and as such the self signed >> > > certificate is itself the trust anchor. When the remote side >> > > connects and presents the self signed cert, this matches the trust >> > > anchor stored and connection forms without expiration exception. >> > > >> > > I hope this is reasonable summary. Please feel free to correct or >> clarify. >> > > I plan to post again later today with my ideas for changes we could >> > > make to the Java Broker to help this use-case. >> > > >> > > Keith,. >> > > >> > > [1] https://issues.apache.org/jira/browse/QPID-7867 >> > > [2] https://docs.oracle.com/javase/8/docs/technotes/ >> guides/security/crypto/CryptoSpec.html >> > > [3] https://docs.oracle.com/javase/8/docs/api/java/ >> security/cert/package-summary.html >> > > [4] http://www.ietf.org/rfc/rfc5280.txt >> > --------------------------------------------------------------------- >> > To unsubscribe, e-mail: users-unsubscr...@qpid.apache.org >> > For additional commands, e-mail: users-h...@qpid.apache.org >> > >> >> --------------------------------------------------------------------- >> To unsubscribe, e-mail: users-unsubscr...@qpid.apache.org >> For additional commands, e-mail: users-h...@qpid.apache.org >> >> --------------------------------------------------------------------- To unsubscribe, e-mail: users-unsubscr...@qpid.apache.org For additional commands, e-mail: users-h...@qpid.apache.org
