From 65b7ea1162e9338c8014344e99ca6137e5fa8ba5 Mon Sep 17 00:00:00 2001
From: Emmanuel Dreyfus <manu@netbsd.org>
Date: Mon, 30 Oct 2017 15:45:51 +0100
Subject: [PATCH] Fix certificate validation

- Remove X509ChainPolicy usage. It was there to avoid CRL
  lookups, but it caused validation to always fail. Just using
  X509Certificate2::Verify() just works, even if the CA
  references an unavailable CRL.

- Verify that the LDAP host name matches the certificate CN or
  one of its SubjectAltName. Failure to do that leaves pGina
  vulnerable to Man In The Middle setups, where the attacker
  just use a certificate signed by a valid CA for another machine.
  Since the Windows stores contain mainy CA, thie setup si quite
  easy to acheive.
---
 Plugins/LdapPlugin/Ldap/LdapServer.cs | 64 ++++++++++++++++++++++-----
 1 file changed, 53 insertions(+), 11 deletions(-)

diff --git a/Plugins/LdapPlugin/Ldap/LdapServer.cs b/Plugins/LdapPlugin/Ldap/LdapServer.cs
index a8525428..76b8452f 100755
--- a/Plugins/LdapPlugin/Ldap/LdapServer.cs
+++ b/Plugins/LdapPlugin/Ldap/LdapServer.cs
@@ -30,6 +30,7 @@ DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR
 using System.Text;
 using System.Reflection;
 using System.DirectoryServices.Protocols;
+using System.Security.Cryptography;
 using System.Security.Cryptography.X509Certificates;
 using System.Net;
 using System.IdentityModel.Selectors;
@@ -153,6 +154,46 @@ private void Connect()
             }
         }
 
+        /// <summary>
+        /// Check that certificate CN or SubjectAltName matches LDAP server name
+        /// </summary>
+        /// <param name="conn">The LDAP connection.</param>
+        /// <param name="cert">The server's certificate</param>
+        /// <returns>true if verification succeeds, false otherwise.</returns>
+        private bool VerifyCertName(LdapConnection conn, X509Certificate2 cert)
+        {
+            string name = conn.SessionOptions.HostName;
+            string[] dnparts = cert.SubjectName.Name.Split(',');
+            foreach (var dnpart in dnparts)
+            {
+                int i = dnpart.IndexOf("=");
+                if (i > 0 && dnpart.Substring(0, i) == "CN")
+                    if (dnpart.Substring(i + 1) == name)
+                        return true;
+            }
+
+            foreach (X509Extension extension in cert.Extensions)
+            {
+                if (extension.Oid.Value == "2.5.29.17")
+                {
+                    AsnEncodedData asndata = new AsnEncodedData(extension.Oid, extension.RawData);
+                    string san = asndata.Format(true);
+                    String[] sans = san.Split('\r');
+                    foreach (var s in sans)
+                    {
+                        int i = s.IndexOf("=");
+                        if (i > 0)
+                            {
+                                string r = s.Substring(i + 1);
+                                if (r == name)
+                                    return true;
+                            }
+                    }
+                }
+            }
+            return false;
+        }
+
         /// <summary>
         /// This is the verify certificate callback method used when initially binding to the
         /// LDAP server.  This manages all certificate validation.
@@ -180,19 +221,20 @@ private bool VerifyCert(LdapConnection conn, X509Certificate cert)
             {
                 m_logger.Debug("Verifying server cert with Windows store.");
 
-                // We set the RevocationMode to NoCheck because most custom (self-generated) CAs
-                // do not work properly with revocation lists.  This is slightly less secure, but
-                // the most common use case for this plugin probably doesn't rely on revocation
-                // lists.
-                X509ChainPolicy policy = new X509ChainPolicy() {
-                    RevocationMode = X509RevocationMode.NoCheck
-                };
-
-                // Create a validator using the policy
-                X509CertificateValidator validator = X509CertificateValidator.CreatePeerOrChainTrustValidator(true, policy);
                 try
                 {
-                    validator.Validate(serverCert);
+                    if (!serverCert.Verify())
+                    {
+                        m_logger.Debug("Server certificate verification failed.");
+                        return false;
+                    }
+
+                    // Check that certificate name match server name
+                    if (!VerifyCertName(conn, serverCert))
+                    {
+                        m_logger.Debug("Server certificate does not match hostname.");
+                        return false;
+                    }
 
                     // If we get here, validation succeeded.
                     m_logger.Debug("Server certificate verification succeeded.");