Improved SAN names parsing and client certificate verification when 'SPIFFE' is used

lib/mysql_data_stream.cpp

@@ -167,29 +167,43 @@ enum sslstatus MySQL_Data_Stream::do_ssl_handshake() {
 	int n = SSL_do_handshake(ssl);
 	if (n == 1) {
 		//proxy_info("SSL handshake completed\n");
-		long rc = SSL_get_verify_result(ssl);
-		if (rc != X509_V_OK && rc != X509_V_ERR_SELF_SIGNED_CERT_IN_CHAIN && rc != X509_V_ERR_UNABLE_TO_VERIFY_LEAF_SIGNATURE) {
-			proxy_error("Disconnecting %s:%d: X509 client SSL certificate verify error: (%d:%s)\n" , addr.addr, addr.port, rc, X509_verify_cert_error_string(rc));
-			return SSLSTATUS_FAIL;
-		} else {
-			X509 *cert;
-			cert = SSL_get_peer_certificate(ssl);
-			if (cert) {
-				ASN1_STRING *str;
-				GENERAL_NAME *sanName;
-				STACK_OF(GENERAL_NAME) *san_names = NULL;
-				san_names = (stack_st_GENERAL_NAME *)X509_get_ext_d2i((X509 *) cert, NID_subject_alt_name, NULL, NULL);
-				if (san_names) {
-					sanName = sk_GENERAL_NAME_value(san_names, 0);
-					str = sanName->d.dNSName;
-					proxy_info("%s\n" , str->data);
-					x509_subject_alt_name = strdup((const char*)str->data);
+		X509 *cert;
+		cert = SSL_get_peer_certificate(ssl);
+		if (cert) {
+			GENERAL_NAMES *alt_names = (stack_st_GENERAL_NAME *)X509_get_ext_d2i((X509*)cert, NID_subject_alt_name, 0, 0);
+			int alt_name_count = sk_GENERAL_NAME_num(alt_names);
+
+			// Iterate all the SAN names, looking for SPIFFE identifier
+			for (int i = 0; i < alt_name_count; i++) {
+				GENERAL_NAME *san = sk_GENERAL_NAME_value(alt_names, i);
+
+				// We only care about URI names
+				if (san->type == GEN_URI) {
+					if (san->d.uniformResourceIdentifier->data) {
+						const char* resource_data =
+							reinterpret_cast<const char*>(san->d.uniformResourceIdentifier->data);
+						const char* spiffe_loc = strstr(resource_data, "spiffe");
+
+						// First name starting with 'spiffe' is considered the match.
+						if (spiffe_loc == resource_data) {
+							x509_subject_alt_name = strdup(resource_data);
+						}
+					}
 				}
-			} else {
-				// we currently disable this annoying error
-				// in future we can configure this as per user level, specifying if the certificate is mandatory or not
-				// see issue #3424
-				//proxy_error("X509 error: no required certificate sent by client\n");
+			}
+		} else {
+			// we currently disable this annoying error
+			// in future we can configure this as per user level, specifying if the certificate is mandatory or not
+			// see issue #3424
+			//proxy_error("X509 error: no required certificate sent by client\n");
+		}
+		// In case the supplied certificate has a 'SAN'-'URI' identifier
+		// starting with 'spiffe', client certificate verification is performed.
+		if (x509_subject_alt_name != NULL) {
+			long rc = SSL_get_verify_result(ssl);
+			if (rc != X509_V_OK) {
+				proxy_error("Disconnecting %s:%d: X509 client SSL certificate verify error: (%d:%s)\n" , addr.addr, addr.port, rc, X509_verify_cert_error_string(rc));
+				return SSLSTATUS_FAIL;
 			}
 		}
 	}

src/proxy_tls.cpp

@@ -497,6 +497,13 @@ int ProxySQL_create_or_load_TLS(bool bootstrap, std::string& msg) {
 		}
 		GloVars.global.ssl_key_pem_mem = load_file(ssl_key_fp);
         GloVars.global.ssl_cert_pem_mem = load_file(ssl_cert_fp);
+
+		// We set the locations for the certificates to be used for
+		// verifications purposes.
+		if (!SSL_CTX_load_verify_locations(GloVars.global.ssl_ctx, ssl_ca_fp, ssl_ca_fp)) {
+			proxy_error("Unable to load CA certificates location for verification. Shutting down\n");
+			exit(EXIT_SUCCESS); // we exit gracefully to not be restarted
+		}
 	} else {
 		// here we use global.tmp_ssl_ctx instead of global.ssl_ctx
 		// because we will try to swap at the end
@@ -505,6 +512,8 @@ int ProxySQL_create_or_load_TLS(bool bootstrap, std::string& msg) {
 				if (SSL_CTX_add_extra_chain_cert(GloVars.global.tmp_ssl_ctx, x509ca) == 1) { // 1 on success
 					if (SSL_CTX_use_PrivateKey(GloVars.global.tmp_ssl_ctx, pkey) == 1) { // 1 on success
 						if (SSL_CTX_check_private_key(GloVars.global.tmp_ssl_ctx) == 1) { // 1 on success
+							if (SSL_CTX_load_verify_locations(GloVars.global.tmp_ssl_ctx, ssl_ca_fp, ssl_ca_fp) == 1) { // 1 on success
+
 							// take the mutex
 							std::lock_guard<std::mutex> lock(GloVars.global.ssl_mutex);
 							// note: we don't free the current SSL context, perhaps used by some connections
@@ -515,6 +524,12 @@ int ProxySQL_create_or_load_TLS(bool bootstrap, std::string& msg) {
 							free(GloVars.global.ssl_cert_pem_mem);
 							GloVars.global.ssl_key_pem_mem = load_file(ssl_key_fp);
         					GloVars.global.ssl_cert_pem_mem = load_file(ssl_cert_fp);
+
+							} else {
+								proxy_error("Failed to load location of CA certificates for verification\n");
+								msg = "Unable to load CA certificates location for verification";
+								ret = 1;
+							}
 						} else {
 							proxy_error("Private key does not match the public certificate\n");
 							msg = "Private key does not match the public certificate";