RFR: 8283577: SSLEngine.unwrap on read-only input ByteBuffer

[Bradford Wetmore]

On Fri, 29 Apr 2022 03:58:57 GMT, Anthony Scarpino <ascarpino at openjdk.org> wrote:

> Hi,
> 
> I need a review of this fix to allow a read-only 'src' buffer to be used with SSLEngine.unwrap(). A temporary read-write buffer is created in the SSLCipher operation when a read-only buffer is passed. If the 'src' is read-write, there is no effect on the current operation
> 
> The PR also includes a CSR for an API implementation note to the SSLEngine.unwrap. The 'src' buffer may be modified during the decryption operation. 'unwrap()' has had this behavior forever, so there is no compatibility issue with this note. Using the 'src' buffer for in-place decryption was a performance decision.
> 
> Tony

Finished the SSLEngine/tests, starting on the SSLCipher.  Here's the notes so far.

test/jdk/javax/net/ssl/SSLSession/ReadOnlyEngine.java line 1:

> 1: /*

Wondering why this is in javax/net/ssl/SSLSession instead of sun/security/ssl/SSLCipher.

test/jdk/javax/net/ssl/SSLSession/ReadOnlyEngine.java line 51:

> 49: public class ReadOnlyEngine {
> 50: 
> 51:     private static String pathToStores = "../etc";

These 6 can be final if you want.

test/jdk/javax/net/ssl/SSLSession/ReadOnlyEngine.java line 96:

> 94:                     status = engine.getHandshakeStatus();
> 95:                     break;
> 96:                 case FINISHED:

Can combine FINISHED/NOT_HANDSHAKING?

test/jdk/javax/net/ssl/SSLSession/ReadOnlyEngine.java line 157:

> 155:         // Do TLS handshake
> 156:         do {
> 157:             statusClient = doHandshake(client, out, in);

It's potentially a little inefficient returning after each wrap/unwrap() instead of doing the task right away, but it works.  No need to change.

Is this style something you copied from another test?  The  SSLEngineTemplate in the templates directory is what I often use.

test/jdk/javax/net/ssl/SSLSession/ReadOnlyEngine.java line 160:

> 158:             statusServer = doHandshake(server, in, out);
> 159:         } while (statusClient != HandshakeStatus.NOT_HANDSHAKING ||
> 160:             statusServer != HandshakeStatus.NOT_HANDSHAKING);

Minor indent problem.

test/jdk/javax/net/ssl/SSLSession/ReadOnlyEngine.java line 162:

> 160:             statusServer != HandshakeStatus.NOT_HANDSHAKING);
> 161: 
> 162:         // Read NST

What is NST?

test/jdk/javax/net/ssl/SSLSession/ReadOnlyEngine.java line 172:

> 170:         out.clear();
> 171:         String testString = "ASDF";
> 172:         in.put(testString.getBytes()).flip();

If you're going to convert back from UTF_8 later, you should probably convert using getBytes(UTF_8) here.

test/jdk/javax/net/ssl/SSLSession/ReadOnlyEngine.java line 188:

> 186:         in.clear();
> 187:         System.out.println("2: Server send: " + testString);
> 188:         in.put(testString.getBytes()).flip();

Same

test/jdk/javax/net/ssl/SSLSession/ReadOnlyEngine.java line 189:

> 187:         System.out.println("2: Server send: " + testString);
> 188:         in.put(testString.getBytes()).flip();
> 189:         send(server, in, out);

Did you want to try asReadOnlyBuffer() here also?

test/jdk/javax/net/ssl/SSLSession/ReadOnlyEngine.java line 191:

> 189:         send(server, in, out);
> 190:         in.clear();
> 191:         receive(client, out, in);

And here?

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

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