[QUIC] Certificate name validation#56175
Conversation
|
Tagging subscribers to this area: @dotnet/ncl Issue DetailsExtracts Fixes #55193 For now just manually verified in asp.net core test. I will add specific tests tomorrow. cc: @wfurt
|
src/libraries/System.Net.Quic/src/System/Net/Quic/Implementations/MsQuic/MsQuicConnection.cs
Outdated
Show resolved
Hide resolved
|
with this, validation should be easier in ldap @AnthonyMastrean (#55017) |
wfurt
left a comment
There was a problem hiding this comment.
generally looks god to me. We should add some testes.
- set TargetHost to something else than "loopback" with existing cert and the name error should show up.
- In ideal case we should check that connection to 127.0.0.1 and ::1 works without getting the name error since they both should be in altName (at least the one we generate)
I think current tests ignore all the passed info and simple call new chain rebuild without name check.
| <data name="net_quic_writing_notallowed" xml:space="preserve"> | ||
| <value>Writing is not allowed on stream.</value> | ||
| </data> | ||
| <data name="net_ssl_app_protocols_invalid" xml:space="preserve"> |
There was a problem hiding this comment.
do we need this? looks like extra.
There was a problem hiding this comment.
Yes we need this, it's used by one of the included interop source files:
| </ItemGroup> | ||
|
|
||
| <ItemGroup> | ||
| <ProjectReference Include="$(LibrariesProjectRoot)System.Security.Cryptography.OpenSsl\src\System.Security.Cryptography.OpenSsl.csproj" /> |
There was a problem hiding this comment.
Do we still need this with all the added interop?
There was a problem hiding this comment.
Yes, we do, the interop uses public classes from this project. It doesn't build without it and the same it's referenced in S.N.Http.
|
@wfurt tests added as you suggested. If you're happy with it, feel free to merge on my behalf. |
Extracts
CertificateValidationfor Windows into shared sources and make use of it and the Unix version in S.N.Quic to properly verify the cert the same waySslStreamdoes.Fixes #55193
For now just manually verified in asp.net core test. I will add specific tests tomorrow.
cc: @wfurt