Skip to content

Commit 231cf40

Browse files
AndreasReichUnityAlex
authored andcommitted
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 add2078 commit 231cf40

File tree

3 files changed

+41
-6
lines changed

3 files changed

+41
-6
lines changed

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

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

1010
using System.Security.Authentication;
1111
using System.Net.Security;
12+
using System.Security.Cryptography.X509Certificates;
1213

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

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

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

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

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

Lines changed: 10 additions & 6 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
{
@@ -61,16 +63,18 @@ public override X509ChainElementCollection ChainElements {
6163

6264
public override void AddStatus (X509ChainStatusFlags error)
6365
{
66+
if (chainStatusList == null)
67+
chainStatusList = new List<X509ChainStatus>();
68+
chainStatusList.Add (new X509ChainStatus(error));
6469
}
6570

6671
public override X509ChainPolicy ChainPolicy {
6772
get { return policy; }
6873
set { policy = value; }
6974
}
7075

71-
public override X509ChainStatus[] ChainStatus {
72-
get { throw new NotImplementedException (); }
73-
}
76+
public override X509ChainStatus[] ChainStatus => chainStatusList?.ToArray() ?? new X509ChainStatus[0];
77+
7478

7579
public override bool Build (X509Certificate2 certificate)
7680
{

0 commit comments

Comments
 (0)