During review of https://github.com/freeipa/freeipa/pull/490 we found a couple of potential improvements. Since the purpose of the function are not security relevant but merely sanity checks in the installer, we agreed to handle the improvements in a separate PR.

Here are my comments from the PR + additional comments:

ipalib.x509.match_hostname

1) Should the function name + doc string be changed in order to indicate that the function is not battle hardened but intended for sanity checks?

2) As of RFC 6125 section 6.4.4. you also have to take SRV-ID and URI-ID into account. Since ssl.match_hostname supports IP addresses, you also need to include iPAddress fields or validate that hostname never looks like an IP address.

This will cause a security bug as soon as https://bugs.python.org/issue28196 is fixed.

ipalib.certdb

verify_ca_cert_validity

1) The function lacks a doc string that states its intent. The doc string should make clear that the function is designed and intended for sanity checks in installer and management scripts only. It checks for several obvious problems to give the user a better feedback. It is not a full validation function and should not be used where a false positive can lead to a security issue.

2) RFC 5280 section 4.1.2.6 mandates subject only for certs with basic constraint CA: TRUE. Please check for basic constraint first.

3) How is the verify_ca_cert_validity method used? Is it used to check for just any CA cert or also used to verify that the CA cert can be used to sign an intermediate CA in Dogtag? In the latter case, this should also check that path_length is >= 1.

4) Validation is missing one important field. As of RFC 5280 section 4.2.1.3, CA certs must have a key usage extension with at least keyCertSign bit set.

verify_server_cert_validity

1) Explain purpose of the function, too (see 1. of verify_ca_cert_validity).