NegotiateAuthentication: Implement additional API surface#71777
NegotiateAuthentication: Implement additional API surface#71777wfurt merged 2 commits intodotnet:mainfrom
Conversation
ghost
added
area-System.Net
new-api-needs-documentation
community-contribution
|
Note regarding the This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change. |
|
Tagging subscribers to this area: @dotnet/ncl Issue DetailsUpdated unit tests Contributes to #70909
|
filipnavara
changed the title
filipnavara
force-pushed
the
negotiate-wrap-api2
branch
from
b2ca130 to
d6f8157
Compare
Updated unit tests Migrate System.Net.Mail to use NegotiateAuthentication API
…otiateAuthentication
filipnavara
force-pushed
the
negotiate-wrap-api2
branch
from
d6f8157 to
fed4d67
Compare
|
Rebased to resolve conflicts. |
| // is negotiated. | ||
| if (OperatingSystem.IsLinux()) | ||
| { | ||
| protectionLevel = ProtectionLevel.EncryptAndSign; |
There was a problem hiding this comment.
Is there harm of doing it also on Windows/macOS?
There was a problem hiding this comment.
(FWIW: this is not a new check in this PR, it is just re-indented existing one)
In theory it would be fine on all systems but I don't have sufficient data to confirm. The negotiated protocol can end up being either Kerberos or NTLM. If it's Kerberos on Linux/macOS we get forced confidentiality anyway. For all other scenarios it could result in unilaterally bumped security requirements from the client side. If the server is Exchange then it would work. If the server is something running on an embedded device with Cyrus SASL backend I would not be so sure that everything would still work (notably the NTLM implementation in Cyrus is not even interoperable with Windows 11 in default configuration).
There was a problem hiding this comment.
Short version: It's mostly just being cautious.
There was a problem hiding this comment.
That is fine. I was nice to see less platform specific code through this PR and if we could avoid one more platform check I would be happy. But even with it it seems like great improvement.
|
Thanks for review! I'm very happy that we managed to get the API in at the last minute :-) |
ghost
locked as resolved and limited conversation to collaborators
bartonjs
added
cryptographic-docs-impact
needs-further-triage
4 participants
Best reviewed commit by commit.
Fixes #70909