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

[Bradford Wetmore]

On Fri, 11 Aug 2023 21:38:04 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.
>
> Daniel Jeliński has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - Fix exception handling
>  - Fix indentation

This comment comes too late to catch this integration, but a thought as I was looking over the code.

src/java.base/share/classes/sun/security/ssl/SSLEngineImpl.java line 1278:

> 1276:                             throw context.conContext.fatal(Alert.INTERNAL_ERROR,
> 1277:                                     "Unhandled exception", e);
> 1278:                         }

Is there any chance this will double alert?

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

PR Review: https://git.openjdk.org/jdk/pull/15148#pullrequestreview-1647747066
PR Review Comment: https://git.openjdk.org/jdk/pull/15148#discussion_r1339387873