Thanks all for the input.  The most preferred option was
implementation an additional trust store feature that would, if
enabled, disallow the use of expired trust anchors ('b') using the
approach of re-computing the certification path ('2') .
I intend to implement this under QPID-7867 over the next day or two.

cheers, Keith.


On 3 August 2017 at 14:29, Robbie Gemmell <robbie.gemm...@gmail.com> wrote:
> 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

Reply via email to