On Wed, Jul 20, 2011 at 02:12:46PM +0100, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
>
> If key usage or purpose data is not present in the cert, the
> RFC recommends that access be allowed. Also fix checking of
> key usage to include requirements for client/server certs,
> and fix key purpose checking to treat data as a list of bits
> ---
> src/rpc/virnettlscontext.c | 78 +++++++++++++++++++++++++------------------
> 1 files changed, 45 insertions(+), 33 deletions(-)
>
> diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c
> index 402029f..1ee5ab2 100644
> --- a/src/rpc/virnettlscontext.c
> +++ b/src/rpc/virnettlscontext.c
> @@ -111,6 +111,7 @@ static gnutls_x509_crt_t virNetTLSContextSanityCheckCert(bool isServer,
> char *buffer = NULL;
> size_t size;
> unsigned int usage;
> + bool allowClient = false, allowServer = false;
>
> VIR_DEBUG("isServer %d isCA %d certFile %s",
> isServer, isCA, certFile);
> @@ -193,24 +194,34 @@ static gnutls_x509_crt_t virNetTLSContextSanityCheckCert(bool isServer,
>
> VIR_DEBUG("Cert %s key usage status %d usage %d", certFile, status, usage);
> if (status < 0) {
> - virNetError(VIR_ERR_SYSTEM_ERROR,
> - _("Unable to query certificate %s key usage %s"),
> - certFile, gnutls_strerror(status));
> - goto cleanup;
> + if (status == GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE) {
> + usage = isCA ? GNUTLS_KEY_KEY_CERT_SIGN :
> + GNUTLS_KEY_DIGITAL_SIGNATURE|GNUTLS_KEY_KEY_ENCIPHERMENT;
> + } else {
> + virNetError(VIR_ERR_SYSTEM_ERROR,
> + _("Unable to query certificate %s key usage %s"),
> + certFile, gnutls_strerror(status));
> + goto cleanup;
> + }
> }
>
> - if (usage & GNUTLS_KEY_KEY_CERT_SIGN) {
> - if (!isCA) {
> - virNetError(VIR_ERR_SYSTEM_ERROR, isServer ?
> - _("Certificate %s usage is for certificate signing, but wanted a server certificate") :
> - _("Certificate %s usage is for certificate signing, but wanted a client certificate"),
> + if (isCA) {
> + if (!(usage & GNUTLS_KEY_KEY_CERT_SIGN)) {
> + virNetError(VIR_ERR_SYSTEM_ERROR,
> + _("Certificate %s usage does not permit certificate signing"),
> certFile);
> goto cleanup;
> }
> } else {
> - if (isCA) {
> + if (!(usage & GNUTLS_KEY_DIGITAL_SIGNATURE)) {
> + virNetError(VIR_ERR_SYSTEM_ERROR,
> + _("Certificate %s usage does not permit digital signature"),
> + certFile);
> + goto cleanup;
> + }
> + if (!(usage & GNUTLS_KEY_KEY_ENCIPHERMENT)) {
> virNetError(VIR_ERR_SYSTEM_ERROR,
> - _("Certificate %s usage is for not certificate signing"),
> + _("Certificate %s usage does not permit key encipherment"),
> certFile);
> goto cleanup;
> }
> @@ -221,7 +232,11 @@ static gnutls_x509_crt_t virNetTLSContextSanityCheckCert(bool isServer,
> status = gnutls_x509_crt_get_key_purpose_oid(cert, i, buffer, &size, NULL);
>
> if (status == GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE) {
> - VIR_DEBUG("No key purpose data available, skipping checks");
> + VIR_DEBUG("No key purpose data available at slot %d", i);
> +
> + /* If there is no data at all, then we must allow client/server to pass */
> + if (i == 0)
> + allowServer = allowClient = true;
> break;
> }
> if (status != GNUTLS_E_SHORT_MEMORY_BUFFER) {
> @@ -246,34 +261,31 @@ static gnutls_x509_crt_t virNetTLSContextSanityCheckCert(bool isServer,
>
> VIR_DEBUG("Key purpose %d %s", status, buffer);
> if (STREQ(buffer, GNUTLS_KP_TLS_WWW_SERVER)) {
> - if (isCA || !isServer) {
> - virNetError(VIR_ERR_SYSTEM_ERROR, isCA ?
> - _("Certificate %s purpose is TLS server, but wanted a CA certificate") :
> - _("Certificate %s client purpose is TLS server, but wanted a TLS client certificate"),
> - certFile);
> - goto cleanup;
> - }
> + allowServer = true;
> } else if (STREQ(buffer, GNUTLS_KP_TLS_WWW_CLIENT)) {
> - if (isCA || isServer) {
> - virNetError(VIR_ERR_SYSTEM_ERROR, isCA ?
> - _("Certificate %s purpose is TLS client, but wanted a CA certificate") :
> - _("Certificate %s server purpose is TLS client, but wanted a TLS server certificate"),
> - certFile);
> - goto cleanup;
> - }
> + allowClient = true;
> } else if (STRNEQ(buffer, GNUTLS_KP_ANY)) {
> - virNetError(VIR_ERR_SYSTEM_ERROR, (isCA ?
> - _("Certificate %s purpose is wrong, wanted a CA certificate") :
> - (isServer ?
> - _("Certificate %s purpose is wrong, wanted a TLS server certificate") :
> - _("Certificate %s purpose is wrong, wanted a TLS client certificate"))),
> - certFile);
> - goto cleanup;
> + allowServer = allowClient = true;
> }
>
> VIR_FREE(buffer);
> }
>
> + if (!isCA) { /* No purpose checks required for CA certs */
> + if (isServer && !allowServer) {
> + virNetError(VIR_ERR_SYSTEM_ERROR,
> + _("Certificate %s purpose does not allow use for with a TLS server"),
> + certFile);
> + goto cleanup;
> + }
> + if (!isServer && !allowClient) {
> + virNetError(VIR_ERR_SYSTEM_ERROR,
> + _("Certificate %s purpose does not allow use for with a TLS client"),
> + certFile);
> + goto cleanup;
> + }
> + }
> +
>
> ret = 0;
Okay, not a trivial change but relax the contraints,
ACK,
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list