From: Arran Cudbard-Bell Date: Thu, 17 Jun 2021 20:36:52 +0000 (-0500) Subject: Remove verify_client_cert_cmd X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=40acb3bb0b7e3d023d70dcf407690c61fea57abf;p=thirdparty%2Ffreeradius-server.git Remove verify_client_cert_cmd We don't always have the client certificate to dump out, and similar checks can be performed using unlang. --- diff --git a/raddb/mods-available/eap b/raddb/mods-available/eap index a1fec48172b..80ef6e051d2 100644 --- a/raddb/mods-available/eap +++ b/raddb/mods-available/eap @@ -697,51 +697,6 @@ eap { # } - # - # ### Dynamic CRLs or OCSP - # - # As of version 2.1.10, client certificates can be validated - # via an external command. This allows dynamic CRLs or OCSP to - # be used. - # - # This configuration is commented out in the default - # configuration. Uncomment it, and configure the correct paths - # below to enable it. - # - verify { - # - # tmpdir:: - # - # A temporary directory where the client certificates - # are stored. This directory MUST be owned by the UID - # of the server, and MUST not be accessible by any - # other users. When the server starts, it will do - # `chmod go-rwx` on the directory, for security - # reasons. The directory MUST exist when the server - # starts. - # - # You should also delete all of the files in the - # directory when the server starts. - # -# tmpdir = /tmp/radiusd - - # - # client:: - # - # The command used to verify the client cert. We - # recommend using the OpenSSL command-line tool. - # - # The `${..ca_path}` text is a reference to the ca_path - # variable defined above. - # - # The `%{TLS-Client-Cert-Filename}` is the name of the - # temporary file containing the cert in PEM format. - # This file is automatically deleted by the server when - # the command returns. - # -# client = "/path/to/openssl verify -CApath ${..ca_path} %{TLS-Client-Cert-Filename}" - } - # # ### OCSP Configuration # diff --git a/src/lib/tls/conf-h b/src/lib/tls/conf-h index 2fa2afb064e..0a999fe97a2 100644 --- a/src/lib/tls/conf-h +++ b/src/lib/tls/conf-h @@ -161,8 +161,6 @@ struct fr_tls_conf_s { #endif char const *check_cert_issuer; //!< Verify cert issuer matches the expansion of this string. - char const *verify_tmp_dir; - char const *verify_client_cert_cmd; bool require_client_cert; #ifdef HAVE_OPENSSL_OCSP_H diff --git a/src/lib/tls/conf.c b/src/lib/tls/conf.c index bfd87c28919..4a665df1f20 100644 --- a/src/lib/tls/conf.c +++ b/src/lib/tls/conf.c @@ -96,12 +96,6 @@ static CONF_PARSER cache_config[] = { CONF_PARSER_TERMINATOR }; -static CONF_PARSER verify_config[] = { - { FR_CONF_OFFSET("tmpdir", FR_TYPE_STRING, fr_tls_conf_t, verify_tmp_dir) }, - { FR_CONF_OFFSET("client", FR_TYPE_STRING, fr_tls_conf_t, verify_client_cert_cmd) }, - CONF_PARSER_TERMINATOR -}; - #ifdef HAVE_OPENSSL_OCSP_H static CONF_PARSER ocsp_config[] = { { FR_CONF_OFFSET("enable", FR_TYPE_BOOL, fr_tls_ocsp_conf_t, enable), .dflt = "no" }, @@ -193,8 +187,6 @@ CONF_PARSER fr_tls_server_config[] = { { FR_CONF_OFFSET("cache", FR_TYPE_SUBSECTION, fr_tls_conf_t, cache), .subcs = (void const *) cache_config }, - { FR_CONF_POINTER("verify", FR_TYPE_SUBSECTION, NULL), .subcs = (void const *) verify_config }, - #ifdef HAVE_OPENSSL_OCSP_H { FR_CONF_OFFSET("ocsp", FR_TYPE_SUBSECTION, fr_tls_conf_t, ocsp), .subcs = (void const *) ocsp_config }, @@ -427,19 +419,6 @@ fr_tls_conf_t *fr_tls_conf_parse_server(CONF_SECTION *cs) } #endif /*HAVE_OPENSSL_OCSP_H*/ - if (conf->verify_tmp_dir) { - if (chmod(conf->verify_tmp_dir, S_IRWXU) < 0) { - ERROR("Failed changing permissions on %s: %s", - conf->verify_tmp_dir, fr_syserror(errno)); - goto error; - } - } - - if (conf->verify_client_cert_cmd && !conf->verify_tmp_dir) { - ERROR("You MUST set the verify directory in order to use verify_client_cmd"); - goto error; - } - /* * Ensure our virtual server contains the * correct sections for the specified diff --git a/src/lib/tls/validate.c b/src/lib/tls/validate.c index a53a80ff606..de46fd5848d 100644 --- a/src/lib/tls/validate.c +++ b/src/lib/tls/validate.c @@ -268,61 +268,6 @@ int fr_tls_validate_cert_cb(int ok, X509_STORE_CTX *x509_ctx) } } /* check_cert_cn */ - while (conf->verify_client_cert_cmd) { - char filename[256]; - int fd; - FILE *fp; - fr_pair_t *vp; - - snprintf(filename, sizeof(filename), "%s/client.XXXXXXXX", conf->verify_tmp_dir); - -#ifdef __COVERITY__ - /* - * POSIX-2008 requires that mkstemp creates the file - * with 0600 permissions. So setting umask is pointless - * and although it won't cause crashes, will cause - * race conditions in threaded environments. - */ - umask(0600); -#endif - fd = mkstemp(filename); - if (fd < 0) { - RDEBUG2("Failed creating file in %s: %s", - conf->verify_tmp_dir, fr_syserror(errno)); - break; - } - - fp = fdopen(fd, "w"); - if (!fp) { - close(fd); - REDEBUG("Failed opening file \"%s\": %s", filename, fr_syserror(errno)); - break; - } - - if (!PEM_write_X509(fp, cert)) { - fclose(fp); - REDEBUG("Failed writing certificate to file"); - goto do_unlink; - } - fclose(fp); - - MEM(pair_update_request(&vp, attr_tls_client_cert_filename) >= 0); - fr_pair_value_strdup(vp, filename); - - RDEBUG2("Verifying client certificate with cmd"); - if (radius_exec_program(request, NULL, 0, NULL, request, conf->verify_client_cert_cmd, - &request->request_pairs, true, true, fr_time_delta_from_sec(EXEC_TIMEOUT)) != 0) { - REDEBUG("Client certificate CN \"%s\" failed external verification", common_name); - my_ok = 0; - } else { - RDEBUG2("Client certificate CN \"%s\" passed external validation", common_name); - } - - do_unlink: - unlink(filename); - break; - } - #ifdef HAVE_OPENSSL_OCSP_H /* * Do OCSP last, so we have the complete set of attributes