RFR: 8179503: Java should support GET OCSP calls [v2]

[Xue-Lei Andrew Fan]

On Thu, 31 Dec 2020 05:31:21 GMT, Jamil Nimeh <jnimeh at openjdk.org> wrote:

>> src/java.base/share/classes/sun/security/provider/certpath/OCSP.java line 235:
>> 
>>> 233: 
>>> 234:         URL url;
>>> 235:         HttpURLConnection con;
>> 
>> I was just wondering, if the new httpclient APIs could be used here?  Maybe it is a out of the scope of this update.
>
> It probably could be done, but HttpURLConnection has been working fine so I didn't see the need to change it.

HttpURLConnection run in block mode, which could be a problem for high volume environment.  But definitely, it is not the purpose of this update.

>> src/java.base/share/classes/sun/security/provider/certpath/OCSP.java line 240:
>> 
>>> 238:                         encodeToString(bytes), "UTF-8");
>>> 239: 
>>> 240:         if (encodedGetReq.length() <= 255) {
>> 
>> If the request less than 256, the GET method will be used.  RFC 6960 declare this as a "MAY" feature ("To enable HTTP caching, small requests ... MAY be submitted using GET").  Although RFC 5019 declare it as a "MUST" feature  ("When sending requests that are less than or equal to 255 bytes ... clients MUST use the GET method"), but RFC 5109 is released before RFC 6960.  I'm not very sure if there is interop issues that a server may not accept the "Get" method for some reasons.  I don't worry about it too much, but just for sure you have considered the cases.
>
> I've tried the GET code with various public OCSP responders as well as a few things like OpenSSL's ocsp command (1.1.1d) and Dogtag 10.  There is the potential for some compatibility issues, but I think it's pretty small.  For instance, OpenSSL's 1.0.2 and earlier's ocsp command running in daemon mode does not accept GET requests at all.  But with 1.1.0 and onward, GET is fully supported.  I don't think that specifically would be an issue in any large scale deployment - I doubt those large-scale implementations use something like openssl ocsp.  There are some clients like the OCSP stapling subsystem in Nginx that will use GETs for small requests also, so I would hope that server-side support would be pretty widespread by now (HTTP GET was even in RFC 2560).

Hm, let's see if there is an interop issue in practice, before we think about a workaround.  Thank you!

>> src/java.base/share/classes/sun/security/provider/certpath/OCSP.java line 275:
>> 
>>> 273:             contentLength = Integer.MAX_VALUE;
>>> 274:         }
>>> 275: 
>> 
>> Is there a leak if the URL connection is not closed?
>
> From looking at the javadoc for URLConnection, I think the close has to happen on the Input/OutputStreams.  I have the OutputStream inside a try-with-resources block.  But I think I need to do the same with the InputStream so both are closed at the end of the transaction.

I general consider to use HttpURLConnection.disconnect() and I/O close.  Per the  HttpURLConnection class specification, "Each HttpURLConnection instance is used to make a single request but the underlying network connection to the HTTP server may be transparently shared by other instances. Calling the close() methods on the InputStream or OutputStream of an HttpURLConnection after a request may free network resources associated with this instance but has no effect on any shared persistent connection. Calling the disconnect() method may close the underlying socket if a persistent connection is otherwise idle at that time.".

-------------

PR: https://git.openjdk.java.net/jdk/pull/1760