From 66271edaea40e861dbd498eb3b372e61c0f52ead Mon Sep 17 00:00:00 2001 From: Andy Beverley Date: Sat, 3 Feb 2024 20:30:49 +0000 Subject: [PATCH 1/7] Fix verification when key specifies hash algorithm --- libopenarc/arc.c | 60 ++++++++++++++++++++++++++++++++++++++++-------- libopenarc/arc.h | 1 + 2 files changed, 51 insertions(+), 10 deletions(-) diff --git a/libopenarc/arc.c b/libopenarc/arc.c index 1c6c9ded..e3be03cd 100644 --- a/libopenarc/arc.c +++ b/libopenarc/arc.c @@ -289,6 +289,48 @@ arc_key_hashesok(ARC_LIB *lib, u_char *hashlist) /* NOTREACHED */ } +/* +** ARC_PARSE_ALGORITHM -- parse an algorithm and set the msg hash and key, +** as well as set the message digest algorithm for +** RSA_verify in the variable nid +** +** Parameters: +** msg -- ARC_MESSAGE handle +** alg -- string containing the algorithm to parse +** nid -- variable to write the message digest algorithm +** +** Return value: +** An ARC_STAT_* constant. +*/ + +ARC_STAT +arc_parse_algorithm(ARC_MESSAGE *msg, u_char *alg, int *nid) +{ + arc_alg_t algtype; + + algtype = arc_name_to_code(algorithms, alg); + + if (algtype == ARC_SIGN_RSASHA1) + { + msg->arc_hashtype = ARC_HASHTYPE_SHA1; + msg->arc_keytype = ARC_KEYTYPE_RSA; + *nid = NID_sha1; + } + else if (algtype == ARC_SIGN_RSASHA256) + { + msg->arc_hashtype = ARC_HASHTYPE_SHA256; + msg->arc_keytype = ARC_KEYTYPE_RSA; + *nid = NID_sha256; + } + else + { + arc_error(msg, "unknown or invalid algorithm"); + return ARC_STAT_BADALG; + } + + return ARC_STAT_OK; +} + /* ** ARC_GENAMSHDR -- generate a signature or seal header field ** @@ -1965,6 +2007,10 @@ arc_validate_msg(ARC_MESSAGE *msg, u_int setnum) msg->arc_selector = arc_param_get(kvset, "s"); msg->arc_domain = arc_param_get(kvset, "d"); + /* store algorithm in msg, needed for arc_get_key() */ + alg = arc_param_get(kvset, "a"); + arc_parse_algorithm(msg, alg, &nid); + /* get the key from DNS (or wherever) */ status = arc_get_key(msg, FALSE); if (status != ARC_STAT_OK) @@ -2039,11 +2085,6 @@ arc_validate_msg(ARC_MESSAGE *msg, u_int setnum) return ARC_STAT_CANTVRFY; } - alg = arc_param_get(kvset, "a"); - nid = NID_sha1; - if (alg != NULL && strcmp(alg, "rsa-sha256") == 0) - nid = NID_sha256; - rsastat = RSA_verify(nid, hh, hhlen, sig, siglen, rsa); RSA_free(rsa); @@ -2119,6 +2160,10 @@ arc_validate_seal(ARC_MESSAGE *msg, u_int setnum) msg->arc_selector = arc_param_get(kvset, "s"); msg->arc_domain = arc_param_get(kvset, "d"); + /* store algorithm in msg, needed for arc_get_key() */ + alg = arc_param_get(kvset, "a"); + arc_parse_algorithm(msg, alg, &nid); + if (msg->arc_selector == NULL) { arc_error(msg, "seal at i=%u has no selector", setnum); @@ -2191,11 +2236,6 @@ arc_validate_seal(ARC_MESSAGE *msg, u_int setnum) return ARC_STAT_INTERNAL; } - alg = arc_param_get(kvset, "a"); - nid = NID_sha1; - if (alg != NULL && strcmp(alg, "rsa-sha256") == 0) - nid = NID_sha256; - rsastat = RSA_verify(nid, sh, shlen, sig, siglen, rsa); RSA_free(rsa); diff --git a/libopenarc/arc.h b/libopenarc/arc.h index 34cf0736..beeaf346 100644 --- a/libopenarc/arc.h +++ b/libopenarc/arc.h @@ -81,6 +81,7 @@ typedef int ARC_STAT; #define ARC_STAT_KEYFAIL 11 /* key retrieval failed */ #define ARC_STAT_MULTIDNSREPLY 12 /* multiple DNS replies */ #define ARC_STAT_SIGGEN 13 /* seal generation failed */ +#define ARC_STAT_BADALG 14 /* unknown or invalid algorithm */ /* ** ARC_CHAIN -- chain state From 8323d348d7bc7da71b94eacd1186d1ba8417f705 Mon Sep 17 00:00:00 2001 From: Andy Beverley Date: Sun, 4 Feb 2024 12:04:53 +0000 Subject: [PATCH 2/7] Sanity check inputs to arc_parse_algorithm --- libopenarc/arc.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libopenarc/arc.c b/libopenarc/arc.c index e3be03cd..9d97368d 100644 --- a/libopenarc/arc.c +++ b/libopenarc/arc.c @@ -308,6 +308,9 @@ arc_parse_algorithm(ARC_MESSAGE *msg, u_char *alg, int *nid) { arc_alg_t algtype; + assert(msg != NULL); + assert(alg != NULL); + algtype = arc_name_to_code(algorithms, alg); if (algtype == ARC_SIGN_RSASHA1) From 00345b18f40a926c408a6e5750596d4543b4c849 Mon Sep 17 00:00:00 2001 From: Andy Beverley Date: Sun, 4 Feb 2024 12:10:15 +0000 Subject: [PATCH 3/7] Check nid is correctly defined after algorithm parse --- libopenarc/arc.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/libopenarc/arc.c b/libopenarc/arc.c index 9d97368d..63467eac 100644 --- a/libopenarc/arc.c +++ b/libopenarc/arc.c @@ -2013,6 +2013,11 @@ arc_validate_msg(ARC_MESSAGE *msg, u_int setnum) /* store algorithm in msg, needed for arc_get_key() */ alg = arc_param_get(kvset, "a"); arc_parse_algorithm(msg, alg, &nid); + if (nid == NULL) + { + arc_error(msg, "hash algorithm not correctly retrieved for algorithm %s", alg); + return ARC_STAT_BADALG; + } /* get the key from DNS (or wherever) */ status = arc_get_key(msg, FALSE); @@ -2166,6 +2171,11 @@ arc_validate_seal(ARC_MESSAGE *msg, u_int setnum) /* store algorithm in msg, needed for arc_get_key() */ alg = arc_param_get(kvset, "a"); arc_parse_algorithm(msg, alg, &nid); + if (nid == NULL) + { + arc_error(msg, "hash algorithm not correctly retrieved for algorithm %s", alg); + return ARC_STAT_BADALG; + } if (msg->arc_selector == NULL) { From 6be659f8e33dff517f9b6f06983063b29b4f0325 Mon Sep 17 00:00:00 2001 From: Andy Beverley Date: Sun, 4 Feb 2024 18:45:09 +0000 Subject: [PATCH 4/7] Handle missing algorithm gracefully --- libopenarc/arc.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/libopenarc/arc.c b/libopenarc/arc.c index 63467eac..0840ae22 100644 --- a/libopenarc/arc.c +++ b/libopenarc/arc.c @@ -309,7 +309,12 @@ arc_parse_algorithm(ARC_MESSAGE *msg, u_char *alg, int *nid) arc_alg_t algtype; assert(msg != NULL); - assert(alg != NULL); + + if (alg == NULL) + { + arc_error(msg, "missing algorithm passed to arc_parse_algorithm"); + return ARC_STAT_BADALG; + } algtype = arc_name_to_code(algorithms, alg); From 8499dc645467b93a99df1acd92f54f15511f8b8e Mon Sep 17 00:00:00 2001 From: Andy Beverley Date: Sun, 4 Feb 2024 18:46:36 +0000 Subject: [PATCH 5/7] Check status from arc_parse_algorithm --- libopenarc/arc.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/libopenarc/arc.c b/libopenarc/arc.c index 0840ae22..59f2d969 100644 --- a/libopenarc/arc.c +++ b/libopenarc/arc.c @@ -2017,8 +2017,8 @@ arc_validate_msg(ARC_MESSAGE *msg, u_int setnum) /* store algorithm in msg, needed for arc_get_key() */ alg = arc_param_get(kvset, "a"); - arc_parse_algorithm(msg, alg, &nid); - if (nid == NULL) + status = arc_parse_algorithm(msg, alg, &nid); + if (status != ARC_STAT_OK) { arc_error(msg, "hash algorithm not correctly retrieved for algorithm %s", alg); return ARC_STAT_BADALG; @@ -2175,8 +2175,8 @@ arc_validate_seal(ARC_MESSAGE *msg, u_int setnum) /* store algorithm in msg, needed for arc_get_key() */ alg = arc_param_get(kvset, "a"); - arc_parse_algorithm(msg, alg, &nid); - if (nid == NULL) + status = arc_parse_algorithm(msg, alg, &nid); + if (status != ARC_STAT_OK) { arc_error(msg, "hash algorithm not correctly retrieved for algorithm %s", alg); return ARC_STAT_BADALG; From 6edc760c4d2bc0d0d2cb74cb3371dbf39e340fe7 Mon Sep 17 00:00:00 2001 From: Andy Beverley Date: Mon, 5 Feb 2024 07:50:08 +0000 Subject: [PATCH 6/7] Simplifed error handling --- libopenarc/arc.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/libopenarc/arc.c b/libopenarc/arc.c index 59f2d969..cca16c2d 100644 --- a/libopenarc/arc.c +++ b/libopenarc/arc.c @@ -332,7 +332,7 @@ arc_parse_algorithm(ARC_MESSAGE *msg, u_char *alg, int *nid) } else { - arc_error(msg, "unknown or invalid algorithm"); + arc_error(msg, "unknown or invalid algorithm: %s", alg); return ARC_STAT_BADALG; } @@ -2019,10 +2019,8 @@ arc_validate_msg(ARC_MESSAGE *msg, u_int setnum) alg = arc_param_get(kvset, "a"); status = arc_parse_algorithm(msg, alg, &nid); if (status != ARC_STAT_OK) - { - arc_error(msg, "hash algorithm not correctly retrieved for algorithm %s", alg); - return ARC_STAT_BADALG; - } + // arc_error already set by arc_parse_algorithm() + return status; /* get the key from DNS (or wherever) */ status = arc_get_key(msg, FALSE); @@ -2177,10 +2175,8 @@ arc_validate_seal(ARC_MESSAGE *msg, u_int setnum) alg = arc_param_get(kvset, "a"); status = arc_parse_algorithm(msg, alg, &nid); if (status != ARC_STAT_OK) - { - arc_error(msg, "hash algorithm not correctly retrieved for algorithm %s", alg); - return ARC_STAT_BADALG; - } + // arc_error already set by arc_parse_algorithm() + return status; if (msg->arc_selector == NULL) { From 043a8d15b5b457033ecb403a5408539249a8ba95 Mon Sep 17 00:00:00 2001 From: Andy Beverley Date: Mon, 5 Feb 2024 07:52:14 +0000 Subject: [PATCH 7/7] Add assert for function input --- libopenarc/arc.c | 1 + 1 file changed, 1 insertion(+) diff --git a/libopenarc/arc.c b/libopenarc/arc.c index cca16c2d..4abcdade 100644 --- a/libopenarc/arc.c +++ b/libopenarc/arc.c @@ -309,6 +309,7 @@ arc_parse_algorithm(ARC_MESSAGE *msg, u_char *alg, int *nid) arc_alg_t algtype; assert(msg != NULL); + assert(nid != NULL); if (alg == NULL) {