BUG/MINOR: ssl: Prevent disk access when using "add ssl crt-list"
If an unknown CA file was first mentioned in an "add ssl crt-list" CLI
command, it would result in a call to X509_STORE_load_locations which
performs a disk access which is forbidden during runtime. The same would
happen if a "ca-verify-file" or "crl-file" was specified. This was due
to the fact that the crt-list file parsing and the crt-list related CLI
commands parsing use the same functions.
The patch simply adds a new parameter to all the ssl_bind parsing
functions so that they know if the call is made during init or by the
CLI, and the ssl_store_load_locations function can then reject any new
cafile_entry creation coming from a CLI call.
It can be backported as far as 2.2.
(cherry picked from commit fb00f31af4ba67c69a12807729514a2bdcd47efa)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
diff --git a/src/cfgparse-ssl.c b/src/cfgparse-ssl.c
index d24b85a..f9fa7a2 100644
--- a/src/cfgparse-ssl.c
+++ b/src/cfgparse-ssl.c
@@ -530,7 +530,7 @@
/***************************** Bind keyword Parsing ********************************************/
/* for ca-file and ca-verify-file */
-static int ssl_bind_parse_ca_file_common(char **args, int cur_arg, char **ca_file_p, char **err)
+static int ssl_bind_parse_ca_file_common(char **args, int cur_arg, char **ca_file_p, int from_cli, char **err)
{
if (!*args[cur_arg + 1]) {
memprintf(err, "'%s' : missing CAfile path", args[cur_arg]);
@@ -542,7 +542,7 @@
else
memprintf(ca_file_p, "%s", args[cur_arg + 1]);
- if (!ssl_store_load_locations_file(*ca_file_p)) {
+ if (!ssl_store_load_locations_file(*ca_file_p, !from_cli)) {
memprintf(err, "'%s' : unable to load %s", args[cur_arg], *ca_file_p);
return ERR_ALERT | ERR_FATAL;
}
@@ -550,23 +550,23 @@
}
/* parse the "ca-file" bind keyword */
-static int ssl_bind_parse_ca_file(char **args, int cur_arg, struct proxy *px, struct ssl_bind_conf *conf, char **err)
+static int ssl_bind_parse_ca_file(char **args, int cur_arg, struct proxy *px, struct ssl_bind_conf *conf, int from_cli, char **err)
{
- return ssl_bind_parse_ca_file_common(args, cur_arg, &conf->ca_file, err);
+ return ssl_bind_parse_ca_file_common(args, cur_arg, &conf->ca_file, from_cli, err);
}
static int bind_parse_ca_file(char **args, int cur_arg, struct proxy *px, struct bind_conf *conf, char **err)
{
- return ssl_bind_parse_ca_file(args, cur_arg, px, &conf->ssl_conf, err);
+ return ssl_bind_parse_ca_file(args, cur_arg, px, &conf->ssl_conf, 0, err);
}
/* parse the "ca-verify-file" bind keyword */
-static int ssl_bind_parse_ca_verify_file(char **args, int cur_arg, struct proxy *px, struct ssl_bind_conf *conf, char **err)
+static int ssl_bind_parse_ca_verify_file(char **args, int cur_arg, struct proxy *px, struct ssl_bind_conf *conf, int from_cli, char **err)
{
- return ssl_bind_parse_ca_file_common(args, cur_arg, &conf->ca_verify_file, err);
+ return ssl_bind_parse_ca_file_common(args, cur_arg, &conf->ca_verify_file, from_cli, err);
}
static int bind_parse_ca_verify_file(char **args, int cur_arg, struct proxy *px, struct bind_conf *conf, char **err)
{
- return ssl_bind_parse_ca_verify_file(args, cur_arg, px, &conf->ssl_conf, err);
+ return ssl_bind_parse_ca_verify_file(args, cur_arg, px, &conf->ssl_conf, 0, err);
}
/* parse the "ca-sign-file" bind keyword */
@@ -597,7 +597,7 @@
}
/* parse the "ciphers" bind keyword */
-static int ssl_bind_parse_ciphers(char **args, int cur_arg, struct proxy *px, struct ssl_bind_conf *conf, char **err)
+static int ssl_bind_parse_ciphers(char **args, int cur_arg, struct proxy *px, struct ssl_bind_conf *conf, int from_cli, char **err)
{
if (!*args[cur_arg + 1]) {
memprintf(err, "'%s' : missing cipher suite", args[cur_arg]);
@@ -610,12 +610,12 @@
}
static int bind_parse_ciphers(char **args, int cur_arg, struct proxy *px, struct bind_conf *conf, char **err)
{
- return ssl_bind_parse_ciphers(args, cur_arg, px, &conf->ssl_conf, err);
+ return ssl_bind_parse_ciphers(args, cur_arg, px, &conf->ssl_conf, 0, err);
}
#if (HA_OPENSSL_VERSION_NUMBER >= 0x10101000L)
/* parse the "ciphersuites" bind keyword */
-static int ssl_bind_parse_ciphersuites(char **args, int cur_arg, struct proxy *px, struct ssl_bind_conf *conf, char **err)
+static int ssl_bind_parse_ciphersuites(char **args, int cur_arg, struct proxy *px, struct ssl_bind_conf *conf, int from_cli, char **err)
{
if (!*args[cur_arg + 1]) {
memprintf(err, "'%s' : missing cipher suite", args[cur_arg]);
@@ -628,7 +628,7 @@
}
static int bind_parse_ciphersuites(char **args, int cur_arg, struct proxy *px, struct bind_conf *conf, char **err)
{
- return ssl_bind_parse_ciphersuites(args, cur_arg, px, &conf->ssl_conf, err);
+ return ssl_bind_parse_ciphersuites(args, cur_arg, px, &conf->ssl_conf, 0, err);
}
#endif
@@ -672,7 +672,7 @@
}
/* parse the "crl-file" bind keyword */
-static int ssl_bind_parse_crl_file(char **args, int cur_arg, struct proxy *px, struct ssl_bind_conf *conf, char **err)
+static int ssl_bind_parse_crl_file(char **args, int cur_arg, struct proxy *px, struct ssl_bind_conf *conf, int from_cli, char **err)
{
#ifndef X509_V_FLAG_CRL_CHECK
memprintf(err, "'%s' : library does not support CRL verify", args[cur_arg]);
@@ -688,7 +688,7 @@
else
memprintf(&conf->crl_file, "%s", args[cur_arg + 1]);
- if (!ssl_store_load_locations_file(conf->crl_file)) {
+ if (!ssl_store_load_locations_file(conf->crl_file, !from_cli)) {
memprintf(err, "'%s' : unable to load %s", args[cur_arg], conf->crl_file);
return ERR_ALERT | ERR_FATAL;
}
@@ -697,11 +697,11 @@
}
static int bind_parse_crl_file(char **args, int cur_arg, struct proxy *px, struct bind_conf *conf, char **err)
{
- return ssl_bind_parse_crl_file(args, cur_arg, px, &conf->ssl_conf, err);
+ return ssl_bind_parse_crl_file(args, cur_arg, px, &conf->ssl_conf, 0, err);
}
/* parse the "curves" bind keyword keyword */
-static int ssl_bind_parse_curves(char **args, int cur_arg, struct proxy *px, struct ssl_bind_conf *conf, char **err)
+static int ssl_bind_parse_curves(char **args, int cur_arg, struct proxy *px, struct ssl_bind_conf *conf, int from_cli, char **err)
{
#if defined(SSL_CTX_set1_curves_list)
if (!*args[cur_arg + 1]) {
@@ -717,11 +717,11 @@
}
static int bind_parse_curves(char **args, int cur_arg, struct proxy *px, struct bind_conf *conf, char **err)
{
- return ssl_bind_parse_curves(args, cur_arg, px, &conf->ssl_conf, err);
+ return ssl_bind_parse_curves(args, cur_arg, px, &conf->ssl_conf, 0, err);
}
/* parse the "ecdhe" bind keyword keyword */
-static int ssl_bind_parse_ecdhe(char **args, int cur_arg, struct proxy *px, struct ssl_bind_conf *conf, char **err)
+static int ssl_bind_parse_ecdhe(char **args, int cur_arg, struct proxy *px, struct ssl_bind_conf *conf, int from_cli, char **err)
{
#if HA_OPENSSL_VERSION_NUMBER < 0x0090800fL
memprintf(err, "'%s' : library does not support elliptic curve Diffie-Hellman (too old)", args[cur_arg]);
@@ -742,7 +742,7 @@
}
static int bind_parse_ecdhe(char **args, int cur_arg, struct proxy *px, struct bind_conf *conf, char **err)
{
- return ssl_bind_parse_ecdhe(args, cur_arg, px, &conf->ssl_conf, err);
+ return ssl_bind_parse_ecdhe(args, cur_arg, px, &conf->ssl_conf, 0, err);
}
/* parse the "crt-ignore-err" and "ca-ignore-err" bind keywords */
@@ -851,7 +851,7 @@
return 0;
}
-static int ssl_bind_parse_tls_method_minmax(char **args, int cur_arg, struct proxy *px, struct ssl_bind_conf *conf, char **err)
+static int ssl_bind_parse_tls_method_minmax(char **args, int cur_arg, struct proxy *px, struct ssl_bind_conf *conf, int from_cli, char **err)
{
int ret;
@@ -885,7 +885,7 @@
}
/* parse the "allow-0rtt" bind keyword */
-static int ssl_bind_parse_allow_0rtt(char **args, int cur_arg, struct proxy *px, struct ssl_bind_conf *conf, char **err)
+static int ssl_bind_parse_allow_0rtt(char **args, int cur_arg, struct proxy *px, struct ssl_bind_conf *conf, int from_cli, char **err)
{
conf->early_data = 1;
return 0;
@@ -898,7 +898,7 @@
}
/* parse the "npn" bind keyword */
-static int ssl_bind_parse_npn(char **args, int cur_arg, struct proxy *px, struct ssl_bind_conf *conf, char **err)
+static int ssl_bind_parse_npn(char **args, int cur_arg, struct proxy *px, struct ssl_bind_conf *conf, int from_cli, char **err)
{
#if defined(OPENSSL_NPN_NEGOTIATED) && !defined(OPENSSL_NO_NEXTPROTONEG)
char *p1, *p2;
@@ -949,7 +949,7 @@
static int bind_parse_npn(char **args, int cur_arg, struct proxy *px, struct bind_conf *conf, char **err)
{
- return ssl_bind_parse_npn(args, cur_arg, px, &conf->ssl_conf, err);
+ return ssl_bind_parse_npn(args, cur_arg, px, &conf->ssl_conf, 0, err);
}
@@ -1015,7 +1015,7 @@
}
/* parse the "alpn" bind keyword */
-static int ssl_bind_parse_alpn(char **args, int cur_arg, struct proxy *px, struct ssl_bind_conf *conf, char **err)
+static int ssl_bind_parse_alpn(char **args, int cur_arg, struct proxy *px, struct ssl_bind_conf *conf, int from_cli, char **err)
{
#ifdef TLSEXT_TYPE_application_layer_protocol_negotiation
int ret;
@@ -1034,7 +1034,7 @@
static int bind_parse_alpn(char **args, int cur_arg, struct proxy *px, struct bind_conf *conf, char **err)
{
- return ssl_bind_parse_alpn(args, cur_arg, px, &conf->ssl_conf, err);
+ return ssl_bind_parse_alpn(args, cur_arg, px, &conf->ssl_conf, 0, err);
}
/* parse the "ssl" bind keyword */
@@ -1201,7 +1201,7 @@
}
/* parse the "verify" bind keyword */
-static int ssl_bind_parse_verify(char **args, int cur_arg, struct proxy *px, struct ssl_bind_conf *conf, char **err)
+static int ssl_bind_parse_verify(char **args, int cur_arg, struct proxy *px, struct ssl_bind_conf *conf, int from_cli, char **err)
{
if (!*args[cur_arg + 1]) {
memprintf(err, "'%s' : missing verify method", args[cur_arg]);
@@ -1224,18 +1224,18 @@
}
static int bind_parse_verify(char **args, int cur_arg, struct proxy *px, struct bind_conf *conf, char **err)
{
- return ssl_bind_parse_verify(args, cur_arg, px, &conf->ssl_conf, err);
+ return ssl_bind_parse_verify(args, cur_arg, px, &conf->ssl_conf, 0, err);
}
/* parse the "no-ca-names" bind keyword */
-static int ssl_bind_parse_no_ca_names(char **args, int cur_arg, struct proxy *px, struct ssl_bind_conf *conf, char **err)
+static int ssl_bind_parse_no_ca_names(char **args, int cur_arg, struct proxy *px, struct ssl_bind_conf *conf, int from_cli, char **err)
{
conf->no_ca_names = 1;
return 0;
}
static int bind_parse_no_ca_names(char **args, int cur_arg, struct proxy *px, struct bind_conf *conf, char **err)
{
- return ssl_bind_parse_no_ca_names(args, cur_arg, px, &conf->ssl_conf, err);
+ return ssl_bind_parse_no_ca_names(args, cur_arg, px, &conf->ssl_conf, 0, err);
}
/***************************** "server" keywords Parsing ********************************************/
@@ -1333,7 +1333,7 @@
else
memprintf(&newsrv->ssl_ctx.ca_file, "%s", args[*cur_arg + 1]);
- if (!ssl_store_load_locations_file(newsrv->ssl_ctx.ca_file)) {
+ if (!ssl_store_load_locations_file(newsrv->ssl_ctx.ca_file, 1)) {
memprintf(err, "'%s' : unable to load %s", args[*cur_arg], newsrv->ssl_ctx.ca_file);
return ERR_ALERT | ERR_FATAL;
}
@@ -1422,7 +1422,7 @@
else
memprintf(&newsrv->ssl_ctx.crl_file, "%s", args[*cur_arg + 1]);
- if (!ssl_store_load_locations_file(newsrv->ssl_ctx.crl_file)) {
+ if (!ssl_store_load_locations_file(newsrv->ssl_ctx.crl_file, 1)) {
memprintf(err, "'%s' : unable to load %s", args[*cur_arg], newsrv->ssl_ctx.crl_file);
return ERR_ALERT | ERR_FATAL;
}
diff --git a/src/ssl_crtlist.c b/src/ssl_crtlist.c
index 077d6da..976e4be 100644
--- a/src/ssl_crtlist.c
+++ b/src/ssl_crtlist.c
@@ -311,7 +311,7 @@
* <crt_path> is a ptr in <line>
* Return an error code
*/
-int crtlist_parse_line(char *line, char **crt_path, struct crtlist_entry *entry, const char *file, int linenum, char **err)
+int crtlist_parse_line(char *line, char **crt_path, struct crtlist_entry *entry, const char *file, int linenum, int from_cli, char **err)
{
int cfgerr = 0;
int arg, newarg, cur_arg, i, ssl_b = 0, ssl_e = 0;
@@ -395,13 +395,14 @@
goto error;
}
}
+
cur_arg = ssl_b ? ssl_b : 1;
while (cur_arg < ssl_e) {
newarg = 0;
for (i = 0; ssl_bind_kws[i].kw != NULL; i++) {
if (strcmp(ssl_bind_kws[i].kw, args[cur_arg]) == 0) {
newarg = 1;
- cfgerr |= ssl_bind_kws[i].parse(args, cur_arg, NULL, ssl_conf, err);
+ cfgerr |= ssl_bind_kws[i].parse(args, cur_arg, NULL, ssl_conf, from_cli, err);
if (cur_arg + 1 + ssl_bind_kws[i].skip > ssl_e) {
memprintf(err, "parsing [%s:%d]: ssl args out of '[]' for %s",
file, linenum, args[cur_arg]);
@@ -512,7 +513,7 @@
goto error;
}
- cfgerr |= crtlist_parse_line(thisline, &crt_path, entry, file, linenum, err);
+ cfgerr |= crtlist_parse_line(thisline, &crt_path, entry, file, linenum, 0, err);
if (cfgerr & ERR_CODE)
goto error;
@@ -1216,7 +1217,7 @@
goto error;
}
/* cert_path is filled here */
- cfgerr |= crtlist_parse_line(payload, &cert_path, entry, "CLI", 1, &err);
+ cfgerr |= crtlist_parse_line(payload, &cert_path, entry, "CLI", 1, 1, &err);
if (cfgerr & ERR_CODE)
goto error;
} else {
diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index bd7b51b..10e2f44 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -300,11 +300,16 @@
return NULL;
}
-int ssl_store_load_locations_file(char *path)
+int ssl_store_load_locations_file(char *path, int create_if_none)
{
- if (ssl_store_get0_locations_file(path) == NULL) {
+ X509_STORE *store = ssl_store_get0_locations_file(path);
+
+ /* If this function is called by the CLI, we should not call the
+ * X509_STORE_load_locations function because it performs forbidden disk
+ * accesses. */
+ if (!store && create_if_none) {
struct cafile_entry *ca_e;
- X509_STORE *store = X509_STORE_new();
+ store = X509_STORE_new();
if (X509_STORE_load_locations(store, path, NULL)) {
int pathlen;
pathlen = strlen(path);
@@ -313,13 +318,13 @@
memcpy(ca_e->path, path, pathlen + 1);
ca_e->ca_store = store;
ebst_insert(&cafile_tree, &ca_e->node);
- return 1;
}
+ } else {
+ X509_STORE_free(store);
+ store = NULL;
}
- X509_STORE_free(store);
- return 0;
}
- return 1;
+ return (store != NULL);
}
/* mimic what X509_STORE_load_locations do with store_ctx */