From: Daniel P. Berrangé <berrange@redhat.com> A future patch will require fule access checks to be done as part of locating the certificate files, as we will have the ability to load many more files, most of which will be optional. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- po/POTFILES | 1 + src/rpc/virnettlsconfig.c | 168 +++++++++++++++++++++++++++++++++---- src/rpc/virnettlsconfig.h | 37 ++++---- src/rpc/virnettlscontext.c | 161 ++++++++++++++--------------------- 4 files changed, 236 insertions(+), 131 deletions(-) diff --git a/po/POTFILES b/po/POTFILES index 23da794f84..f0aad35c8c 100644 --- a/po/POTFILES +++ b/po/POTFILES @@ -230,6 +230,7 @@ src/rpc/virnetserverservice.c src/rpc/virnetsocket.c src/rpc/virnetsshsession.c src/rpc/virnettlscert.c +src/rpc/virnettlsconfig.c src/rpc/virnettlscontext.c src/secret/secret_driver.c src/security/security_apparmor.c diff --git a/src/rpc/virnettlsconfig.c b/src/rpc/virnettlsconfig.c index ffab3b4fc8..1479eb01ae 100644 --- a/src/rpc/virnettlsconfig.c +++ b/src/rpc/virnettlsconfig.c @@ -21,12 +21,15 @@ #include <config.h> #include "virnettlsconfig.h" +#include "viralloc.h" #include "virlog.h" #include "virutil.h" +#include "virfile.h" +#include "virerror.h" #define VIR_FROM_THIS VIR_FROM_RPC -VIR_LOG_INIT("rpc.nettlscontext"); +VIR_LOG_INIT("rpc.nettlsconfig"); char *virNetTLSConfigUserPKIBaseDir(void) { @@ -142,30 +145,143 @@ void virNetTLSConfigSystemIdentity(bool isServer, key); } -void virNetTLSConfigCustomCreds(const char *pkipath, - bool isServer, - char **cacert, - char **cacrl, - char **cert, - char **key) + +int virNetTLSConfigCheckTrust(const char *cacert, const char *cacrl, + bool *cacertExists, bool *cacrlExists, + bool allowMissingCA) +{ + if (cacertExists) + *cacertExists = true; + if (cacrlExists) + *cacrlExists = true; + VIR_DEBUG("Checking CA certificate '%s' and CRL '%s'", cacert, NULLSTR(cacrl)); + if (!virFileExists(cacert)) { + if (allowMissingCA) { + VIR_DEBUG("CA certificate '%s' does not exist", cacert); + if (cacertExists) + *cacertExists = false; + } else { + virReportSystemError(errno, _("CA certificate '%1$s' does not exist"), + cacert); + return -1; + } + } + if (cacrl != NULL && !virFileExists(cacrl)) { + VIR_DEBUG("CA CRL '%s' does not exist", cacrl); + if (cacrlExists) + *cacrlExists = false; + } + return 0; +} + +static int virNetTLSConfigEnsureTrust(char **cacert, char **cacrl, + bool allowMissingCA) +{ + bool cacertExists, cacrlExists; + + if (virNetTLSConfigCheckTrust(*cacert, *cacrl, + &cacertExists, &cacrlExists, + allowMissingCA) < 0) + return -1; + + if (!cacertExists) + VIR_FREE(*cacert); + if (!cacrlExists) + VIR_FREE(*cacrl); + + return 0; +} + +int virNetTLSConfigCheckIdentity(const char *cert, const char *key, + bool *identityExists, bool allowMissing) +{ + if (identityExists) + *identityExists = true; + VIR_DEBUG("Checking certificate '%s' and key '%s'", cert, key); + if (!virFileExists(cert)) { + int saved_errno = errno; + if (allowMissing) { + if (virFileExists(key)) { + virReportSystemError( + saved_errno, + _("Certificate '%1$s' does not exist, but key '%2$s' does"), + cert, key); + return -1; + } + if (identityExists) + *identityExists = false; + VIR_DEBUG("Missing cert '%s' / key '%s'", cert, key); + return 0; + } else { + virReportSystemError(saved_errno, _("Certificate '%1$s' does not exist"), + cert); + return -1; + } + } else { + if (!virFileExists(key)) { + virReportSystemError(errno, + _("Key '%1$s' does not exist, but certificate '%2$s' does"), + key, cert); + return -1; + } + } + + return 0; +} + + +static int virNetTLSConfigEnsureIdentity(char **cert, char **key, + bool allowMissing) +{ + bool identityExists; + + if (virNetTLSConfigCheckIdentity(*cert, *key, &identityExists, + allowMissing) < 0) + return -1; + + if (!identityExists) { + VIR_FREE(*cert); + VIR_FREE(*key); + } + + return 0; +} + + +int virNetTLSConfigCustomCreds(const char *pkipath, + bool isServer, + char **cacert, + char **cacrl, + char **cert, + char **key) { VIR_DEBUG("Locating creds in custom dir %s", pkipath); virNetTLSConfigTrust(pkipath, pkipath, cacert, cacrl); + + if (virNetTLSConfigEnsureTrust(cacert, cacrl, false) < 0) + return -1; + virNetTLSConfigIdentity(isServer, pkipath, pkipath, cert, key); + + + if (virNetTLSConfigEnsureIdentity(cert, key, !isServer) < 0) + return -1; + + return 0; } -void virNetTLSConfigUserCreds(bool isServer, - char **cacert, - char **cacrl, - char **cert, - char **key) +int virNetTLSConfigUserCreds(bool isServer, + char **cacert, + char **cacrl, + char **cert, + char **key) { g_autofree char *pkipath = virNetTLSConfigUserPKIBaseDir(); @@ -175,18 +291,27 @@ void virNetTLSConfigUserCreds(bool isServer, pkipath, cacert, cacrl); + + if (virNetTLSConfigEnsureTrust(cacert, cacrl, true) < 0) + return -1; + virNetTLSConfigIdentity(isServer, pkipath, pkipath, cert, key); + + if (virNetTLSConfigEnsureIdentity(cert, key, true) < 0) + return -1; + + return 0; } -void virNetTLSConfigSystemCreds(bool isServer, - char **cacert, - char **cacrl, - char **cert, - char **key) +int virNetTLSConfigSystemCreds(bool isServer, + char **cacert, + char **cacrl, + char **cert, + char **key) { VIR_DEBUG("Locating creds in system dir %s", LIBVIRT_PKI_DIR); @@ -194,9 +319,18 @@ void virNetTLSConfigSystemCreds(bool isServer, LIBVIRT_CACRL_DIR, cacert, cacrl); + + if (virNetTLSConfigEnsureTrust(cacert, cacrl, false) < 0) + return -1; + virNetTLSConfigIdentity(isServer, LIBVIRT_CERT_DIR, LIBVIRT_KEY_DIR, cert, key); + + if (virNetTLSConfigEnsureIdentity(cert, key, !isServer) < 0) + return -1; + + return 0; } diff --git a/src/rpc/virnettlsconfig.h b/src/rpc/virnettlsconfig.h index a9378c18b7..9ad213fe06 100644 --- a/src/rpc/virnettlsconfig.h +++ b/src/rpc/virnettlsconfig.h @@ -50,20 +50,25 @@ void virNetTLSConfigSystemIdentity(bool isServer, char **cert, char **key); +int virNetTLSConfigCheckIdentity(const char *cert, const char *key, + bool *identityExists, bool allowMissing); +int virNetTLSConfigCheckTrust(const char *cacert, const char *cacrl, + bool *cacertExists, bool *cacrlExists, + bool allowMissingCA); -void virNetTLSConfigCustomCreds(const char *pkipath, - bool isServer, - char **cacert, - char **cacrl, - char **cert, - char **key); -void virNetTLSConfigUserCreds(bool isServer, - char **cacert, - char **cacrl, - char **cert, - char **key); -void virNetTLSConfigSystemCreds(bool isServer, - char **cacert, - char **cacrl, - char **cert, - char **key); +int virNetTLSConfigCustomCreds(const char *pkipath, + bool isServer, + char **cacert, + char **cacrl, + char **cert, + char **key); +int virNetTLSConfigUserCreds(bool isServer, + char **cacert, + char **cacrl, + char **cert, + char **key); +int virNetTLSConfigSystemCreds(bool isServer, + char **cacert, + char **cacrl, + char **cert, + char **key); diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c index 37f635f47f..7061eb5953 100644 --- a/src/rpc/virnettlscontext.c +++ b/src/rpc/virnettlscontext.c @@ -31,7 +31,6 @@ #include "virnettlscert.h" #include "virstring.h" -#include "viralloc.h" #include "virerror.h" #include "virfile.h" #include "virutil.h" @@ -88,22 +87,6 @@ static int virNetTLSContextOnceInit(void) VIR_ONCE_GLOBAL_INIT(virNetTLSContext); -static int -virNetTLSContextCheckCertFile(const char *type, const char *file, bool allowMissing) -{ - if (!virFileExists(file)) { - if (allowMissing) - return 1; - - virReportSystemError(errno, - _("Cannot read %1$s '%2$s'"), - type, file); - return -1; - } - return 0; -} - - static void virNetTLSLog(int level G_GNUC_UNUSED, const char *str G_GNUC_UNUSED) { @@ -112,7 +95,6 @@ static void virNetTLSLog(int level G_GNUC_UNUSED, static int virNetTLSContextLoadCredentials(virNetTLSContext *ctxt, - bool isServer, const char *cacert, const char *cacrl, const char *const *certs, @@ -121,66 +103,42 @@ static int virNetTLSContextLoadCredentials(virNetTLSContext *ctxt, int err; size_t i; - if (cacert && cacert[0] != '\0') { - if (virNetTLSContextCheckCertFile("CA certificate", cacert, false) < 0) - return -1; + VIR_DEBUG("loading CA cert from %s", cacert); + err = gnutls_certificate_set_x509_trust_file(ctxt->x509cred, + cacert, + GNUTLS_X509_FMT_PEM); + if (err < 0) { + virReportError(VIR_ERR_SYSTEM_ERROR, + _("Unable to set x509 CA certificate: %1$s: %2$s"), + cacert, gnutls_strerror(err)); + return -1; + } - VIR_DEBUG("loading CA cert from %s", cacert); - err = gnutls_certificate_set_x509_trust_file(ctxt->x509cred, - cacert, - GNUTLS_X509_FMT_PEM); + if (cacrl) { + VIR_DEBUG("loading CRL from %s", cacrl); + err = gnutls_certificate_set_x509_crl_file(ctxt->x509cred, + cacrl, + GNUTLS_X509_FMT_PEM); if (err < 0) { virReportError(VIR_ERR_SYSTEM_ERROR, - _("Unable to set x509 CA certificate: %1$s: %2$s"), - cacert, gnutls_strerror(err)); - return -1; - } - } - - if (cacrl && cacrl[0] != '\0') { - int rv; - if ((rv = virNetTLSContextCheckCertFile("CA revocation list", cacrl, true)) < 0) + _("Unable to set x509 certificate revocation list: %1$s: %2$s"), + cacrl, gnutls_strerror(err)); return -1; - - if (rv == 0) { - VIR_DEBUG("loading CRL from %s", cacrl); - err = gnutls_certificate_set_x509_crl_file(ctxt->x509cred, - cacrl, - GNUTLS_X509_FMT_PEM); - if (err < 0) { - virReportError(VIR_ERR_SYSTEM_ERROR, - _("Unable to set x509 certificate revocation list: %1$s: %2$s"), - cacrl, gnutls_strerror(err)); - return -1; - } - } else { - VIR_DEBUG("Skipping non-existent CA CRL %s", cacrl); } + } else { + VIR_DEBUG("no CRL file to load"); } for (i = 0; certs[i] != NULL && keys[i] != NULL; i++) { - int rv; - if ((rv = virNetTLSContextCheckCertFile("certificate", certs[i], !isServer)) < 0) - return -1; - if (rv == 0 && - (rv = virNetTLSContextCheckCertFile("private key", keys[i], !isServer)) < 0) + VIR_DEBUG("loading cert and key from %s and %s", certs[i], keys[i]); + err = gnutls_certificate_set_x509_key_file(ctxt->x509cred, + certs[i], keys[i], + GNUTLS_X509_FMT_PEM); + if (err < 0) { + virReportError(VIR_ERR_SYSTEM_ERROR, + _("Unable to set x509 key and certificate: %1$s, %2$s: %3$s"), + keys[i], certs[i], gnutls_strerror(err)); return -1; - - if (rv == 0) { - VIR_DEBUG("loading cert and key from %s and %s", certs[i], keys[i]); - err = - gnutls_certificate_set_x509_key_file(ctxt->x509cred, - certs[i], keys[i], - GNUTLS_X509_FMT_PEM); - if (err < 0) { - virReportError(VIR_ERR_SYSTEM_ERROR, - _("Unable to set x509 key and certificate: %1$s, %2$s: %3$s"), - keys[i], certs[i], gnutls_strerror(err)); - return -1; - } - } else { - VIR_DEBUG("Skipping non-existent cert %s key %s on client", - certs[i], keys[i]); } } @@ -200,6 +158,15 @@ static virNetTLSContext *virNetTLSContextNew(const char *cacert, { virNetTLSContext *ctxt; int err; + g_autofree char *certlist = certs ? g_strjoinv(", ", (char **)certs) : NULL; + g_autofree char *keylist = keys ? g_strjoinv(", ", (char **)keys) : NULL; + g_autofree char *acllist = x509dnACL ? g_strjoinv(", ", (char **)x509dnACL) : NULL; + + VIR_DEBUG("CA cert=%s CRL=%s certs='%s' keys='%s' ACL='%s' " + "priority=%s sanity-check=%d require-valid=%d is-server=%d", + cacert, NULLSTR(cacrl), NULLSTR(certlist), NULLSTR(keylist), + NULLSTR(acllist), priority, sanityCheckCert, requireValidCert, + isServer); if (virNetTLSContextInitialize() < 0) return NULL; @@ -228,7 +195,7 @@ static virNetTLSContext *virNetTLSContextNew(const char *cacert, virNetTLSCertSanityCheck(isServer, cacert, certs) < 0) goto error; - if (virNetTLSContextLoadCredentials(ctxt, isServer, cacert, cacrl, + if (virNetTLSContextLoadCredentials(ctxt, cacert, cacrl, certs, keys) < 0) goto error; @@ -268,38 +235,22 @@ static int virNetTLSContextLocateCredentials(const char *pkipath, * files actually exist there */ if (pkipath) { - virNetTLSConfigCustomCreds(pkipath, isServer, - cacert, cacrl, - cert, key); + if (virNetTLSConfigCustomCreds(pkipath, isServer, + cacert, cacrl, + cert, key) < 0) + return -1; } else { - if (tryUserPkiPath) { + if (tryUserPkiPath && virNetTLSConfigUserCreds(isServer, cacert, cacrl, - cert, key); - - /* - * If some of the files can't be found, fallback - * to the global location for them - */ - if (!virFileExists(*cacert)) - VIR_FREE(*cacert); - if (!virFileExists(*cacrl)) - VIR_FREE(*cacrl); - - /* Check these as a pair, since it they are - * mutually dependent - */ - if (!virFileExists(*key) || !virFileExists(*cert)) { - VIR_FREE(*key); - VIR_FREE(*cert); - } - } + cert, key) < 0) + return -1; - virNetTLSConfigSystemCreds(isServer, - cacert, cacrl, - cert, key); + if (virNetTLSConfigSystemCreds(isServer, + cacert, cacrl, + cert, key) < 0) + return -1; } - return 0; } @@ -359,6 +310,13 @@ virNetTLSContext *virNetTLSContextNewServer(const char *cacert, bool sanityCheckCert, bool requireValidCert) { + size_t i; + if (virNetTLSConfigCheckTrust(cacert, cacrl, NULL, NULL, false) < 0) + return NULL; + for (i = 0; certs[i] != NULL && keys[i] != NULL; i++) { + if (virNetTLSConfigCheckIdentity(certs[i], keys[i], NULL, false) < 0) + return NULL; + } return virNetTLSContextNew(cacert, cacrl, certs, keys, x509dnACL, priority, sanityCheckCert, requireValidCert, true); } @@ -393,7 +351,7 @@ int virNetTLSContextReloadForServer(virNetTLSContext *ctxt, if (virNetTLSCertSanityCheck(true, cacert, certs)) goto error; - if (virNetTLSContextLoadCredentials(ctxt, true, cacert, cacrl, + if (virNetTLSContextLoadCredentials(ctxt, cacert, cacrl, certs, keys)) goto error; @@ -417,6 +375,13 @@ virNetTLSContext *virNetTLSContextNewClient(const char *cacert, bool sanityCheckCert, bool requireValidCert) { + size_t i; + if (virNetTLSConfigCheckTrust(cacert, cacrl, NULL, NULL, false) < 0) + return NULL; + for (i = 0; certs[i] != NULL && keys[i] != NULL; i++) { + if (virNetTLSConfigCheckIdentity(certs[i], keys[i], NULL, false) < 0) + return NULL; + } return virNetTLSContextNew(cacert, cacrl, certs, keys, NULL, priority, sanityCheckCert, requireValidCert, false); } -- 2.51.1