Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Commit 00bcad0

Browse files
krwqstephentoub
authored andcommitted
SslStream mutates state of options (#28666)
* decouple SslState from SslServerAuthenticationOptions * rename certCallback to certSelectionCallback * remove mutable ServerCertSelectionCallback from SslServerAuthenticationOptions * remove mutable RemoteCertValidationCallback from SslServerAuthenticationOptions * fix FakeSslState to match args
1 parent 6f38488 commit 00bcad0

File tree

5 files changed

+26
-32
lines changed

5 files changed

+26
-32
lines changed

src/System.Net.Security/src/System/Net/Security/SslAuthenticationOptions.cs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ internal SslAuthenticationOptions(SslServerAuthenticationOptions sslServerAuthen
3737
// Common options.
3838
AllowRenegotiation = sslServerAuthenticationOptions.AllowRenegotiation;
3939
ApplicationProtocols = sslServerAuthenticationOptions.ApplicationProtocols;
40-
CertValidationDelegate = sslServerAuthenticationOptions._certValidationDelegate;
4140
CheckCertName = false;
4241
EnabledSslProtocols = sslServerAuthenticationOptions.EnabledSslProtocols;
4342
EncryptionPolicy = sslServerAuthenticationOptions.EncryptionPolicy;
@@ -49,7 +48,6 @@ internal SslAuthenticationOptions(SslServerAuthenticationOptions sslServerAuthen
4948
// Server specific options.
5049
CertificateRevocationCheckMode = sslServerAuthenticationOptions.CertificateRevocationCheckMode;
5150
ServerCertificate = sslServerAuthenticationOptions.ServerCertificate;
52-
ServerCertSelectionDelegate = sslServerAuthenticationOptions._serverCertDelegate;
5351
}
5452

5553
internal bool AllowRenegotiation { get; set; }
@@ -67,7 +65,7 @@ internal SslAuthenticationOptions(SslServerAuthenticationOptions sslServerAuthen
6765
internal bool CheckCertName { get; set; }
6866
internal RemoteCertValidationCallback CertValidationDelegate { get; set; }
6967
internal LocalCertSelectionCallback CertSelectionDelegate { get; set; }
70-
internal ServerCertCallback ServerCertSelectionDelegate { get; set; }
68+
internal ServerCertSelectionCallback ServerCertSelectionDelegate { get; set; }
7169
}
7270
}
7371

src/System.Net.Security/src/System/Net/Security/SslServerAuthenticationOptions.cs

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,6 @@ public class SslServerAuthenticationOptions
1515
private EncryptionPolicy _encryptionPolicy = EncryptionPolicy.RequireEncryption;
1616
private bool _allowRenegotiation = true;
1717

18-
internal RemoteCertValidationCallback _certValidationDelegate;
19-
internal ServerCertCallback _serverCertDelegate;
20-
2118
public bool AllowRenegotiation
2219
{
2320
get => _allowRenegotiation;

src/System.Net.Security/src/System/Net/Security/SslState.cs

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ internal void ValidateCreateContext(SslClientAuthenticationOptions sslClientAuth
132132
}
133133
}
134134

135-
internal void ValidateCreateContext(SslServerAuthenticationOptions sslServerAuthenticationOptions)
135+
internal void ValidateCreateContext(SslAuthenticationOptions sslAuthenticationOptions)
136136
{
137137
ThrowIfExceptional();
138138

@@ -146,20 +146,11 @@ internal void ValidateCreateContext(SslServerAuthenticationOptions sslServerAuth
146146
throw new InvalidOperationException(SR.net_auth_client_server);
147147
}
148148

149-
if (sslServerAuthenticationOptions.ServerCertificate == null && sslServerAuthenticationOptions._serverCertDelegate == null)
150-
{
151-
throw new ArgumentNullException(nameof(sslServerAuthenticationOptions.ServerCertificate));
152-
}
153-
154-
if (sslServerAuthenticationOptions.ServerCertificate != null && sslServerAuthenticationOptions._serverCertDelegate != null)
155-
{
156-
throw new InvalidOperationException(SR.Format(SR.net_conflicting_options, nameof(ServerCertificateSelectionCallback)));
157-
}
158-
159149
_exception = null;
150+
_sslAuthenticationOptions = sslAuthenticationOptions;
151+
160152
try
161153
{
162-
_sslAuthenticationOptions = new SslAuthenticationOptions(sslServerAuthenticationOptions);
163154
_context = new SecureChannel(_sslAuthenticationOptions);
164155
}
165156
catch (Win32Exception e)

src/System.Net.Security/src/System/Net/Security/SslStream.cs

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ public enum EncryptionPolicy
3636
// Internal versions of the above delegates.
3737
internal delegate bool RemoteCertValidationCallback(string host, X509Certificate2 certificate, X509Chain chain, SslPolicyErrors sslPolicyErrors);
3838
internal delegate X509Certificate LocalCertSelectionCallback(string targetHost, X509CertificateCollection localCertificates, X509Certificate2 remoteCertificate, string[] acceptableIssuers);
39-
internal delegate X509Certificate ServerCertCallback(string hostName);
39+
internal delegate X509Certificate ServerCertSelectionCallback(string hostName);
4040

4141
public class SslStream : AuthenticatedStream
4242
{
@@ -150,10 +150,26 @@ private X509Certificate ServerCertSelectionCallbackWrapper(string targetHost)
150150
return _userServerCertificateSelectionCallback(this, targetHost);
151151
}
152152

153-
private void SetServerCertificateSelectionCallbackWrapper(SslServerAuthenticationOptions sslServerAuthenticationOptions)
153+
private SslAuthenticationOptions CreateAuthenticationOptions(SslServerAuthenticationOptions sslServerAuthenticationOptions)
154154
{
155+
if (sslServerAuthenticationOptions.ServerCertificate == null && sslServerAuthenticationOptions.ServerCertificateSelectionCallback == null)
156+
{
157+
throw new ArgumentNullException(nameof(sslServerAuthenticationOptions.ServerCertificate));
158+
}
159+
160+
if (sslServerAuthenticationOptions.ServerCertificate != null && sslServerAuthenticationOptions.ServerCertificateSelectionCallback != null)
161+
{
162+
throw new InvalidOperationException(SR.Format(SR.net_conflicting_options, nameof(ServerCertificateSelectionCallback)));
163+
}
164+
165+
var authOptions = new SslAuthenticationOptions(sslServerAuthenticationOptions);
166+
155167
_userServerCertificateSelectionCallback = sslServerAuthenticationOptions.ServerCertificateSelectionCallback;
156-
sslServerAuthenticationOptions._serverCertDelegate = _userServerCertificateSelectionCallback == null ? null : new ServerCertCallback(ServerCertSelectionCallbackWrapper);
168+
authOptions.ServerCertSelectionDelegate = _userServerCertificateSelectionCallback == null ? null : new ServerCertSelectionCallback(ServerCertSelectionCallbackWrapper);
169+
170+
authOptions.CertValidationDelegate = _certValidationDelegate;
171+
172+
return authOptions;
157173
}
158174

159175
//
@@ -244,12 +260,7 @@ private IAsyncResult BeginAuthenticateAsServer(SslServerAuthenticationOptions ss
244260
SecurityProtocol.ThrowOnNotAllowed(sslServerAuthenticationOptions.EnabledSslProtocols);
245261
SetAndVerifyValidationCallback(sslServerAuthenticationOptions.RemoteCertificateValidationCallback);
246262

247-
// Set the delegate on the options.
248-
sslServerAuthenticationOptions._certValidationDelegate = _certValidationDelegate;
249-
250-
SetServerCertificateSelectionCallbackWrapper(sslServerAuthenticationOptions);
251-
252-
_sslState.ValidateCreateContext(sslServerAuthenticationOptions);
263+
_sslState.ValidateCreateContext(CreateAuthenticationOptions(sslServerAuthenticationOptions));
253264

254265
LazyAsyncResult result = new LazyAsyncResult(_sslState, asyncState, asyncCallback);
255266
_sslState.ProcessAuthentication(result);
@@ -348,10 +359,7 @@ private void AuthenticateAsServer(SslServerAuthenticationOptions sslServerAuthen
348359
SecurityProtocol.ThrowOnNotAllowed(sslServerAuthenticationOptions.EnabledSslProtocols);
349360
SetAndVerifyValidationCallback(sslServerAuthenticationOptions.RemoteCertificateValidationCallback);
350361

351-
// Set the delegate on the options.
352-
sslServerAuthenticationOptions._certValidationDelegate = _certValidationDelegate;
353-
354-
_sslState.ValidateCreateContext(sslServerAuthenticationOptions);
362+
_sslState.ValidateCreateContext(CreateAuthenticationOptions(sslServerAuthenticationOptions));
355363
_sslState.ProcessAuthentication(null);
356364
}
357365
#endregion

src/System.Net.Security/tests/UnitTests/Fakes/FakeSslState.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ internal void ValidateCreateContext(SslClientAuthenticationOptions sslClientAuth
2525
{
2626
}
2727

28-
internal void ValidateCreateContext(SslServerAuthenticationOptions sslServerAuthenticationOptions)
28+
internal void ValidateCreateContext(SslAuthenticationOptions sslAuthenticationOptions)
2929
{
3030
}
3131

0 commit comments

Comments
 (0)