Support sites with invalid IDN in SslStream #82934
Conversation
|
Tagging subscribers to this area: @dotnet/ncl, @vcsjones Issue Detailsfixes #82464 This is attempt to allow TLS handshake on site with invalid IDN to match browsers and other HTTP/TLS implementations. Since this is somewhat corner case the change is focusing on allowing the handshake go through via setting policy to ignore name. It does not try to verify that the problematic names would for example match wildcard certificates. It would be up to the caller to verify certificates as most browsers would also show warning and not proceed automatically. Handling has been centralize to I'm not sure how this impacts Android @simonrozsival. I would think that doing the puny code before calling Java should be just fine since that is what should go out on wire.
|
|
@wfurt yeah, this change should be just fine for Android. We can run tests on Android just to make sure. |
|
/azp run runtime-android |
|
Azure Pipelines successfully started running 1 pipeline(s). |
src/libraries/System.Net.Security/src/System/Net/Security/SslAuthenticationOptions.cs
Outdated
Show resolved
Hide resolved
| @@ -46,6 +49,20 @@ private static unsafe bool IsValidAddress(ReadOnlySpan<char> ipSpan) | |||
| return false; | |||
| } | |||
|
|
|||
| private static unsafe bool IsSafeDnsString(ReadOnlySpan<char> name) | |||
| { | |||
| for (int i = 0; i < name.Length; i++) | |||
There was a problem hiding this comment.
This could use the new IndexOfAnyValues<T>, but not sure if the allocation of static map is worth it for such short strings.
There was a problem hiding this comment.
Yes, this should use IndexOfAnyValues. The allocation there is very small, we'll only store a bitmap for the ASCII range.
// Alphanumeric or '-', '.', '_'
private static readonly IndexOfAnyValues<char> s_safeDnsChars =
IndexOfAnyValues.Create("-.0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ_abcdefghijklmnopqrstuvwxyz");
private static bool IsSafeDnsString(ReadOnlySpan<char> name) =>
name.IndexOfAnyExcept(s_safeDnsChars) < 0;There was a problem hiding this comment.
IndexOfAnyValues crossed my mind but the set of characters I don't want is large for Unicode. I was not aware of IndexOfAnyExcept
There was a problem hiding this comment.
updated. Just to be sure: this would only match that ASCII characters, not their Unicode equivalent (if any), right @MihaZupan ?
There was a problem hiding this comment.
That's right, the set of values you pass to Create is "ordinal".
|
thanks @simonrozsival. I did not know you can just run Android. I use |
|
@wfurt The When I comment out the |
|
/azp run runtime-android |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run runtime-android |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
test failures are unrelated |
ghost
locked as resolved and limited conversation to collaborators
fixes #82464
This is attempt to allow TLS handshake on site with invalid IDN to match browsers and other HTTP/TLS implementations.
Since this is somewhat corner case the change is focusing on allowing the handshake go through via setting policy to ignore name. It does not try to verify that the problematic names would for example match wildcard certificates. It would be up to the caller to verify certificates as most browsers would also show warning and not proceed automatically.
Handling has been centralize to
SslAuthenticationOptionsand eliminated code scattered in PAL.I'm not sure how this impacts Android @simonrozsival. I would think that doing the puny code before calling Java should be just fine since that is what should go out on wire.