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

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

Reply via email to