On Wed, Sep 10, 2014 at 16:38, Ted Unangst wrote: > On Fri, Aug 15, 2014 at 13:06, Ted Unangst wrote: > >> Instead, ressl should copy all parameters as necessary and >> free them. This does introduce an error case into formerly void >> functions, but I think that's ok. The alternative would be to use >> fixed arrays inside ressl_config, but that seems much worse. > > Here's a complete diff. > > (I think we should also zero the key memory if not null, but that's not > included in this change.)
Kent Spillner noticed I hadn't cleaned up the references to default config in ressl.c. Here's a fixed diff, that also zeroes key memory. Index: ressl.c =================================================================== RCS file: /cvs/src/lib/libressl/ressl.c,v retrieving revision 1.12 diff -u -p -r1.12 ressl.c --- ressl.c 15 Aug 2014 16:55:32 -0000 1.12 +++ ressl.c 11 Sep 2014 19:18:49 -0000 @@ -29,7 +29,7 @@ #include <ressl.h> #include "ressl_internal.h" -extern struct ressl_config ressl_config_default; +static struct ressl_config *ressl_config_default; int ressl_init(void) @@ -42,6 +42,9 @@ ressl_init(void) SSL_load_error_strings(); SSL_library_init(); + if ((ressl_config_default = ressl_config_new()) == NULL) + return (-1); + ressl_initialised = 1; return (0); @@ -78,7 +81,7 @@ ressl_new(void) if ((ctx = calloc(1, sizeof(*ctx))) == NULL) return (NULL); - ctx->config = &ressl_config_default; + ctx->config = ressl_config_default; ressl_reset(ctx); @@ -89,7 +92,7 @@ int ressl_configure(struct ressl *ctx, struct ressl_config *config) { if (config == NULL) - config = &ressl_config_default; + config = ressl_config_default; ctx->config = config; Index: ressl.h =================================================================== RCS file: /cvs/src/lib/libressl/ressl.h,v retrieving revision 1.13 diff -u -p -r1.13 ressl.h --- ressl.h 27 Aug 2014 10:46:53 -0000 1.13 +++ ressl.h 10 Sep 2014 20:23:46 -0000 @@ -31,15 +31,15 @@ const char *ressl_error(struct ressl *ct struct ressl_config *ressl_config_new(void); void ressl_config_free(struct ressl_config *config); -void ressl_config_set_ca_file(struct ressl_config *config, char *ca_file); -void ressl_config_set_ca_path(struct ressl_config *config, char *ca_path); -void ressl_config_set_cert_file(struct ressl_config *config, char *cert_file); -void ressl_config_set_cert_mem(struct ressl_config *config, char *cert, +int ressl_config_set_ca_file(struct ressl_config *config, char *ca_file); +int ressl_config_set_ca_path(struct ressl_config *config, char *ca_path); +int ressl_config_set_cert_file(struct ressl_config *config, char *cert_file); +int ressl_config_set_cert_mem(struct ressl_config *config, char *cert, size_t len); -void ressl_config_set_ciphers(struct ressl_config *config, char *ciphers); +int ressl_config_set_ciphers(struct ressl_config *config, char *ciphers); int ressl_config_set_ecdhcurve(struct ressl_config *config, const char *); -void ressl_config_set_key_file(struct ressl_config *config, char *key_file); -void ressl_config_set_key_mem(struct ressl_config *config, char *key, +int ressl_config_set_key_file(struct ressl_config *config, char *key_file); +int ressl_config_set_key_mem(struct ressl_config *config, char *key, size_t len); void ressl_config_set_verify_depth(struct ressl_config *config, int verify_depth); Index: ressl_config.c =================================================================== RCS file: /cvs/src/lib/libressl/ressl_config.c,v retrieving revision 1.8 diff -u -p -r1.8 ressl_config.c --- ressl_config.c 27 Aug 2014 10:46:53 -0000 1.8 +++ ressl_config.c 11 Sep 2014 19:14:27 -0000 @@ -21,27 +21,55 @@ #include <ressl.h> #include "ressl_internal.h" -/* - * Default configuration. - */ -struct ressl_config ressl_config_default = { - .ca_file = _PATH_SSL_CA_FILE, - .ca_path = NULL, - .ciphers = NULL, - .ecdhcurve = NID_X9_62_prime256v1, - .verify = 1, - .verify_depth = 6, -}; +#define SET_STRING(config, name, val) do { \ + free(config->name); \ + config->name = NULL; \ + if (val != NULL) { \ + if ((config->name = strdup(val)) == NULL) \ + return -1; \ + } \ +} while (0) + +#define SET_MEM(config, name, namelen, val, vallen) do { \ + free(config->name); \ + config->name = NULL; \ + if (val != NULL) { \ + if ((config->name = memdup(val, vallen)) == NULL) \ + return -1; \ + config->namelen = vallen; \ + } \ +} while (0) + +static void * +memdup(const void *in, size_t len) +{ + void *out; + + if ((out = malloc(len)) == NULL) + return NULL; + memcpy(out, in, len); + return out; +} struct ressl_config * ressl_config_new(void) { struct ressl_config *config; - if ((config = malloc(sizeof(*config))) == NULL) + if ((config = calloc(1, sizeof(*config))) == NULL) return (NULL); - memcpy(config, &ressl_config_default, sizeof(*config)); + /* + * Default configuration. + */ + if (ressl_config_set_ca_file(config, _PATH_SSL_CA_FILE) != 0) { + ressl_config_free(config); + return (NULL); + } + ressl_config_verify(config); + ressl_config_set_verify_depth(config, 6); + /* ? use function ? */ + config->ecdhcurve = NID_X9_62_prime256v1; return (config); } @@ -49,38 +77,54 @@ ressl_config_new(void) void ressl_config_free(struct ressl_config *config) { + if (config == NULL) + return; + free(config->ca_file); + free(config->ca_path); + free(config->cert_file); + free(config->cert_mem); + free(config->ciphers); + free(config->key_file); + if (config->key_mem != NULL) { + explicit_bzero(config->key_mem, config->key_len); + free(config->key_mem); + } free(config); } -void +int ressl_config_set_ca_file(struct ressl_config *config, char *ca_file) { - config->ca_file = ca_file; + SET_STRING(config, ca_file, ca_file); + return 0; } -void +int ressl_config_set_ca_path(struct ressl_config *config, char *ca_path) { - config->ca_path = ca_path; + SET_STRING(config, ca_path, ca_path); + return 0; } -void +int ressl_config_set_cert_file(struct ressl_config *config, char *cert_file) { - config->cert_file = cert_file; + SET_STRING(config, cert_file, cert_file); + return 0; } -void +int ressl_config_set_cert_mem(struct ressl_config *config, char *cert, size_t len) { - config->cert_mem = cert; - config->cert_len = len; + SET_MEM(config, cert_mem, cert_len, cert, len); + return 0; } -void +int ressl_config_set_ciphers(struct ressl_config *config, char *ciphers) { - config->ciphers = ciphers; + SET_STRING(config, ciphers, ciphers); + return 0; } int @@ -95,17 +139,18 @@ ressl_config_set_ecdhcurve(struct ressl_ return (0); } -void +int ressl_config_set_key_file(struct ressl_config *config, char *key_file) { - config->key_file = key_file; + SET_STRING(config, key_file, key_file); + return 0; } -void +int ressl_config_set_key_mem(struct ressl_config *config, char *key, size_t len) { - config->key_mem = key; - config->key_len = len; + SET_MEM(config, key_mem, key_len, key, len); + return 0; } void Index: ressl_internal.h =================================================================== RCS file: /cvs/src/lib/libressl/ressl_internal.h,v retrieving revision 1.10 diff -u -p -r1.10 ressl_internal.h --- ressl_internal.h 27 Aug 2014 10:46:53 -0000 1.10 +++ ressl_internal.h 10 Sep 2014 20:27:51 -0000 @@ -26,14 +26,14 @@ #define _PATH_SSL_CA_FILE "/etc/ssl/cert.pem" struct ressl_config { - const char *ca_file; - const char *ca_path; - const char *cert_file; + char *ca_file; + char *ca_path; + char *cert_file; char *cert_mem; size_t cert_len; - const char *ciphers; + char *ciphers; int ecdhcurve; - const char *key_file; + char *key_file; char *key_mem; size_t key_len; int verify;