Hi Martin,
The TLS 1.3 spec is replacing the Trusted CA Indication
(trusted_ca_keys) extension with a new Certificate Authorities
(certificate_authorities) extension. See more details about the
specification in the TLS 1.3 draft:
https://tools.ietf.org/html/draft-ietf-tls-tls13-20#section-4.2.4
Both serves a similar purpose, but the trusted_ca_keys extension will
not be used in TLS 1.3 any more. The "trusted_ca_keys" extension will
only be used for legacy protocol versions (TLS 1.2/1.1/1.0).
There are two options to me:
1. Supports the certificate_authorities, but not trusted_ca_keys extension.
It is acceptable to me as trusted_ca_keys is for legacy use only and the
certificate_authorities extension is the future. Plus, the
certificate_authorities extension can also be used for TLS 1.2 and
previous versions.
2. Supports both the certificate_authorities and trusted_ca_keys extensions.
As far as I know, I did not see much benefit of this option unless the
trusted_ca_keys extension is widely used in practice.
If I did not miss something, the APIs you designed can still be used for
the certificate_authorities extension, with a little bit update.
What do you think?
Thanks & Regards,
Xuelei
On 6/15/2017 12:05 PM, Martin Balao wrote:
Hi Xuelei,
The new webrev.02 is ready:
*
http://people.redhat.com/mbalaoal/webrevs/jdk_8046295_trusted_ca/2017_06_15/8046295.webrev.02/
(browse online)
*
http://people.redhat.com/mbalaoal/webrevs/jdk_8046295_trusted_ca/2017_06_15/8046295.webrev.02.zip
(zip, download)
The following changes have been implemented since the previous webrev.01:
* s/getUseTrustedCAIndication() methods in SSLEngine/SSLSocket and in
SSLEngineImpl/SSLSocketImpl removed. s/getSSLParameters is now the only
way to set or get the use of the Trusted CA Indication extension. An
exception is no longer thrown if trying to disable the extension for a
server, but the change takes no effect as the extension is mandatory for
servers. X509KeyManagerImpl modified to use SSLParameters to get
information regarding if Trusted CA Indication is enabled and should
guide the certificate choice.
* TrustedAuthorityIndicator.IdentifierType has been moved from enum to
String, to follow JSSE conventions. I understand how important is to be
consistent. However, I still believe that an enum is a better fit for
this value and does not prevent from future extension. We are choosing
from a closed set (strictly defined by the RFC) and that's what enum
allows to express. From the client point of view/API, it's very handy
that the type gives you information regarding the allowed choices for
the parameter. You don't necessarily have to look for implementation
details or documentation but you can just leverage on the strongly typed
language. It's also likely that enums are faster for comparisons than
strings, but that's not the main point here.
* Removed X509Certificate from TrustedAuthorityIndicator class (method
and property). It was there for informational purposes (when
TrustedAuthorityIndicator was built from a certificate by a client and
the whole extension indicators converted to String).
* "equals" and "hashCode" methods moved from TrustedAuthorityIndicator
to TrustedAuthorityIndicatorImpl class.
* "getLength" method removed from TrustedAuthorityIndicator class.
It's possible to get the encoded buffer and the length from there.
* "getData" method renamed to "getEncoded" in
TrustedAuthorityIndicator class.
* "trustedAuthorityEncodedData" renamed to "encodedData" in
TrustedAuthorityIndicator and TrustedAuthorityIndicatorImpl classes
* "identifier" and "encodedData" instance variables moved from
TrustedAuthorityIndicator to TrustedAuthorityIndicatorImpl class.
* "getEncoded" and "getIdentifier" are now abstract methods in
TrustedAuthorityIndicator, and their implementation is in
TrustedAuthorityIndicatorImpl class.
* "getIdentifier" method renamed to "getType" in
TrustedAuthorityIndicator and TrustedAuthorityIndicatorImpl classes
("identifier" instance variable and parameter in
TrustedAuthorityIndicatorImpl class renamed to "type").
* Test cases (server and client) updated to reflect the new interface
(enabling the use of the extension through SSLParameters)
However, some changes are still not implemented and I have some concerns:
1) I still believe that identifier type information has to be on
TrustedAuthorityIndicator class somehow, and implementations restricted
on what they can return as part of "getType" method. This is strictly
specified by the RFC TrustedAuthorityIndicator class represents, and I
find desirable that any implementation is enforced to be compliant to
that. If we remove all of that (including the enum),
TrustedAuthorityIndicator looks too generic and does not reflect (in my
opinion) what it really is. It'd also be chaotic if different
implementations call pre-agreed type as "preagreed", "pre-agreed",
"PRE_AGREED", etc. I prefer stricter and more explicit interfaces.
2) I agree that type mappings can be seen as part of an implementation,
but they were in TrustedAuthorityIndicator (as protected) because every
implementation is highly likely to need them and we can avoid the
necessity for repeated code/mappings. The same for "type" and
"encodedData" variables or even "hashCode" and "equals" methods. That's
why I was thinking more of an abstract class and not an interface, as it
happens (in example) with SNIServerName.
3) I think that "implies" method on TrustedAuthorityIndicator should be
also part of the class/interface, because that's the whole point of a
Trusted Authority Information: to allow queries for a given certificate.
This is, in fact, the only thing a server wants from one of these
objects. My concern is that if we remove this requirement for an
implementation, the interface looks more like a byte buffer holder.
I'd appreciate if you can re-consider these items.
Thanks,
Martin.-
On Wed, Jun 14, 2017 at 7:17 PM, Xuelei Fan <xuelei....@oracle.com
<mailto:xuelei....@oracle.com>> wrote:
Hi Martin,
The big picture of the design looks pretty good to me, except a few
comment about the JSSE conventions. I appreciate it very much. By
the way, I need more time to look into the details of the
specification and implementation.
In order to keep the APIs simple and small, SSLParameters is
preferred as the only configuration port for common cases. I may
suggest to remove the s/getUseTrustedCAIndication() methods in
SSLEngine/SSLSocket.
The identify type is defined as an enum
TrustedAuthorityIndicator.IdentifierType. In the future, if more
type is added, we need to update the specification by adding a new
enum item. Enum is preferred in JDK, but for good extensibility, in
general JSSE does not use enum in public APIs for extensible
properties. I may suggest to use String (or integer/byte, I prefer
to use String) as the type. The standard trusted authority
indicator algorithm (identifier) can be documented in the "Java
Cryptography Architecture Standard Algorithm Name Documentation"[1].
In TrustedAuthorityIndicator class, some methods, like
getIdentifierTypeFromCode(), getCodeFromIdentifierType(), implies(),
getLength(), equals() and hashCode() look more like implementation
logic. I may suggest remove them from public APIs.
I did not see the benefit to have X509Certificate in the
TrustedAuthorityIndicator class. The class is mainly used for
server side certificate selection. X509Certificate could be unknown
for a indicator. I may suggestion remove the related methods and
properties.
After that, as there is no requirement to instantiate
TrustedAuthorityIndicator class in application code, looks like it
may be enough to use an interface to represent a trusted authorities:
public interface TrustedAuthorityIndicator {
// identifier type, standard algorithm name
String/int/Byte getType();
// identifier
byte[] getEncoded();
}
What do you think?
Thanks & Regards,
Xuelei
[1]
https://docs.oracle.com/javase/8/docs/technotes/guides/security/StandardNames.html
<https://docs.oracle.com/javase/8/docs/technotes/guides/security/StandardNames.html>
On 6/13/2017 3:41 PM, Martin Balao wrote:
Hi Xuelei,
The new webrev.01 is ready:
*
http://people.redhat.com/mbalaoal/webrevs/jdk_8046295_trusted_ca/2017_06_13/8046295.webrev.01/
<http://people.redhat.com/mbalaoal/webrevs/jdk_8046295_trusted_ca/2017_06_13/8046295.webrev.01/>
(browse online)
*
http://people.redhat.com/mbalaoal/webrevs/jdk_8046295_trusted_ca/2017_06_13/8046295.webrev.01.zip
<http://people.redhat.com/mbalaoal/webrevs/jdk_8046295_trusted_ca/2017_06_13/8046295.webrev.01.zip>
(zip, download)
The following changes have been implemented since the previous
webrev.00:
* Pre-agreed support removed from server-side
* Unnecessary overhead and minium benefits for JSSE.
* Enabling the use of Trusted CA Indication extension for
clients through TrustManager objects was reverted. Trusted CA
Indication extension can now be enabled through: 1) SSLEngine,
2) SSLSocket, or 3) SSLParameters (which can be applied to both
SSLEngine and SSLSocket objects). Trusted CA Indication
extension is mandatory for servers.
* SunX509KeyManagerImpl old key manager ("SunX509" algorithm)
is now out of scope. This key manager does not support other TLS
extensions as Server Name Indication (SNI), which is far more
relevant than Trusted CA Indication. The new X509KeyManagerImpl
key manager ("PKIX" algorithm) is now in scope.
* Client requested indications are now an ExtendedSSLSession
attribute. ServerHandshaker gets the information from the Client
Hello message (now parsed by TrustedCAIndicationExtension class
instead of TrustedAuthorityIndicator) and sets it in the
ExtendedSSLSession (SSLSessionImpl object). The key manager
(i.e.: X509KeyManagerImpl), when choosing a server alias, may
now get the information from the ExtendedSSLSession object and
guide the certificate selection based on it.
* In order to allow multiple key managers to use Trusted
Authority Indicators information and to allow multiple Trusted
Authority Indicators implementations, TrustedAuthorityIndicator
has now been split in an abstract class
(TrustedAuthorityIndicator, located in javax.net.ssl) and an
implementation class (TrustedAuthorityIndicatorImpl, located in
sun.security.ssl). No coupling was added between javax.net.ssl
and sun.security.ssl packages.
* Documentation extended and improved.
* Test cases (server and client) updated to reflect the new
interface and supported key manager.
Look forward to your new review!
Kind regards,
Martin.-
On Fri, Jun 9, 2017 at 6:15 PM, Xuelei Fan
<xuelei....@oracle.com <mailto:xuelei....@oracle.com>
<mailto:xuelei....@oracle.com <mailto:xuelei....@oracle.com>>>
wrote:
I'm OK to use SSLParameters. Thank you very much for
considering a
new design.
Xuelei
On 6/9/2017 1:10 PM, Martin Balao wrote:
Hi Xuelei,
I didn't notice that some of the SSLSocket contructors
did not
establish the connection, so SSLParameters can be
effective for
Trusted CA Indication. This was an invalid argument on
my side,
sorry.
As for the configuration to enable the extension, it's
probably
not necessary on the Server side because -as you
mentioned- it
is mandatory and there is no harm in supporting it.
However, it
has to be configurable on the Client side because -as we
previously discussed- the client may cause a handshake
failure
if the server does not support the extension. I'd
prefer the
Client configuring the SSLSocket through SSLParameters
instead
of a system-wide property -which has even more impact
than the
TrustManager approach-. Would this work for you?
> In JSSE, the benefits pre_agreed option can get by
customizing the key/trust manager, so I did not see too
much
benefits to support this option in JDK
I understand your point and will remove support for
"pre_agreed".
On Fri, Jun 9, 2017 at 1:37 AM, Xuelei Fan
<xuelei....@oracle.com <mailto:xuelei....@oracle.com>
<mailto:xuelei....@oracle.com <mailto:xuelei....@oracle.com>>
<mailto:xuelei....@oracle.com
<mailto:xuelei....@oracle.com> <mailto:xuelei....@oracle.com
<mailto:xuelei....@oracle.com>>>>
wrote:
On 6/8/2017 8:36 PM, Xuelei Fan wrote:
The trusted authorities can be get from client
trust
manager. Server can choose the best matching of
server
certificate of the
client requested trusted authorities.
>
I missed the point that the key manager need to
know the client
requested trusted authorities for the choosing.
So may
need a new
SSLSession attribute (See similar method in
ExtendedSSLSession).
Xuelei
Yes, an attribute on SSLSession may do the job (both
when Key
Manager receives a SSLSocket and a SSLEngine).
Kind regards,
Martin.-
