CLEANUP: ssl: make ssl_sock_load_cert*() return real error codes
These functions were returning only 0 or 1 to mention success or error,
and made it impossible to return a warning. Let's make them return error
codes from ERR_* and map all errors to ERR_ALERT|ERR_FATAL for now since
this is the only code that was set on non-zero return value.
In addition some missing comments were added or adjusted around the
functions' return values.
(cherry picked from commit bbc91965bf4bc7e08c5a9b93fdfa28a64c0949d3)
[EBR: also include a part of 054563de1]
Signed-off-by: Emeric Brun <ebrun@haproxy.com>
diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index 0ff058c..10f91a2 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -3093,9 +3093,8 @@
*
* This will allow the user to explicitly group multiple cert/keys for a single purpose
*
- * Returns
- * 0 on success
- * 1 on failure
+ * Returns a set of ERR_* flags possibly with an error in <err>.
+ *
*/
static int ssl_sock_load_multi_cert(const char *path, struct bind_conf *bind_conf, struct ssl_bind_conf *ssl_conf,
char **sni_filter, int fcount, char **err)
@@ -3111,7 +3110,7 @@
* of keytypes
*/
struct key_combo_ctx key_combos[SSL_SOCK_POSSIBLE_KT_COMBOS] = { {0} };
- int rv = 0;
+ int errcode = 0;
X509_NAME *xname = NULL;
char *str = NULL;
#ifdef SSL_CTRL_SET_TLSEXT_HOSTNAME
@@ -3125,7 +3124,7 @@
snprintf(fp, sizeof(fp), "%s.%s", path, SSL_SOCK_KEYTYPE_NAMES[n]);
if (stat(fp, &buf) == 0) {
if (ssl_sock_load_crt_file_into_ckch(fp, &certs_and_keys[n], err) == 1) {
- rv = 1;
+ errcode |= ERR_ALERT | ERR_FATAL;
goto end;
}
}
@@ -3149,7 +3148,7 @@
if (ret < 0) {
memprintf(err, "%sunable to allocate SSL context.\n",
err && *err ? *err : "");
- rv = 1;
+ errcode |= ERR_ALERT | ERR_FATAL;
goto end;
}
}
@@ -3172,7 +3171,7 @@
if (ret < 0) {
memprintf(err, "%sunable to allocate SSL context.\n",
err && *err ? *err : "");
- rv = 1;
+ errcode |= ERR_ALERT | ERR_FATAL;
goto end;
}
}
@@ -3195,7 +3194,7 @@
if (ret < 0) {
memprintf(err, "%sunable to allocate SSL context.\n",
err && *err ? *err : "");
- rv = 1;
+ errcode |= ERR_ALERT | ERR_FATAL;
goto end;
}
}
@@ -3210,7 +3209,7 @@
if (eb_is_empty(&sni_keytypes_map)) {
memprintf(err, "%sunable to load SSL certificate file '%s' file does not exist.\n",
err && *err ? *err : "", path);
- rv = 1;
+ errcode |= ERR_ALERT | ERR_FATAL;
goto end;
}
@@ -3244,7 +3243,7 @@
if (cur_ctx == NULL) {
memprintf(err, "%sunable to allocate SSL context.\n",
err && *err ? *err : "");
- rv = 1;
+ errcode |= ERR_ALERT | ERR_FATAL;
goto end;
}
@@ -3255,7 +3254,7 @@
snprintf(cur_file, MAXPATHLEN+1, "%s.%s", path, SSL_SOCK_KEYTYPE_NAMES[n]);
if (ssl_sock_put_ckch_into_ctx(cur_file, &certs_and_keys[n], cur_ctx, err) != 0) {
SSL_CTX_free(cur_ctx);
- rv = 1;
+ errcode |= ERR_ALERT | ERR_FATAL;
goto end;
}
@@ -3266,7 +3265,7 @@
memprintf(err, "%s '%s.ocsp' is present and activates OCSP but it is impossible to compute the OCSP certificate ID (maybe the issuer could not be found)'.\n",
*err ? *err : "", cur_file);
SSL_CTX_free(cur_ctx);
- rv = 1;
+ errcode |= ERR_ALERT | ERR_FATAL;
goto end;
}
#elif (defined OPENSSL_IS_BORINGSSL)
@@ -3280,12 +3279,11 @@
if (ssl_dh_ptr_index >= 0)
SSL_CTX_set_ex_data(cur_ctx, ssl_dh_ptr_index, NULL);
- rv = ssl_sock_load_dh_params(cur_ctx, NULL);
- if (rv < 0) {
+ if (ssl_sock_load_dh_params(cur_ctx, NULL) < 0) {
if (err)
memprintf(err, "%sunable to load DH parameters from file '%s'.\n",
*err ? *err : "", path);
- rv = 1;
+ errcode |= ERR_ALERT | ERR_FATAL;
goto end;
}
#endif
@@ -3299,7 +3297,7 @@
kinfo, str, key_combos[i-1].order);
if (key_combos[i-1].order < 0) {
memprintf(err, "%sunable to create a sni context.\n", err && *err ? *err : "");
- rv = 1;
+ errcode |= ERR_ALERT | ERR_FATAL;
goto end;
}
node = ebmb_next(node);
@@ -3333,7 +3331,7 @@
node = next;
}
- return rv;
+ return errcode;
}
#else
/* This is a dummy, that just logs an error and returns error */
@@ -3479,6 +3477,7 @@
return ret;
}
+/* Returns a set of ERR_* flags possibly with an error in <err>. */
static int ssl_sock_load_cert_file(const char *path, struct bind_conf *bind_conf, struct ssl_bind_conf *ssl_conf,
char **sni_filter, int fcount, char **err)
{
@@ -3489,14 +3488,14 @@
if (!ctx) {
memprintf(err, "%sunable to allocate SSL context for cert '%s'.\n",
err && *err ? *err : "", path);
- return 1;
+ return ERR_ALERT | ERR_FATAL;
}
if (SSL_CTX_use_PrivateKey_file(ctx, path, SSL_FILETYPE_PEM) <= 0) {
memprintf(err, "%sunable to load SSL private key from PEM file '%s'.\n",
err && *err ? *err : "", path);
SSL_CTX_free(ctx);
- return 1;
+ return ERR_ALERT | ERR_FATAL;
}
ret = ssl_sock_load_cert_chain_file(ctx, path, bind_conf, ssl_conf, sni_filter, fcount);
@@ -3505,13 +3504,13 @@
err && *err ? *err : "", path);
if (ret < 0) /* serious error, must do that ourselves */
SSL_CTX_free(ctx);
- return 1;
+ return ERR_ALERT | ERR_FATAL;
}
if (SSL_CTX_check_private_key(ctx) <= 0) {
memprintf(err, "%sinconsistencies between private key and certificate loaded from PEM file '%s'.\n",
err && *err ? *err : "", path);
- return 1;
+ return ERR_ALERT | ERR_FATAL;
}
/* we must not free the SSL_CTX anymore below, since it's already in
@@ -3529,7 +3528,7 @@
if (err)
memprintf(err, "%sunable to load DH parameters from file '%s'.\n",
*err ? *err : "", path);
- return 1;
+ return ERR_ALERT | ERR_FATAL;
}
#endif
@@ -3539,7 +3538,7 @@
if (err)
memprintf(err, "%s '%s.ocsp' is present and activates OCSP but it is impossible to compute the OCSP certificate ID (maybe the issuer could not be found)'.\n",
*err ? *err : "", path);
- return 1;
+ return ERR_ALERT | ERR_FATAL;
}
#elif (defined OPENSSL_IS_BORINGSSL)
ssl_sock_set_ocsp_response_from_file(ctx, path);
@@ -3552,7 +3551,7 @@
if (err)
memprintf(err, "%s '%s.sctl' is present but cannot be read or parsed'.\n",
*err ? *err : "", path);
- return 1;
+ return ERR_ALERT | ERR_FATAL;
}
}
#endif
@@ -3561,7 +3560,7 @@
if (bind_conf->default_ctx) {
memprintf(err, "%sthis version of openssl cannot load multiple SSL certificates.\n",
err && *err ? *err : "");
- return 1;
+ return ERR_ALERT | ERR_FATAL;
}
#endif
if (!bind_conf->default_ctx) {
@@ -3572,6 +3571,8 @@
return 0;
}
+
+/* Returns a set of ERR_* flags possibly with an error in <err>. */
int ssl_sock_load_cert(char *path, struct bind_conf *bind_conf, char **err)
{
struct dirent **de_list;
@@ -3599,7 +3600,7 @@
if (n < 0) {
memprintf(err, "%sunable to scan directory '%s' : %s.\n",
err && *err ? *err : "", path, strerror(errno));
- cfgerr++;
+ cfgerr |= ERR_ALERT | ERR_FATAL;
}
else {
for (i = 0; i < n; i++) {
@@ -3613,7 +3614,7 @@
if (stat(fp, &buf) != 0) {
memprintf(err, "%sunable to stat SSL certificate from file '%s' : %s.\n",
err && *err ? *err : "", fp, strerror(errno));
- cfgerr++;
+ cfgerr |= ERR_ALERT | ERR_FATAL;
goto ignore_entry;
}
if (!S_ISREG(buf.st_mode))
@@ -3649,7 +3650,7 @@
}
snprintf(fp, sizeof(fp), "%s/%s", path, dp);
- cfgerr += ssl_sock_load_multi_cert(fp, bind_conf, NULL, NULL, 0, err);
+ cfgerr |= ssl_sock_load_multi_cert(fp, bind_conf, NULL, NULL, 0, err);
/* Successfully processed the bundle */
goto ignore_entry;
@@ -3657,7 +3658,7 @@
}
#endif
- cfgerr += ssl_sock_load_cert_file(fp, bind_conf, NULL, NULL, 0, err);
+ cfgerr |= ssl_sock_load_cert_file(fp, bind_conf, NULL, NULL, 0, err);
ignore_entry:
free(de);
}
@@ -3667,7 +3668,7 @@
return cfgerr;
}
- cfgerr = ssl_sock_load_multi_cert(path, bind_conf, NULL, NULL, 0, err);
+ cfgerr |= ssl_sock_load_multi_cert(path, bind_conf, NULL, NULL, 0, err);
return cfgerr;
}
@@ -3717,6 +3718,7 @@
}
}
+/* Returns a set of ERR_* flags possibly with an error in <err>. */
int ssl_sock_load_cert_list_file(char *file, struct bind_conf *bind_conf, struct proxy *curproxy, char **err)
{
char thisline[CRT_LINESIZE];
@@ -3728,7 +3730,7 @@
if ((f = fopen(file, "r")) == NULL) {
memprintf(err, "cannot open file '%s' : %s", file, strerror(errno));
- return 1;
+ return ERR_ALERT | ERR_FATAL;
}
while (fgets(thisline, sizeof(thisline), f) != NULL) {
@@ -3747,7 +3749,7 @@
*/
memprintf(err, "line %d too long in file '%s', limit is %d characters",
linenum, file, (int)sizeof(thisline)-1);
- cfgerr = 1;
+ cfgerr |= ERR_ALERT | ERR_FATAL;
break;
}
@@ -3764,12 +3766,12 @@
} else if (*line == '[') {
if (ssl_b) {
memprintf(err, "too many '[' on line %d in file '%s'.", linenum, file);
- cfgerr = 1;
+ cfgerr |= ERR_ALERT | ERR_FATAL;
break;
}
if (!arg) {
memprintf(err, "file must start with a cert on line %d in file '%s'", linenum, file);
- cfgerr = 1;
+ cfgerr |= ERR_ALERT | ERR_FATAL;
break;
}
ssl_b = arg;
@@ -3778,12 +3780,12 @@
} else if (*line == ']') {
if (ssl_e) {
memprintf(err, "too many ']' on line %d in file '%s'.", linenum, file);
- cfgerr = 1;
+ cfgerr |= ERR_ALERT | ERR_FATAL;
break;
}
if (!ssl_b) {
memprintf(err, "missing '[' in line %d in file '%s'.", linenum, file);
- cfgerr = 1;
+ cfgerr |= ERR_ALERT | ERR_FATAL;
break;
}
ssl_e = arg;
@@ -3792,7 +3794,7 @@
} else if (newarg) {
if (arg == MAX_CRT_ARGS) {
memprintf(err, "too many args on line %d in file '%s'.", linenum, file);
- cfgerr = 1;
+ cfgerr |= ERR_ALERT | ERR_FATAL;
break;
}
newarg = 0;
@@ -3800,7 +3802,7 @@
}
line++;
}
- if (cfgerr)
+ if (cfgerr & ERR_CODE)
break;
args[arg++] = line;
@@ -3813,7 +3815,7 @@
if ((strlen(global_ssl.crt_base) + 1 + strlen(crt_path)) > MAXPATHLEN) {
memprintf(err, "'%s' : path too long on line %d in file '%s'",
crt_path, linenum, file);
- cfgerr = 1;
+ cfgerr |= ERR_ALERT | ERR_FATAL;
break;
}
snprintf(path, sizeof(path), "%s/%s", global_ssl.crt_base, crt_path);
@@ -3827,11 +3829,11 @@
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, curproxy, ssl_conf, err);
+ cfgerr |= ssl_bind_kws[i].parse(args, cur_arg, curproxy, ssl_conf, err);
if (cur_arg + 1 + ssl_bind_kws[i].skip > ssl_e) {
memprintf(err, "ssl args out of '[]' for %s on line %d in file '%s'",
args[cur_arg], linenum, file);
- cfgerr = 1;
+ cfgerr |= ERR_ALERT | ERR_FATAL;
}
cur_arg += 1 + ssl_bind_kws[i].skip;
break;
@@ -3840,11 +3842,12 @@
if (!cfgerr && !newarg) {
memprintf(err, "unknown ssl keyword %s on line %d in file '%s'.",
args[cur_arg], linenum, file);
- cfgerr = 1;
+ cfgerr |= ERR_ALERT | ERR_FATAL;
break;
}
}
- if (cfgerr) {
+
+ if (cfgerr & ERR_CODE) {
ssl_sock_free_ssl_conf(ssl_conf);
free(ssl_conf);
ssl_conf = NULL;
@@ -3852,14 +3855,14 @@
}
if (stat(crt_path, &buf) == 0) {
- cfgerr = ssl_sock_load_cert_file(crt_path, bind_conf, ssl_conf,
- &args[cur_arg], arg - cur_arg - 1, err);
- } else {
- cfgerr = ssl_sock_load_multi_cert(crt_path, bind_conf, ssl_conf,
+ cfgerr |= ssl_sock_load_cert_file(crt_path, bind_conf, ssl_conf,
&args[cur_arg], arg - cur_arg - 1, err);
+ } else {
+ cfgerr |= ssl_sock_load_multi_cert(crt_path, bind_conf, ssl_conf,
+ &args[cur_arg], arg - cur_arg - 1, err);
}
- if (cfgerr) {
+ if (cfgerr & ERR_CODE) {
memprintf(err, "error processing line %d in file '%s' : %s", linenum, file, *err);
break;
}
@@ -7822,7 +7825,7 @@
}
#endif
-/* parse the "crt" bind keyword */
+/* parse the "crt" bind keyword. Returns a set of ERR_* flags possibly with an error in <err>. */
static int bind_parse_crt(char **args, int cur_arg, struct proxy *px, struct bind_conf *conf, char **err)
{
char path[MAXPATHLEN];
@@ -7838,32 +7841,27 @@
return ERR_ALERT | ERR_FATAL;
}
snprintf(path, sizeof(path), "%s/%s", global_ssl.crt_base, args[cur_arg + 1]);
- if (ssl_sock_load_cert(path, conf, err) > 0)
- return ERR_ALERT | ERR_FATAL;
-
- return 0;
+ return ssl_sock_load_cert(path, conf, err);
}
- if (ssl_sock_load_cert(args[cur_arg + 1], conf, err) > 0)
- return ERR_ALERT | ERR_FATAL;
-
- return 0;
+ return ssl_sock_load_cert(args[cur_arg + 1], conf, err);
}
-/* parse the "crt-list" bind keyword */
+/* parse the "crt-list" bind keyword. Returns a set of ERR_* flags possibly with an error in <err>. */
static int bind_parse_crt_list(char **args, int cur_arg, struct proxy *px, struct bind_conf *conf, char **err)
{
+ int err_code;
+
if (!*args[cur_arg + 1]) {
memprintf(err, "'%s' : missing certificate location", args[cur_arg]);
return ERR_ALERT | ERR_FATAL;
}
- if (ssl_sock_load_cert_list_file(args[cur_arg + 1], conf, px, err) > 0) {
+ err_code = ssl_sock_load_cert_list_file(args[cur_arg + 1], conf, px, err);
+ if (err_code)
memprintf(err, "'%s' : %s", args[cur_arg], *err);
- return ERR_ALERT | ERR_FATAL;
- }
- return 0;
+ return err_code;
}
/* parse the "crl-file" bind keyword */