Skip to content

Commit 24ce88f

Browse files
committed
X509ChainImplUnityTls reports status now
Fixes Fogbugz ticket 1261388. Impl sticks close to current Mono Btls implementation on _master_ - the implementation on our fork has the same issues as prior to this fix and throws NotImplementedException
1 parent fb1dbb3 commit 24ce88f

File tree

3 files changed

+43
-5
lines changed

3 files changed

+43
-5
lines changed

mcs/class/System/Mono.UnityTls/UnityTlsConversions.cs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#endif
99

1010
using System.Security.Authentication;
11+
using System.Security.Cryptography.X509Certificates;
1112

1213
namespace Mono.Unity
1314
{
@@ -102,6 +103,30 @@ public static MonoSslPolicyErrors VerifyResultToPolicyErrror (UnityTls.unitytls_
102103
error |= MonoSslPolicyErrors.RemoteCertificateChainErrors;
103104
return error;
104105
}
106+
107+
public static X509ChainStatusFlags VerifyResultToChainStatus (UnityTls.unitytls_x509verify_result verifyResult)
108+
{
109+
// First, check "non-flags"
110+
if (verifyResult == UnityTls.unitytls_x509verify_result.UNITYTLS_X509VERIFY_SUCCESS)
111+
return X509ChainStatusFlags.NoError;
112+
else if (verifyResult == UnityTls.unitytls_x509verify_result.UNITYTLS_X509VERIFY_FATAL_ERROR)
113+
return X509ChainStatusFlags.UntrustedRoot; // Inaccurate, throw exception instead?
114+
115+
// Yes, we ignore user error flags here. They still affect if a chain is accepted, but they are not status flags of the chain!
116+
X509ChainStatusFlags error = X509ChainStatusFlags.NoError;
117+
if (verifyResult.HasFlag (UnityTls.unitytls_x509verify_result.UNITYTLS_X509VERIFY_FLAG_EXPIRED))
118+
error |= X509ChainStatusFlags.NotTimeValid;
119+
if (verifyResult.HasFlag (UnityTls.unitytls_x509verify_result.UNITYTLS_X509VERIFY_FLAG_REVOKED))
120+
error |= X509ChainStatusFlags.Revoked;
121+
if (verifyResult.HasFlag (UnityTls.unitytls_x509verify_result.UNITYTLS_X509VERIFY_FLAG_CN_MISMATCH))
122+
// Unclear what to return, behaving like Mono's BTLS impl
123+
// https://github.com/mono/mono/blob/1553889bc54f87060158febca7e6b8b9910975f8/mcs/class/System/Mono.Btls/MonoBtlsProvider.cs#L312
124+
error |= X509ChainStatusFlags.UntrustedRoot;
125+
if (verifyResult.HasFlag (UnityTls.unitytls_x509verify_result.UNITYTLS_X509VERIFY_FLAG_NOT_TRUSTED))
126+
error |= X509ChainStatusFlags.UntrustedRoot;
127+
128+
return error;
129+
}
105130
}
106131
}
107132
#endif

mcs/class/System/Mono.UnityTls/UnityTlsProvider.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,12 @@ internal override bool ValidateCertificate (
131131
}
132132

133133
errors = UnityTlsConversions.VerifyResultToPolicyErrror(result);
134+
// There should be a status per certificate, but once again we're following closely the BTLS implementation
135+
// https://github.com/mono/mono/blob/1553889bc54f87060158febca7e6b8b9910975f8/mcs/class/System/Mono.Btls/MonoBtlsProvider.cs#L180
136+
// which also provides only a single status for the entire chain.
137+
// It is notoriously tricky to implement in OpenSSL to get a status for all invididual certificates without finishing the handshake in the process.
138+
// This is partially the reason why unitytls_x509verify_X doesn't expose it (TODO!) and likely the reason Mono's BTLS impl ignores this.
139+
unityTlsChainImpl?.AddStatus(UnityTlsConversions.VerifyResultToChainStatus(result));
134140
return result == UnityTls.unitytls_x509verify_result.UNITYTLS_X509VERIFY_SUCCESS &&
135141
errorState.code == UnityTls.unitytls_error_code.UNITYTLS_SUCCESS;
136142
}

mcs/class/System/Mono.UnityTls/X509ChainImplUnityTls.cs

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#if SECURITY_DEP
22

33
using System;
4+
using System.Collections.Generic;
45
using System.Text;
56
using System.Security;
67
using System.Security.Cryptography;
@@ -12,9 +13,10 @@ namespace Mono.Unity
1213
// Follows mostly X509ChainImplBtls
1314
class X509ChainImplUnityTls : X509ChainImpl
1415
{
15-
X509ChainElementCollection elements;
16-
UnityTls.unitytls_x509list_ref nativeCertificateChain;
17-
X509ChainPolicy policy = new X509ChainPolicy ();
16+
private X509ChainElementCollection elements;
17+
private UnityTls.unitytls_x509list_ref nativeCertificateChain;
18+
private X509ChainPolicy policy = new X509ChainPolicy ();
19+
private List<X509ChainStatus> chainStatusList;
1820

1921
internal X509ChainImplUnityTls (UnityTls.unitytls_x509list_ref nativeCertificateChain)
2022
{
@@ -64,8 +66,13 @@ public override X509ChainPolicy ChainPolicy {
6466
set { policy = value; }
6567
}
6668

67-
public override X509ChainStatus[] ChainStatus {
68-
get { throw new NotImplementedException (); }
69+
public override X509ChainStatus[] ChainStatus => chainStatusList?.ToArray() ?? new X509ChainStatus[0];
70+
71+
public void AddStatus (X509ChainStatusFlags errorCode)
72+
{
73+
if (chainStatusList == null)
74+
chainStatusList = new List<X509ChainStatus>();
75+
chainStatusList.Add (new X509ChainStatus(errorCode));
6976
}
7077

7178
public override bool Build (X509Certificate2 certificate)

0 commit comments

Comments
 (0)