RFR: 8293176: SSLEngine handshaker does not send an alert after a bad parameters

[Matthew Donovan]

On Fri, 4 Aug 2023 08:17:39 GMT, Daniel Jeliński <djelinski at openjdk.org> wrote:

> Please review this patch that ensures that all exceptions thrown by SSLEngine delegated tasks are translated to alerts.
> 
> All exceptions should already be translated to SSLExceptions and alerts by the time we exit from context.dispatch; these exceptions are rethrown by `conContext.fatal` without modification. With this patch the remaining exceptions are translated to `internal_error` alerts.
> 
> SSLSocket implements similar handling in SSLSocketImpl#startHandshake. SSLSocket rethrows `SocketException`s without modification, and translates other `IOException`s to `handshake_failure` alerts. SSLEngine does not need to handle `SocketException`s, and IMO `internal_error` is a better choice here.
> 
> Tier1-3 tests pass.

test/jdk/sun/security/ssl/SSLEngineImpl/SSLEngineDecodeBadPoint.java line 40:

> 38: public class SSLEngineDecodeBadPoint {
> 39:     static final byte[] clientHello = HexFormat.of().parseHex(
> 40:             "160303013a0100013603031570" +

This may be the github display but this line is indented differently than the others.

test/jdk/sun/security/ssl/SSLEngineImpl/SSLEngineDecodeBadPoint.java line 76:

> 74:             eng.wrap(emptyBuf, alert);
> 75:             throw new RuntimeException("Expected wrap to throw");
> 76:         } catch (Exception e) {

Catching `Exception` here will consume the RuntimeException being thrown.

test/jdk/sun/security/ssl/SSLEngineImpl/SSLEngineDecodeBadPoint.java line 77:

> 75:             throw new RuntimeException("Expected wrap to throw");
> 76:         } catch (Exception e) {
> 77:             System.err.println("Received expected exception:");

I find it useful when looking at logs to have the message explicitly say what exception is expected.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/15148#discussion_r1291554810
PR Review Comment: https://git.openjdk.org/jdk/pull/15148#discussion_r1291553638
PR Review Comment: https://git.openjdk.org/jdk/pull/15148#discussion_r1291554198