From bd16deaa873b67eef9238635f57906f1340861c9 Mon Sep 17 00:00:00 2001 From: Michael Brown Date: Tue, 24 Apr 2012 16:10:22 +0100 Subject: [PATCH] [crypto] Do not allow build-time cryptography settings to be overridden If a root certificate has been explicitly specified at build time using TRUST=/path/to/cert then do not allow this to be overridden even from a trustworthy settings source (such as VMware GuestInfo). Similarly, if a client certificate (and private key) has been explicitly specified at build time, then do not allow it to be overridden at runtime. Signed-off-by: Michael Brown --- src/crypto/clientcert.c | 93 +++++++++++++++++++++++++---------------- src/crypto/rootcert.c | 67 ++++++++++++++++++----------- 2 files changed, 99 insertions(+), 61 deletions(-) diff --git a/src/crypto/clientcert.c b/src/crypto/clientcert.c index 692aafb4..159a3f4e 100644 --- a/src/crypto/clientcert.c +++ b/src/crypto/clientcert.c @@ -47,6 +47,13 @@ FILE_LICENCE ( GPL2_OR_LATER ); #warning "Attempting to embed private key with no corresponding certificate" #endif +/* Allow client certificates to be overridden if not explicitly specified */ +#ifdef CERTIFICATE +#define ALLOW_CERT_OVERRIDE 0 +#else +#define ALLOW_CERT_OVERRIDE 1 +#endif + /* Raw client certificate data */ extern char client_certificate_data[]; extern char client_certificate_len[]; @@ -72,13 +79,19 @@ __asm__ ( ".section \".rodata\", \"a\", @progbits\n\t" ".previous\n\t" ); /** Client certificate */ -struct client_certificate client_certificate; +struct client_certificate client_certificate = { + .data = client_certificate_data, + .len = ( ( size_t ) client_certificate_len ), +}; /** Client private key */ -struct client_private_key client_private_key; +struct client_private_key client_private_key = { + .data = client_private_key_data, + .len = ( ( size_t ) client_private_key_len ), +}; /** Client certificate setting */ -struct setting cert_setting __setting ( SETTING_CRYPTO ) = { +static struct setting cert_setting __setting ( SETTING_CRYPTO ) = { .name = "cert", .description = "Client certificate", .tag = DHCP_EB_CERT, @@ -86,7 +99,7 @@ struct setting cert_setting __setting ( SETTING_CRYPTO ) = { }; /** Client private key setting */ -struct setting key_setting __setting ( SETTING_CRYPTO ) = { +static struct setting key_setting __setting ( SETTING_CRYPTO ) = { .name = "key", .description = "Client private key", .tag = DHCP_EB_KEY, @@ -99,45 +112,51 @@ struct setting key_setting __setting ( SETTING_CRYPTO ) = { * @ret rc Return status code */ static int clientcert_apply_settings ( void ) { - static void *cert; - static void *key; + static void *cert = NULL; + static void *key = NULL; int len; int rc; - /* Restore default client certificate */ - client_certificate.data = client_certificate_data; - client_certificate.len = ( ( size_t ) client_certificate_len ); + /* Allow client certificate to be overridden only if + * not explicitly specified at build time. + */ + if ( ALLOW_CERT_OVERRIDE ) { - /* Fetch new client certificate, if any */ - free ( cert ); - len = fetch_setting_copy ( NULL, &cert_setting, &cert ); - if ( len < 0 ) { - rc = len; - DBGC ( &client_certificate, "CLIENTCERT cannot fetch client " - "certificate: %s\n", strerror ( rc ) ); - return rc; - } - if ( cert ) { - client_certificate.data = cert; - client_certificate.len = len; - } + /* Restore default client certificate */ + client_certificate.data = client_certificate_data; + client_certificate.len = ( ( size_t ) client_certificate_len ); - /* Restore default client private key */ - client_private_key.data = client_private_key_data; - client_private_key.len = ( ( size_t ) client_private_key_len ); + /* Fetch new client certificate, if any */ + free ( cert ); + len = fetch_setting_copy ( NULL, &cert_setting, &cert ); + if ( len < 0 ) { + rc = len; + DBGC ( &client_certificate, "CLIENTCERT cannot fetch " + "client certificate: %s\n", strerror ( rc ) ); + return rc; + } + if ( cert ) { + client_certificate.data = cert; + client_certificate.len = len; + } - /* Fetch new client private key, if any */ - free ( key ); - len = fetch_setting_copy ( NULL, &key_setting, &key ); - if ( len < 0 ) { - rc = len; - DBGC ( &client_certificate, "CLIENTCERT cannot fetch client " - "private key: %s\n", strerror ( rc ) ); - return rc; - } - if ( key ) { - client_private_key.data = key; - client_private_key.len = len; + /* Restore default client private key */ + client_private_key.data = client_private_key_data; + client_private_key.len = ( ( size_t ) client_private_key_len ); + + /* Fetch new client private key, if any */ + free ( key ); + len = fetch_setting_copy ( NULL, &key_setting, &key ); + if ( len < 0 ) { + rc = len; + DBGC ( &client_certificate, "CLIENTCERT cannot fetch " + "client private key: %s\n", strerror ( rc ) ); + return rc; + } + if ( key ) { + client_private_key.data = key; + client_private_key.len = len; + } } /* Debug */ diff --git a/src/crypto/rootcert.c b/src/crypto/rootcert.c index 6a9e594c..ee2a3454 100644 --- a/src/crypto/rootcert.c +++ b/src/crypto/rootcert.c @@ -36,6 +36,13 @@ FILE_LICENCE ( GPL2_OR_LATER ); /** Length of a root certificate fingerprint */ #define FINGERPRINT_LEN SHA256_DIGEST_SIZE +/* Allow trusted certificates to be overridden if not explicitly specified */ +#ifdef TRUSTED +#define ALLOW_TRUST_OVERRIDE 0 +#else +#define ALLOW_TRUST_OVERRIDE 1 +#endif + /* Use iPXE root CA if no trusted certificates are explicitly specified */ #ifndef TRUSTED #define TRUSTED \ @@ -50,9 +57,9 @@ FILE_LICENCE ( GPL2_OR_LATER ); static const uint8_t fingerprints[] = { TRUSTED }; /** Root certificate fingerprint setting */ -struct setting trust_setting __setting ( SETTING_CRYPTO ) = { +static struct setting trust_setting __setting ( SETTING_CRYPTO ) = { .name = "trust", - .description = "Trusted root certificate fingerprint", + .description = "Trusted root certificate fingerprints", .tag = DHCP_EB_TRUST, .type = &setting_type_hex, }; @@ -67,38 +74,50 @@ struct x509_root root_certificates = { /** * Initialise root certificate * - * We allow the list of trusted root certificate fingerprints to be - * overridden using the "trust" setting, but only at the point of iPXE + * The list of trusted root certificates can be specified at build + * time using the TRUST= build parameter. If no certificates are + * specified, then the default iPXE root CA certificate is trusted. + * + * If no certificates were explicitly specified, then we allow the + * list of trusted root certificate fingerprints to be overridden + * using the "trust" setting, but only at the point of iPXE * initialisation. This prevents untrusted sources of settings * (e.g. DHCP) from subverting the chain of trust, while allowing * trustworthy sources (e.g. VMware GuestInfo or non-volatile stored - * options) to change the trusted root certificate without requiring a - * rebuild. + * options) to specify the trusted root certificate without requiring + * a rebuild. */ static void rootcert_init ( void ) { - void *external; + void *external = NULL; int len; int rc; - /* Fetch copy of "trust" setting, if it exists. This memory - * will never be freed. + /* Allow trusted root certificates to be overridden only if + * not explicitly specified at build time. */ - len = fetch_setting_copy ( NULL, &trust_setting, &external ); - if ( len < 0 ) { - rc = len; - DBGC ( &root_certificates, "ROOTCERT cannot fetch trusted " - "root certificate fingerprints: %s\n", strerror ( rc ) ); - /* No way to prevent startup; fail safe by trusting no - * certificates. - */ - root_certificates.count = 0; - return; - } + if ( ALLOW_TRUST_OVERRIDE ) { - /* Use certificates from "trust" setting, if present */ - if ( external ) { - root_certificates.fingerprints = external; - root_certificates.count = ( len / FINGERPRINT_LEN ); + /* Fetch copy of "trust" setting, if it exists. This + * memory will never be freed. + */ + len = fetch_setting_copy ( NULL, &trust_setting, &external ); + if ( len < 0 ) { + rc = len; + DBGC ( &root_certificates, "ROOTCERT cannot fetch " + "trusted root certificate fingerprints: %s\n", + strerror ( rc ) ); + /* No way to prevent startup; fail safe by + * trusting no certificates. + */ + root_certificates.count = 0; + return; + } + + /* Use certificates from "trust" setting, if present */ + if ( external ) { + root_certificates.fingerprints = external; + root_certificates.count = ( len / FINGERPRINT_LEN ); + } } DBGC ( &root_certificates, "ROOTCERT using %d %s certificate(s):\n",