From 4cdb302fef1f2160b74acbe1739f62c713fd99f5 Mon Sep 17 00:00:00 2001 From: Richard Levitte Date: Wed, 19 Jun 2019 08:37:19 +0200 Subject: [PATCH] Add extra error text in the property parser With properties being specified in all kinds of places, including hard coded in providers, it's not always easy to figure out exactly what string was incorrect when the parser would just say something like 'parse failed' with no more details. Adding extra data to the error, showing exactly what string is incorrect, helps a bit. At the very least, this gives anyone interested something to grep for. Reviewed-by: Paul Dale (Merged from https://github.com/openssl/openssl/pull/9190) --- crypto/property/property_parse.c | 44 +++++++++++++++++++++++--------- 1 file changed, 32 insertions(+), 12 deletions(-) diff --git a/crypto/property/property_parse.c b/crypto/property/property_parse.c index c63a433ddbe2d..0b4dcfc8aac4c 100644 --- a/crypto/property/property_parse.c +++ b/crypto/property/property_parse.c @@ -92,6 +92,7 @@ static int parse_name(OPENSSL_CTX *ctx, const char *t[], int create, for (;;) { if (!ossl_isalpha(*s)) { PROPerr(PROP_F_PARSE_NAME, PROP_R_NOT_AN_IDENTIFIER); + ERR_add_error_data(2, "HERE-->", *t); return 0; } do { @@ -110,13 +111,14 @@ static int parse_name(OPENSSL_CTX *ctx, const char *t[], int create, s++; } name[i] = '\0'; - *t = skip_space(s); - if (!err) { - *idx = ossl_property_name(ctx, name, user_name && create); - return 1; + if (err) { + PROPerr(PROP_F_PARSE_NAME, PROP_R_NAME_TOO_LONG); + ERR_add_error_data(2, "HERE-->", *t); + return 0; } - PROPerr(PROP_F_PARSE_NAME, PROP_R_NAME_TOO_LONG); - return 0; + *t = skip_space(s); + *idx = ossl_property_name(ctx, name, user_name && create); + return 1; } static int parse_number(const char *t[], PROPERTY_DEFINITION *res) @@ -131,6 +133,7 @@ static int parse_number(const char *t[], PROPERTY_DEFINITION *res) } while (ossl_isdigit(*s)); if (!ossl_isspace(*s) && *s != '\0' && *s != ',') { PROPerr(PROP_F_PARSE_NUMBER, PROP_R_NOT_A_DECIMAL_DIGIT); + ERR_add_error_data(2, "HERE-->", *t); return 0; } *t = skip_space(s); @@ -155,6 +158,7 @@ static int parse_hex(const char *t[], PROPERTY_DEFINITION *res) } while (ossl_isxdigit(*++s)); if (!ossl_isspace(*s) && *s != '\0' && *s != ',') { PROPerr(PROP_F_PARSE_HEX, PROP_R_NOT_AN_HEXADECIMAL_DIGIT); + ERR_add_error_data(2, "HERE-->", *t); return 0; } *t = skip_space(s); @@ -175,6 +179,7 @@ static int parse_oct(const char *t[], PROPERTY_DEFINITION *res) } while (ossl_isdigit(*++s) && *s != '9' && *s != '8'); if (!ossl_isspace(*s) && *s != '\0' && *s != ',') { PROPerr(PROP_F_PARSE_OCT, PROP_R_NOT_AN_OCTAL_DIGIT); + ERR_add_error_data(2, "HERE-->", *t); return 0; } *t = skip_space(s); @@ -199,16 +204,22 @@ static int parse_string(OPENSSL_CTX *ctx, const char *t[], char delim, s++; } if (*s == '\0') { + char buf[2] = { 0, 0 }; + PROPerr(PROP_F_PARSE_STRING, PROP_R_NO_MATCHING_STRING_DELIMETER); + buf[0] = delim; + ERR_add_error_data(3, "HERE-->", buf, *t); return 0; } v[i] = '\0'; - *t = skip_space(s + 1); - if (err) + if (err) { PROPerr(PROP_F_PARSE_STRING, PROP_R_STRING_TOO_LONG); - else + ERR_add_error_data(2, "HERE-->", *t); + } else { res->v.str_val = ossl_property_value(ctx, v, create); + } + *t = skip_space(s + 1); res->type = PROPERTY_TYPE_STRING; return !err; } @@ -232,14 +243,17 @@ static int parse_unquoted(OPENSSL_CTX *ctx, const char *t[], } if (!ossl_isspace(*s) && *s != '\0' && *s != ',') { PROPerr(PROP_F_PARSE_UNQUOTED, PROP_R_NOT_AN_ASCII_CHARACTER); + ERR_add_error_data(2, "HERE-->", s); return 0; } v[i] = 0; - *t = skip_space(s); - if (err) + if (err) { PROPerr(PROP_F_PARSE_UNQUOTED, PROP_R_STRING_TOO_LONG); - else + ERR_add_error_data(2, "HERE-->", *t); + } else { res->v.str_val = ossl_property_value(ctx, v, create); + } + *t = skip_space(s); res->type = PROPERTY_TYPE_STRING; return !err; } @@ -333,6 +347,8 @@ OSSL_PROPERTY_LIST *ossl_parse_property(OPENSSL_CTX *ctx, const char *defn) s = skip_space(s); done = *s == '\0'; while (!done) { + const char *start = s; + prop = OPENSSL_malloc(sizeof(*prop)); if (prop == NULL) goto err; @@ -343,11 +359,13 @@ OSSL_PROPERTY_LIST *ossl_parse_property(OPENSSL_CTX *ctx, const char *defn) prop->oper = PROPERTY_OPER_EQ; if (prop->name_idx == 0) { PROPerr(PROP_F_OSSL_PARSE_PROPERTY, PROP_R_PARSE_FAILED); + ERR_add_error_data(2, "Unknown name HERE-->", start); goto err; } if (match_ch(&s, '=')) { if (!parse_value(ctx, &s, prop, 1)) { PROPerr(PROP_F_OSSL_PARSE_PROPERTY, PROP_R_NO_VALUE); + ERR_add_error_data(2, "HERE-->", start); goto err; } } else { @@ -363,6 +381,7 @@ OSSL_PROPERTY_LIST *ossl_parse_property(OPENSSL_CTX *ctx, const char *defn) } if (*s != '\0') { PROPerr(PROP_F_OSSL_PARSE_PROPERTY, PROP_R_TRAILING_CHARACTERS); + ERR_add_error_data(2, "HERE-->", s); goto err; } res = stack_to_property_list(sk); @@ -424,6 +443,7 @@ OSSL_PROPERTY_LIST *ossl_parse_query(OPENSSL_CTX *ctx, const char *s) } if (*s != '\0') { PROPerr(PROP_F_OSSL_PARSE_QUERY, PROP_R_TRAILING_CHARACTERS); + ERR_add_error_data(2, "HERE-->", s); goto err; } res = stack_to_property_list(sk);