Resolve breaking change with SSL exception

[BrennanConroy]

We updated runtime dependencies which resulted in a couple SSL tests failing due to an unexpected exception type being thrown. This is occurring on Fedora, Debian, and Ubuntu 18 test runs only.

The temporary change to make the tests pass is c180668.

Throwing a different (and inconsistent) exception is a regression.

We should figure out what changed, why, and how we want to resolve it.

I'm guessing the change is from dotnet/runtime#453

@Tratcher @jkotalik @halter73 @anurse @wfurt

[wfurt]

Do you have more details @BrennanConroy? Do you know if the systems listed have OpenSSL 1.1.1? (depends on patch level)

[BrennanConroy]

I have no idea. I just happened to be around when the change came through and "fixed" it and filed the issue. Please follow up with one of the folks I mentioned above.

[analogrelay]

@wfurt It does seem to be OS-specific behavior because it only failed on Fedora, Debian and Ubuntu 18 runs in Helix.

@BrennanConroy can you include the Helix Queue names that failed (the full names).

[wfurt]

If dotnet/runtime#914 is the root cause for this OS-specific behavior would be expected. We would now properly send/receive TLS alerts. The general expectation is that if peer sends alert we would throw AuthenticationException. If peer closes the connection or if there is an incomplete frame we would throw IOException.
But older OpenSSL versions did not send alerts in all cases where new versions do.

Is there some way how to reproduce it locally so I can investigate? I'm not that familiar with aspnetcore repo so I would need some guidance.

[BrennanConroy]

Helix Queues

• (Fedora.28.Amd64.Open)[email protected]/dotnet-buildtools/prereqs:fedora-28-helix-09ca40b-20190508143249
• (Debian.9.Arm64.Open)[email protected]/dotnet-buildtools/prereqs:debian-9-helix-arm64v8-a12566d-20190807161036
• (Debian.9.Arm64.Open)[email protected]/dotnet-buildtools/prereqs:debian-9-helix-arm64v8-a12566d-20190807161036

You can follow the docs to build the repo, or at least the Kestrel part of it. And then run the test at

aspnetcore/src/Servers/Kestrel/test/InMemory.FunctionalTests/HttpsTests.cs

Line 358 in 2cc333a

public async Task ClientAttemptingToUseUnsupportedProtocolIsLoggedAsDebug()

[analogrelay]

@wfurt Can we get some clarity on whether this break (to throw a different exception in some circumstances) was intentional and will be documented as a breaking change? If so, we can just leave our reaction in place and there's no action here from our side, but if not we'd like to revert it.

[analogrelay]

I presume this was intentional then. No further action from our side now, closing this.

[wfurt]

Sorry for the delay. It may not matter of purpose if the tests but we should only throw IOException or Authentication exception in case of mismatch - depending on the underlying implementation. However, updated tests look ok to me so no further action is needed IMHO.