Merge pull request #1401 from Unity-Technologies/unity-master-include-system-store-certificates-in-callback

Unity master include system store certificates in callback

Author: AndreasReich

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

@@ -48,6 +48,13 @@ internal override IMonoSslStream CreateSslStreamInternal (
 			return new UnityTlsStream (innerStream, leaveInnerStreamOpen, sslStream, settings, this);
 		}
 
+		static UnityTls.unitytls_x509verify_result x509verify_callback(void* userData, UnityTls.unitytls_x509_ref cert, UnityTls.unitytls_x509verify_result result, UnityTls.unitytls_errorstate* errorState)
+		{
+			if (userData != null)
+				UnityTls.NativeInterface.unitytls_x509list_append ((UnityTls.unitytls_x509list*)userData, cert, errorState);
+			return result;
+		}
+
 		internal override bool ValidateCertificate (
 			ICertificateValidator2 validator, string targetHost, bool serverMode,
 			X509CertificateCollection certificates, bool wantsChain, ref X509Chain chain,
@@ -62,9 +69,6 @@ internal override bool ValidateCertificate (
 					errors |= MonoSslPolicyErrors.RemoteCertificateNotAvailable;
 					return false;
 				}
-
-				if (wantsChain)
-					chain = MNS.SystemCertificateValidator.CreateX509Chain (certificates);
 			}
 			else
 			{
@@ -85,6 +89,7 @@ internal override bool ValidateCertificate (
 			// convert cert to native or extract from unityTlsChainImpl.
 			var result = UnityTls.unitytls_x509verify_result.UNITYTLS_X509VERIFY_NOT_DONE;
 			UnityTls.unitytls_x509list* certificatesNative = null;
+			UnityTls.unitytls_x509list* finalCertificateChainNative = UnityTls.NativeInterface.unitytls_x509list_create (&errorState);
 			try
 			{
 				// Things the validator provides that we might want to make use of here:
@@ -114,29 +119,42 @@ internal override bool ValidateCertificate (
 						var trustCAnativeRef = UnityTls.NativeInterface.unitytls_x509list_get_ref (trustCAnative, &errorState);
 
 						fixed (byte* targetHostUtf8Ptr = targetHostUtf8) {
-							result = UnityTls.NativeInterface.unitytls_x509verify_explicit_ca (certificatesNativeRef, trustCAnativeRef, targetHostUtf8Ptr, (size_t)targetHostUtf8.Length, null, null, &errorState);
+							result = UnityTls.NativeInterface.unitytls_x509verify_explicit_ca (
+								certificatesNativeRef, trustCAnativeRef, targetHostUtf8Ptr, (size_t)targetHostUtf8.Length, x509verify_callback, finalCertificateChainNative, &errorState);
 						}
 					}
 					finally {
 						UnityTls.NativeInterface.unitytls_x509list_free (trustCAnative);
 					}
 				} else {
 					fixed (byte* targetHostUtf8Ptr = targetHostUtf8) {
-						result = UnityTls.NativeInterface.unitytls_x509verify_default_ca (certificatesNativeRef, targetHostUtf8Ptr, (size_t)targetHostUtf8.Length, null, null, &errorState);
+						result = UnityTls.NativeInterface.unitytls_x509verify_default_ca (
+							certificatesNativeRef, targetHostUtf8Ptr, (size_t)targetHostUtf8.Length, x509verify_callback, finalCertificateChainNative, &errorState);
 					}
 				}
 			}
+			catch {
+				UnityTls.NativeInterface.unitytls_x509list_free (finalCertificateChainNative);
+				throw;
+			}
 			finally	{
 				UnityTls.NativeInterface.unitytls_x509list_free (certificatesNative);
 			}
 
+			chain?.Dispose();
+			var chainImpl = new X509ChainImplUnityTls(
+				UnityTls.NativeInterface.unitytls_x509list_get_ref (finalCertificateChainNative, &errorState),
+				reverseOrder: true // the verify callback starts with the root and ends with the leaf. That's the opposite of chain ordering.
+			);
+			chain = new X509Chain(chainImpl);
+
 			errors = UnityTlsConversions.VerifyResultToPolicyErrror(result);
 			// There should be a status per certificate, but once again we're following closely the BTLS implementation
 			// https://github.com/mono/mono/blob/1553889bc54f87060158febca7e6b8b9910975f8/mcs/class/System/Mono.Btls/MonoBtlsProvider.cs#L180
 			// which also provides only a single status for the entire chain.
 			// It is notoriously tricky to implement in OpenSSL to get a status for all invididual certificates without finishing the handshake in the process.
 			// This is partially the reason why unitytls_x509verify_X doesn't expose it (TODO!) and likely the reason Mono's BTLS impl ignores this.
-			unityTlsChainImpl?.AddStatus(UnityTlsConversions.VerifyResultToChainStatus(result));
+			chainImpl.AddStatus(UnityTlsConversions.VerifyResultToChainStatus(result));
 			return result == UnityTls.unitytls_x509verify_result.UNITYTLS_X509VERIFY_SUCCESS && 
 					errorState.code == UnityTls.unitytls_error_code.UNITYTLS_SUCCESS;
 		}

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

@@ -17,11 +17,13 @@ class X509ChainImplUnityTls : X509ChainImpl
 		private UnityTls.unitytls_x509list_ref nativeCertificateChain;
 		private X509ChainPolicy policy = new X509ChainPolicy ();
 		private List<X509ChainStatus> chainStatusList;
+		private bool reverseOrder;
 
-		internal X509ChainImplUnityTls (UnityTls.unitytls_x509list_ref nativeCertificateChain)
+		internal X509ChainImplUnityTls (UnityTls.unitytls_x509list_ref nativeCertificateChain, bool reverseOrder = false)
 		{
 			this.elements = null;
 			this.nativeCertificateChain = nativeCertificateChain;
+			this.reverseOrder = reverseOrder;
 		}
 
 		public override bool IsValid {
@@ -45,7 +47,7 @@ public override X509ChainElementCollection ChainElements {
 					elements = new X509ChainElementCollection ();
 					UnityTls.unitytls_errorstate errorState = UnityTls.NativeInterface.unitytls_errorstate_create ();
 					var cert = UnityTls.NativeInterface.unitytls_x509list_get_x509 (nativeCertificateChain, (size_t)0, &errorState);
-					for (int i = 0; cert.handle != UnityTls.NativeInterface.UNITYTLS_INVALID_HANDLE; ++i) {
+					for (int i = 1; cert.handle != UnityTls.NativeInterface.UNITYTLS_INVALID_HANDLE; ++i) {
 						size_t certBufferSize = UnityTls.NativeInterface.unitytls_x509_export_der (cert, null, (size_t)0, &errorState);
 						var certBuffer = new byte[(int)certBufferSize];	// Need to reallocate every time since X509Certificate constructor takes no length but only a byte array.
 						fixed(byte* certBufferPtr = certBuffer) {
@@ -57,6 +59,13 @@ public override X509ChainElementCollection ChainElements {
 					}
 				}
 
+				if (reverseOrder) {
+					var reversed = new X509ChainElementCollection ();
+					for (int i=elements.Count - 1; i>=0; --i)
+						reversed.Add(elements[i].Certificate);
+					elements = reversed;
+				}
+
 				return elements;
 			}
 		}