Fix certificate validation

[manu0401]

• 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, this setup is quite
  easy to achieve.

[MutonUfoAI]

You do verify the Ldap host-name against the certificate DN, but that proofs nothing.
I can create any cert with an appropriate DN and a different issuer.

I would go this way:
add this code in VerifyCert(LdapConnection conn, X509Certificate cert)

        try
        {
            X509Store certStore = new X509Store(StoreLocation.LocalMachine);
            certStore.Open(OpenFlags.ReadOnly);
            if (certStore.Certificates.Contains(cert))
                return true;
            else
                return false;
        }
        catch (Exception e)
        {
            m_logger.ErrorFormat("Server certificate validation exception: {0}", e.Message);
            return false;
        }

and add your ldap-cert in the local-machine cert-store under personal.
or add your ldap-cert into pgina as a file and point the config to it.

[manu0401]

You do verify the Ldap host-name against the certificate DN, but that proofs nothing.
I can create any cert with an appropriate DN and a different issuer.

Indeed an attacker can make up its own CA, and issue a certificate with a CN that matches. But that CA will not be in the trusted certificate store, and hence that rogue certificate will not be validated. I tested this scenario with the code I submitted and it behaves correctly.

Or do you suggest an attacked could convince a trusted CA to sign a certificate for a CN in someone else's domain? Indeed this is a possible attack, but a quite rare one, as it is precisely CA's business to take care of what certificate they sign.

You suggest adding the LDAP server certificate to the store, but that does not scales very well if there are many clients, and multiple LDAP servers for which certificates changes on expiration time. This approach means constantly updating the stores of many clients, CA were created to avoid that kind of trouble.

[MutonUfoAI]

The problem with all that stolen and unverified certs is, it has already happened in the past.

You are right there is always a downside to each approach, but the code must work for everyone.
There is a problem at
65b7ea1#diff-24c5c333840025a0cb10a81520b8e96aR165
You can receive IPs or alias addresses which will not match your cert, and even worse the DNS class is not able to return aliases at all.
For testing I've used microsoft.com cert ms.zip, you can have a asterisk sign as DNS addresses too.
The easiest way would be to force admins not to use ip or alias as ldap hosts if they want to verify ...

    public static bool VerifyCertName(LdapConnection conn, X509Certificate2 cert)
    {
        List<string> HostName = new List<string>(){conn.SessionOptions.HostName};

        try
        {
            IPHostEntry hostFQDN = Dns.GetHostEntry(conn.SessionOptions.HostName);
            hostFQDN.Aliases.ToList().ForEach(x => HostName.Add(x));
            hostFQDN.AddressList.ToList().ForEach(x => HostName.Add(x.ToString()));
        }
        catch (Exception ex)
        {
            //m_logger.ErrorFormat("can't resolve FQDN of {0}:{1}", server[0], ex.Message);
        }
        Console.WriteLine(string.Join(Environment.NewLine, HostName));

        string certCN = "";
        string name = HostName.First();
        Console.WriteLine(cert.SubjectName.Decode(X500DistinguishedNameFlags.DoNotUsePlusSign | X500DistinguishedNameFlags.DoNotUseQuotes | X500DistinguishedNameFlags.UseNewLines | X500DistinguishedNameFlags.UseUTF8Encoding));
        string[] str = cert.SubjectName.Decode(X500DistinguishedNameFlags.DoNotUsePlusSign | X500DistinguishedNameFlags.DoNotUseQuotes | X500DistinguishedNameFlags.UseNewLines | X500DistinguishedNameFlags.UseUTF8Encoding).Trim('\r').Split('\n');
        for (int x = 0; x < str.Length; x++)
        {
            if (str[x].StartsWith("CN="))
                certCN = str[x].Replace("CN=", "").Trim();
        }
        Console.WriteLine("[" + certCN + "]");
        certCN = certCN.Replace(".", @"\.").Replace("*", ".*");
        Console.WriteLine("[" + certCN + "]");
        Console.WriteLine(Regex.IsMatch(name, certCN));
        if (Regex.IsMatch(name, certCN))
        {
            //return true;
        }

        List<string> certSAN = new List<string>();
        foreach (X509Extension extension in cert.Extensions)
        {
            AsnEncodedData asndata = new AsnEncodedData(extension.Oid, extension.RawData);
            string[] adata = asndata.Format(true).Trim('\r').Split('\n');
            for (int x = 0; x < adata.Length; x++)
            {
                if (adata[x].StartsWith("DNS-Name="))
                {
                    string SANregex = adata[x].Replace("DNS-Name=", "").Trim();
                    SANregex = SANregex.Replace(".", @"\.").Replace("*", ".*");
                    certSAN.Add(SANregex);
                }
            }
        }
        foreach (string csan in certSAN)
        {
            Console.WriteLine("[" + csan + "]");
            if (Regex.IsMatch(name, csan))
            {
                return true;
            }
        }

        return false;
    }

    static void Main(string[] args)
    {
        LdapDirectoryIdentifier m_serverIdentifier = new LdapDirectoryIdentifier("microsoft.com", 636, false, false);
        LdapConnection m_conn = new LdapConnection(m_serverIdentifier);
        m_conn.SessionOptions.HostName = "hfdhfd.hfdhfd.azure.biz";//"microsoft.com";
        X509Certificate2 cert = new X509Certificate2(@"C:\where\your\cert\is\ms.crt");
        Console.WriteLine(VerifyCertName(m_conn, cert));
    }

[manu0401]

Indeed there have been stolen certs, but the CA model is still considered robust enough for online commerce and banking.

But that is not the problem here, since my change only tighten certificate validation. I agree software must work for any case, and that tighten checks may annoy someone else., Hence what about adding a configuration option for the extra checks? Something along "check certificate name matches LDAP hostname"

I agree I overlooked the wildcard case. I can work on it if that is the only thing that restrains you from merging the change.

[MutonUfoAI]

Your approach is appropriate. The option is called verification....
I'll do the rest.