[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 <mcb30@ipxe.org>

diff --git a/src/crypto/clientcert.c b/src/crypto/clientcert.c
index 692aafb466587d2e7bfb7b0e0d618c3c671a8bf3..159a3f4e592fbfef930ebb3f661864450513fd04 100644 (file)
--- 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 );
-
-       /* 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 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;
+       /* Allow client certificate to be overridden only if
+        * not explicitly specified at build time.
+        */
+       if ( ALLOW_CERT_OVERRIDE ) {
+
+               /* Restore default client certificate */
+               client_certificate.data = client_certificate_data;
+               client_certificate.len = ( ( size_t ) client_certificate_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;
+               }
+
+               /* 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 6a9e594cc8b38b69c5c771c345041bbebf8b2ebd..ee2a3454edc11cd0c08dc08fae190cf14019e4c4 100644 (file)
--- 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.
+       if ( ALLOW_TRUST_OVERRIDE ) {
+
+               /* Fetch copy of "trust" setting, if it exists.  This
+                * memory will never be freed.
                 */
-               root_certificates.count = 0;
-               return;
-       }
+               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 );
+               /* 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",