From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
There is some commonality between the code for sanity checking
certs when initializing libvirt and the code for validating
certs during a live TLS session handshake. This patchset splits
up the sanity checking function into several smaller functions
each doing a specific type of check. The cert validation code
is then updated to also call into these functions
* src/rpc/virnettlscontext.c: Refactor cert validation code
---
src/rpc/virnettlscontext.c | 466 +++++++++++++++++++++++++++-----------------
1 files changed, 283 insertions(+), 183 deletions(-)
diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c
index 7d271bc..37b55ad 100644
--- a/src/rpc/virnettlscontext.c
+++ b/src/rpc/virnettlscontext.c
@@ -97,68 +97,51 @@ static void virNetTLSLog(int level, const char *str) {
}
-static gnutls_x509_crt_t virNetTLSContextSanityCheckCert(bool isServer,
- bool isCA,
- const char *certFile)
+static int virNetTLSContextCheckCertTimes(gnutls_x509_crt_t cert,
+ const char *certFile,
+ bool isServer,
+ bool isCA)
{
- gnutls_datum_t data;
- gnutls_x509_crt_t cert = NULL;
- char *buf = NULL;
- int ret = -1;
time_t now;
- int status;
- int i;
- char *buffer = NULL;
- size_t size;
- unsigned int usage;
- unsigned int critical;
- bool allowClient = false, allowServer = false;
-
- VIR_DEBUG("isServer %d isCA %d certFile %s",
- isServer, isCA, certFile);
if ((now = time(NULL)) == ((time_t)-1)) {
virReportSystemError(errno, "%s",
_("cannot get current time"));
- goto cleanup;
- }
-
- if (gnutls_x509_crt_init(&cert) < 0) {
- virNetError(VIR_ERR_SYSTEM_ERROR, "%s",
- _("Unable to initialize certificate"));
- goto cleanup;
- }
-
- if (virFileReadAll(certFile, (1<<16), &buf) < 0)
- goto cleanup;
-
- data.data = (unsigned char *)buf;
- data.size = strlen(buf);
-
- if (gnutls_x509_crt_import(cert, &data, GNUTLS_X509_FMT_PEM) < 0) {
- virNetError(VIR_ERR_SYSTEM_ERROR, isServer ?
- _("Unable to import server certificate %s") :
- _("Unable to import client certificate %s"),
- certFile);
- goto cleanup;
+ return -1;
}
if (gnutls_x509_crt_get_expiration_time(cert) < now) {
- virNetError(VIR_ERR_SYSTEM_ERROR, isServer ?
- _("The server certificate %s has expired") :
- _("The client certificate %s has expired"),
+ virNetError(VIR_ERR_SYSTEM_ERROR,
+ (isCA ?
+ _("The CA certificate %s has expired") :
+ (isServer ?
+ _("The server certificate %s has expired") :
+ _("The client certificate %s has expired"))),
certFile);
- goto cleanup;
+ return -1;
}
if (gnutls_x509_crt_get_activation_time(cert) > now) {
- virNetError(VIR_ERR_SYSTEM_ERROR, isServer ?
- _("The server certificate %s is not yet active") :
- _("The client certificate %s is not yet active"),
+ virNetError(VIR_ERR_SYSTEM_ERROR,
+ (isCA ?
+ _("The CA certificate %s is not yet active") :
+ (isServer ?
+ _("The server certificate %s is not yet active") :
+ _("The client certificate %s is not yet active"))),
certFile);
- goto cleanup;
+ return -1;
}
+ return 0;
+}
+
+static int virNetTLSContextCheckCertBasicConstraints(gnutls_x509_crt_t cert,
+ const char *certFile,
+ bool isServer,
+ bool isCA)
+{
+ int status;
+
status = gnutls_x509_crt_get_basic_constraints(cert, NULL, NULL, NULL);
VIR_DEBUG("Cert %s basic constraints %d", certFile, status);
@@ -168,29 +151,40 @@ static gnutls_x509_crt_t virNetTLSContextSanityCheckCert(bool isServer,
_("The certificate %s basic constraints show a CA, but we need one for a server") :
_("The certificate %s basic constraints show a CA, but we need one for a client"),
certFile);
- goto cleanup;
+ return -1;
}
} else if (status == 0) { /* It is not a CA cert */
if (isCA) {
virNetError(VIR_ERR_SYSTEM_ERROR,
_("The certificate %s basic constraints do not show a CA"),
certFile);
- goto cleanup;
+ return -1;
}
} else if (status == GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE) { /* Missing basicConstraints */
if (isCA) {
virNetError(VIR_ERR_SYSTEM_ERROR,
_("The certificate %s is missing basic constraints for a CA"),
certFile);
- goto cleanup;
+ return -1;
}
} else { /* General error */
virNetError(VIR_ERR_SYSTEM_ERROR,
_("Unable to query certificate %s basic constraints %s"),
certFile, gnutls_strerror(status));
- goto cleanup;
+ return -1;
}
+ return 0;
+}
+
+static int virNetTLSContextCheckCertKeyUsage(gnutls_x509_crt_t cert,
+ const char *certFile,
+ bool isCA)
+{
+ int status;
+ unsigned int usage;
+ unsigned int critical;
+
status = gnutls_x509_crt_get_key_usage(cert, &usage, &critical);
VIR_DEBUG("Cert %s key usage status %d usage %d critical %u", certFile, status, usage, critical);
@@ -202,7 +196,7 @@ static gnutls_x509_crt_t virNetTLSContextSanityCheckCert(bool isServer,
virNetError(VIR_ERR_SYSTEM_ERROR,
_("Unable to query certificate %s key usage %s"),
certFile, gnutls_strerror(status));
- goto cleanup;
+ return -1;
}
}
@@ -212,7 +206,7 @@ static gnutls_x509_crt_t virNetTLSContextSanityCheckCert(bool isServer,
virNetError(VIR_ERR_SYSTEM_ERROR,
_("Certificate %s usage does not permit certificate signing"),
certFile);
- goto cleanup;
+ return -1;
} else {
VIR_WARN("Certificate %s usage does not permit certificate signing",
certFile);
@@ -224,7 +218,7 @@ static gnutls_x509_crt_t virNetTLSContextSanityCheckCert(bool isServer,
virNetError(VIR_ERR_SYSTEM_ERROR,
_("Certificate %s usage does not permit digital signature"),
certFile);
- goto cleanup;
+ return -1;
} else {
VIR_WARN("Certificate %s usage does not permit digital signature",
certFile);
@@ -235,7 +229,7 @@ static gnutls_x509_crt_t virNetTLSContextSanityCheckCert(bool isServer,
virNetError(VIR_ERR_SYSTEM_ERROR,
_("Certificate %s usage does not permit key encipherment"),
certFile);
- goto cleanup;
+ return -1;
} else {
VIR_WARN("Certificate %s usage does not permit key encipherment",
certFile);
@@ -243,10 +237,25 @@ static gnutls_x509_crt_t virNetTLSContextSanityCheckCert(bool isServer,
}
}
+ return 0;
+}
+
+
+static int virNetTLSContextCheckCertKeyPurpose(gnutls_x509_crt_t cert,
+ const char *certFile,
+ bool isServer)
+{
+ int status;
+ int i;
+ unsigned int purposeCritical;
+ unsigned int critical;
+ char *buffer;
+ size_t size;
+ bool allowClient = false, allowServer = false;
+
critical = 0;
for (i = 0 ; ; i++) {
size = 0;
- unsigned int purposeCritical;
status = gnutls_x509_crt_get_key_purpose_oid(cert, i, buffer, &size, NULL);
if (status == GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE) {
@@ -261,20 +270,21 @@ static gnutls_x509_crt_t virNetTLSContextSanityCheckCert(bool isServer,
virNetError(VIR_ERR_SYSTEM_ERROR,
_("Unable to query certificate %s key purpose %s"),
certFile, gnutls_strerror(status));
- goto cleanup;
+ return -1;
}
if (VIR_ALLOC_N(buffer, size) < 0) {
virReportOOMError();
- goto cleanup;
+ return -1;
}
status = gnutls_x509_crt_get_key_purpose_oid(cert, i, buffer, &size, &purposeCritical);
if (status < 0) {
+ VIR_FREE(buffer);
virNetError(VIR_ERR_SYSTEM_ERROR,
_("Unable to query certificate %s key purpose %s"),
certFile, gnutls_strerror(status));
- goto cleanup;
+ return -1;
}
if (purposeCritical)
critical = true;
@@ -291,24 +301,25 @@ static gnutls_x509_crt_t virNetTLSContextSanityCheckCert(bool isServer,
VIR_FREE(buffer);
}
- if (!isCA) { /* No purpose checks required for CA certs */
- if (isServer && !allowServer) {
+ if (isServer) {
+ if (!allowServer) {
if (critical) {
virNetError(VIR_ERR_SYSTEM_ERROR,
_("Certificate %s purpose does not allow use for with a TLS server"),
certFile);
- goto cleanup;
+ return -1;
} else {
VIR_WARN("Certificate %s purpose does not allow use for with a TLS server",
certFile);
}
}
- if (!isServer && !allowClient) {
+ } else {
+ if (!allowClient) {
if (critical) {
virNetError(VIR_ERR_SYSTEM_ERROR,
_("Certificate %s purpose does not allow use for with a TLS client"),
certFile);
- goto cleanup;
+ return -1;
} else {
VIR_WARN("Certificate %s purpose does not allow use for with a TLS client",
certFile);
@@ -316,6 +327,181 @@ static gnutls_x509_crt_t virNetTLSContextSanityCheckCert(bool isServer,
}
}
+ return 0;
+}
+
+/* Check DN is on tls_allowed_dn_list. */
+static int
+virNetTLSContextCheckCertDNWhitelist(const char *dname,
+ const char *const*wildcards)
+{
+ while (*wildcards) {
+ int ret = fnmatch (*wildcards, dname, 0);
+ if (ret == 0) /* Succesful match */
+ return 1;
+ if (ret != FNM_NOMATCH) {
+ virNetError(VIR_ERR_INTERNAL_ERROR,
+ _("Malformed TLS whitelist regular expression '%s'"),
+ *wildcards);
+ return -1;
+ }
+
+ wildcards++;
+ }
+
+ /* Log the client's DN for debugging */
+ VIR_DEBUG("Failed whitelist check for client DN '%s'", dname);
+
+ /* This is the most common error: make it informative. */
+ virNetError(VIR_ERR_SYSTEM_ERROR, "%s",
+ _("Client's Distinguished Name is not on the list "
+ "of allowed clients (tls_allowed_dn_list). Use "
+ "'certtool -i --infile clientcert.pem' to view the"
+ "Distinguished Name field in the client certificate,"
+ "or run this daemon with --verbose option."));
+ return 0;
+}
+
+
+static int
+virNetTLSContextCheckCertDN(gnutls_x509_crt_t cert,
+ const char *certFile,
+ const char *hostname,
+ const char *const* whitelist)
+{
+ int ret;
+ char name[256];
+ size_t namesize = sizeof name;
+
+ memset(name, 0, namesize);
+
+ ret = gnutls_x509_crt_get_dn(cert, name, &namesize);
+ if (ret != 0) {
+ virNetError(VIR_ERR_SYSTEM_ERROR,
+ _("Failed to get certificate %s distinguished name: %s"),
+ certFile, gnutls_strerror(ret));
+ return -1;
+ }
+
+ if (whitelist &&
+ virNetTLSContextCheckCertDNWhitelist(name, whitelist) <= 0)
+ return -1;
+
+ if (hostname &&
+ !gnutls_x509_crt_check_hostname(cert, hostname)) {
+ virNetError(VIR_ERR_RPC,
+ _("Certificate %s owner does not match the hostname %s"),
+ certFile, hostname);
+ return -1;
+ }
+
+ return 0;
+}
+
+
+static int virNetTLSContextCheckCert(gnutls_x509_crt_t cert,
+ const char *certFile,
+ bool isServer,
+ bool isCA)
+{
+ if (virNetTLSContextCheckCertTimes(cert, certFile,
+ isServer, isCA) < 0)
+ return -1;
+
+ if (virNetTLSContextCheckCertBasicConstraints(cert, certFile,
+ isServer, isCA) < 0)
+ return -1;
+
+ if (virNetTLSContextCheckCertKeyUsage(cert, certFile,
+ isCA) < 0)
+ return -1;
+
+ if (!isCA &&
+ virNetTLSContextCheckCertKeyPurpose(cert, certFile,
+ isServer) < 0)
+ return -1;
+
+ return 0;
+}
+
+
+static int virNetTLSContextCheckCertPair(gnutls_x509_crt_t cert,
+ const char *certFile,
+ gnutls_x509_crt_t cacert,
+ const char *cacertFile,
+ bool isServer)
+{
+ unsigned int status;
+
+ if (gnutls_x509_crt_list_verify(&cert, 1,
+ &cacert, 1,
+ NULL, 0,
+ 0, &status) < 0) {
+ virNetError(VIR_ERR_SYSTEM_ERROR, isServer ?
+ _("Unable to verify server certificate %s against CA certificate %s") :
+ _("Unable to verify client certificate %s against CA certificate %s"),
+ certFile, cacertFile);
+ return -1;
+ }
+
+ if (status != 0) {
+ const char *reason = _("Invalid certificate");
+
+ if (status & GNUTLS_CERT_INVALID)
+ reason = _("The certificate is not trusted.");
+
+ if (status & GNUTLS_CERT_SIGNER_NOT_FOUND)
+ reason = _("The certificate hasn't got a known issuer.");
+
+ if (status & GNUTLS_CERT_REVOKED)
+ reason = _("The certificate has been revoked.");
+
+#ifndef GNUTLS_1_0_COMPAT
+ if (status & GNUTLS_CERT_INSECURE_ALGORITHM)
+ reason = _("The certificate uses an insecure algorithm");
+#endif
+
+ virNetError(VIR_ERR_SYSTEM_ERROR,
+ _("Our own certificate %s failed validation against %s: %s"),
+ certFile, cacertFile, reason);
+ return -1;
+ }
+
+ return 0;
+}
+
+
+static gnutls_x509_crt_t virNetTLSContextLoadCertFromFile(const char *certFile,
+ bool isServer,
+ bool isCA)
+{
+ gnutls_datum_t data;
+ gnutls_x509_crt_t cert = NULL;
+ char *buf = NULL;
+ int ret = -1;
+
+ VIR_DEBUG("isServer %d isCA %d certFile %s",
+ isServer, isCA, certFile);
+
+ if (gnutls_x509_crt_init(&cert) < 0) {
+ virNetError(VIR_ERR_SYSTEM_ERROR, "%s",
+ _("Unable to initialize certificate"));
+ goto cleanup;
+ }
+
+ if (virFileReadAll(certFile, (1<<16), &buf) < 0)
+ goto cleanup;
+
+ data.data = (unsigned char *)buf;
+ data.size = strlen(buf);
+
+ if (gnutls_x509_crt_import(cert, &data, GNUTLS_X509_FMT_PEM) < 0) {
+ virNetError(VIR_ERR_SYSTEM_ERROR, isServer ?
+ _("Unable to import server certificate %s") :
+ _("Unable to import client certificate %s"),
+ certFile);
+ goto cleanup;
+ }
ret = 0;
@@ -324,7 +510,6 @@ cleanup:
gnutls_x509_crt_deinit(cert);
cert = NULL;
}
- VIR_FREE(buffer);
VIR_FREE(buf);
return cert;
}
@@ -337,51 +522,25 @@ static int virNetTLSContextSanityCheckCredentials(bool isServer,
gnutls_x509_crt_t cert = NULL;
gnutls_x509_crt_t cacert = NULL;
int ret = -1;
- unsigned int status;
- if (access(certFile, R_OK) == 0) {
- if (!(cert = virNetTLSContextSanityCheckCert(isServer, false, certFile)))
- goto cleanup;
- }
- if (access(cacertFile, R_OK) == 0) {
- if (!(cacert = virNetTLSContextSanityCheckCert(isServer, true, cacertFile)))
- goto cleanup;
- }
-
- if (cert && cacert) {
- if (gnutls_x509_crt_list_verify(&cert, 1,
- &cacert, 1,
- NULL, 0,
- 0, &status) < 0) {
- virNetError(VIR_ERR_SYSTEM_ERROR, "%s", isServer ?
- _("Unable to verify server certificate against CA certificate") :
- _("Unable to verify client certificate against CA certificate"));
- goto cleanup;
- }
-
- if (status != 0) {
- const char *reason = _("Invalid certificate");
-
- if (status & GNUTLS_CERT_INVALID)
- reason = _("The certificate is not trusted.");
-
- if (status & GNUTLS_CERT_SIGNER_NOT_FOUND)
- reason = _("The certificate hasn't got a known issuer.");
+ if ((access(certFile, R_OK) == 0) &&
+ !(cert = virNetTLSContextLoadCertFromFile(certFile, isServer, false)))
+ goto cleanup;
+ if ((access(cacertFile, R_OK) == 0) &&
+ !(cacert = virNetTLSContextLoadCertFromFile(cacertFile, isServer, false)))
+ goto cleanup;
- if (status & GNUTLS_CERT_REVOKED)
- reason = _("The certificate has been revoked.");
+ if (cert &&
+ virNetTLSContextCheckCert(cert, certFile, isServer, false) < 0)
+ goto cleanup;
-#ifndef GNUTLS_1_0_COMPAT
- if (status & GNUTLS_CERT_INSECURE_ALGORITHM)
- reason = _("The certificate uses an insecure algorithm");
-#endif
+ if (cacert &&
+ virNetTLSContextCheckCert(cacert, cacertFile, isServer, true) < 0)
+ goto cleanup;
- virNetError(VIR_ERR_SYSTEM_ERROR,
- _("Our own certificate %s failed validation against %s: %s"),
- certFile, cacertFile, reason);
- goto cleanup;
- }
- }
+ if (cert && cacert &&
+ virNetTLSContextCheckCertPair(cert, certFile, cacert, cacertFile, isServer) < 0)
+ goto cleanup;
ret = 0;
@@ -760,45 +919,6 @@ void virNetTLSContextRef(virNetTLSContextPtr ctxt)
}
-/* Check DN is on tls_allowed_dn_list. */
-static int
-virNetTLSContextCheckDN(virNetTLSContextPtr ctxt,
- const char *dname)
-{
- const char *const*wildcards;
-
- /* If the list is not set, allow any DN. */
- wildcards = ctxt->x509dnWhitelist;
- if (!wildcards)
- return 1;
-
- while (*wildcards) {
- int ret = fnmatch (*wildcards, dname, 0);
- if (ret == 0) /* Succesful match */
- return 1;
- if (ret != FNM_NOMATCH) {
- virNetError(VIR_ERR_INTERNAL_ERROR,
- _("Malformed TLS whitelist regular expression '%s'"),
- *wildcards);
- return -1;
- }
-
- wildcards++;
- }
-
- /* Log the client's DN for debugging */
- VIR_DEBUG("Failed whitelist check for client DN '%s'", dname);
-
- /* This is the most common error: make it informative. */
- virNetError(VIR_ERR_SYSTEM_ERROR, "%s",
- _("Client's Distinguished Name is not on the list "
- "of allowed clients (tls_allowed_dn_list). Use "
- "'certtool -i --infile clientcert.pem' to view the"
- "Distinguished Name field in the client certificate,"
- "or run this daemon with --verbose option."));
- return 0;
-}
-
static int virNetTLSContextValidCertificate(virNetTLSContextPtr ctxt,
virNetTLSSessionPtr sess)
{
@@ -806,11 +926,6 @@ static int virNetTLSContextValidCertificate(virNetTLSContextPtr ctxt,
unsigned int status;
const gnutls_datum_t *certs;
unsigned int nCerts, i;
- time_t now;
- char name[256];
- size_t namesize = sizeof name;
-
- memset(name, 0, namesize);
if ((ret = gnutls_certificate_verify_peers2(sess->session, &status)) < 0){
virNetError(VIR_ERR_SYSTEM_ERROR,
@@ -819,12 +934,6 @@ static int virNetTLSContextValidCertificate(virNetTLSContextPtr ctxt,
goto authdeny;
}
- if ((now = time(NULL)) == ((time_t)-1)) {
- virReportSystemError(errno, "%s",
- _("cannot get current time"));
- goto authfail;
- }
-
if (status != 0) {
const char *reason = _("Invalid certificate");
@@ -876,46 +985,37 @@ static int virNetTLSContextValidCertificate(virNetTLSContextPtr ctxt,
goto authfail;
}
- if (gnutls_x509_crt_get_expiration_time(cert) < now) {
- /* Warning is reversed from what you expect, since with
- * this code it is the Server checking the client and
- * vica-versa */
- virNetError(VIR_ERR_SYSTEM_ERROR, "%s", sess->isServer ?
- _("The client certificate has expired") :
- _("The server certificate has expired"));
- gnutls_x509_crt_deinit(cert);
- goto authdeny;
- }
-
- if (gnutls_x509_crt_get_activation_time(cert) > now) {
- /* client/server order reversed. see above */
- virNetError(VIR_ERR_SYSTEM_ERROR, "%s", sess->isServer ?
- _("The client certificate is not yet active") :
- _("The server certificate is not yet active"));
+ if (virNetTLSContextCheckCertTimes(cert, "[session]",
+ sess->isServer, i > 0) < 0) {
gnutls_x509_crt_deinit(cert);
goto authdeny;
}
if (i == 0) {
- ret = gnutls_x509_crt_get_dn(cert, name, &namesize);
- if (ret != 0) {
- virNetError(VIR_ERR_SYSTEM_ERROR,
- _("Failed to get certificate distinguished name: %s"),
- gnutls_strerror(ret));
+ if (virNetTLSContextCheckCertDN(cert, "[session]", sess->hostname,
+ ctxt->x509dnWhitelist) < 0) {
gnutls_x509_crt_deinit(cert);
- goto authfail;
+ goto authdeny;
+ }
+
+ /* !sess->isServer, since on the client, we're validating the
+ * server's cert, and on the server, the client's cert
+ */
+ if (virNetTLSContextCheckCertBasicConstraints(cert, "[session]",
+ !sess->isServer, false) < 0) {
+ gnutls_x509_crt_deinit(cert);
+ goto authdeny;
}
- if (virNetTLSContextCheckDN(ctxt, name) <= 0) {
+ if (virNetTLSContextCheckCertKeyUsage(cert, "[session]",
+ false) < 0) {
gnutls_x509_crt_deinit(cert);
goto authdeny;
}
- if (sess->hostname &&
- !gnutls_x509_crt_check_hostname(cert, sess->hostname)) {
- virNetError(VIR_ERR_RPC,
- _("Certificate's owner does not match the hostname (%s)"),
- sess->hostname);
+ /* !sess->isServer - as above */
+ if (virNetTLSContextCheckCertKeyPurpose(cert, "[session]",
+ !sess->isServer) < 0) {
gnutls_x509_crt_deinit(cert);
goto authdeny;
}
--
1.7.6
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list