Re: RFR: 8087112: HTTP API and HTTP/1.1 implementation

2015-09-29 Thread Michael McMahon

Anthony,

Thanks for the feedback. I am currently looking at API issues
but will incorporate these changes when I get back to the implementation.

- Michael

On 27/09/15 09:14, Anthony Vanelverdinghe wrote:

Hi Michael

I forgot to mention that, in Utils.java [1], the exception should also 
be thrown in case token is the empty string. And while I'm nitpicking 
anyway: here 's an alternative implementation which extracts the 
comparison logic in a separate method:


static void validateToken(String token, String errormsg) {
if (token.isEmpty() || token.chars().anyMatch(c -> !isTchar(c))) {
throw new IllegalArgumentException(errormsg);
}
}

private static boolean isTchar(int c) {
return c >= 0x30 && c <= 0x39 // 0 - 9
|| (c >= 0x61 && c <= 0x7a) // a - z
|| (c >= 0x41 && c <= 0x5a) // A - Z
|| (c >= 0x21 && c <= 0x2e && c != 0x22 && c != 0x28 && c 
!= 0x29 && c != 0x2c)

|| (c >= 0x5e && c <= 0x60)
|| (c == 0x7c) || (c == 0x7e);
}

PS: I have signed the OCA [2], so you can reuse any code if you wish

[1] 
http://cr.openjdk.java.net/~michaelm/8087112/01/src/java.base/share/classes/sun/net/httpclient/Utils.java.html

[2] http://www.oracle.com/technetwork/community/oca-486395.html#v

Kind regards,
Anthony


On 25/09/2015 21:59, Anthony Vanelverdinghe wrote:

Hi Michael

Maybe these have been fixed in the meantime, but I think I've found a 
bug:
in 
http://cr.openjdk.java.net/~michaelm/8087112/01/src/java.base/share/classes/sun/net/httpclient/Utils.java.html

single quotes must be accepted, while parentheses must be rejected [1]

and a few typos:
in 
http://cr.openjdk.java.net/~michaelm/8087112/01/src/java.base/share/classes/sun/net/httpclient/SSLTunnelConnection.java.html

at line 56, replace
return connected & delegate.connected();
with
return connected && delegate.connected();

in 
http://cr.openjdk.java.net/~michaelm/8087112/01/src/java.base/share/classes/java/net/package-info.java.udiff.html

replace "superceded" with "superseded"

On a final note, when will the new API be integrated in the weekly 
early access builds [2]?


[1] https://tools.ietf.org/html/rfc7230#section-3.2.6
[2] https://jdk9.java.net/download/

Kind regards
Anthony



Re: TLS ALPN Proposal v5

2015-09-29 Thread David M. Lloyd

Hi Brad, thanks for replying; comments are inline:

On 09/28/2015 08:40 PM, Bradford Wetmore wrote:


Several comments about David's proposal:


1. Only the initial ClientHellos are parsable.
===
The biggest problem I have with an Explorer-based design is that only
the initial ClientHello on a connection is passed in the clear.
Subsequent negotiations on this connection will be completely missed, as
the ClientHellos are now encrypted.

This seems like a deal breaker for me.


You are right, I cannot come up with a good solution for this, so that 
might mean the idea is shot - *however* - I would point out that the 
latest draft of TLS 1.3 [1] completely kills off the capability of the 
client to renegotiate a connection, meaning that this will no longer be 
possible anyway, and given it's a 1% kind of use case, that might be 
enough to let it slide.  Combine this with what I consider to be the 
unlikelihood of this working with HTTP/2.0, and I would feel very safe 
assuming that nobody will ever actually do this.


I would also note that, as you state later on, it would be possible to 
combine this solution with any other solution (including the proposed 
one) to cover both cases.  And given that this is still (in my 
estimation) a "99%" solution, in my opinion it is still a viable 
candidate for adding this functionality to Java 8 as a first pass or 
stopgap as I described in my emails, particularly if the method(s) to 
establish/query the protocol names are a strict subset of the proposed 
Java 9 API (given that we cannot really overhaul the Java SE 8 API at 
this point).



[...]
2.  "SSLExplorer" or something similar is needed.
=
This approach depends on "examining SSLClientHello"s, but there isn't a
class for this other than some sample code from a previous attempt.  I
am assuming that this approach would make such an external API a
necessity?  Being able to parse possible ClientHello formats is not a
straightforward/easy job.  This will add a fair amount of complexity,
and likely not an easy job in the remaining few weeks.  It could be
added later for JDK 10 but that means apps would likely need to roll
their own for 9.


And 8, yes, you definitely would need to roll your own, though Xuelei 
Fan already has a nice example up on his blog that was built for SNI 
(but uses the same principle).  If it were me, I wouldn't even bother 
adding it even in JDK 10, since (a) it applies only to the server side 
and (b) there are a plethora of third-party server-side network I/O and 
security libraries which are natural candidates to host this type of logic.



3.  "no_application_protocol"
=
If the server doesn't support the protocols that the client advertises,
the "no_application_protocol" must be thrown.   We could add a
"no_application_protocol" protocol String that would flag such a
condition internally.


Sure, though if you use the same method on both the client and server to 
specify the matched protocol, then the method necessarily accepts an 
array, in which case a null/unset could mean "no ALPN response" and an 
empty array could mean "no acceptable protocols".  But yeah I agree 
otherwise.



4.  Much of this is already possible.
=
If we were to go with the current API/internal and apps provided their
own ClientHello scanner, many of the benefits of what was proposed are
already available.  Apps can ask for the desired SSLContext, get the
SSLSocket/SSLengine, check the SNI/ALPN values, order/set the enabled
protocols/ciphers/etc + single ALPN value, then wrap the raw socket
using SSLSocketFactory.createSocket(Socket s, InputStream consumed,
boolean autoClose), and start the handshake.  The internal code would
still call matches() but only once.  If you want to be sure the
internals select the ApplicationProtocol, just put in a permissive
ApplicationProtocol.

The API is still more complicated unfortunately as ApplicationProtocol
is still present, but the overall behavior is quite similar.


+1 yep exactly.  I would however turn it around and also say, a more 
complex API could later be added on top of this simpler proposed 
solution, especially after more real world data is acquired, which might 
lower the overall risk as well.



5.  Other failure mode/fallback.

In the new proposal, suppose you do set a single ALPN value in the
application level, and the ServerHandshaker finds some other aspect of
the handshake wasn't appropriate (creds were mentioned several times,
but maybe a ciphersuite went dark due to new AlgorithmConstraints). This
would cause the ServerHandshaker to fail and there's no way to go back
to a different version unless you add a "for ALPN" loop into application.


Yeah all that validation would have to be done up front or manually in 
whatever server configuration is relevant.  If a cipher