From 29c5fa029c510914939f1d59c778cc1d6370dac6 Mon Sep 17 00:00:00 2001
From: Karel Slany <karel.slany@nic.cz>
Date: Mon, 25 Jul 2016 15:26:47 +0200
Subject: [PATCH] Added checks when accessing algorithm structures in array.
---
lib/cookies/alg_containers.c | 42 +++++++++++++++++----
lib/cookies/alg_containers.h | 18 +++++++--
lib/cookies/helper.c | 9 +++--
modules/cookies/cookiemonster.c | 65 ++++++++++++++++++---------------
4 files changed, 90 insertions(+), 44 deletions(-)
diff --git a/lib/cookies/alg_containers.c b/lib/cookies/alg_containers.c
index 9c6433884..e5326c9f4 100644
--- a/lib/cookies/alg_containers.c
+++ b/lib/cookies/alg_containers.c
@@ -23,10 +23,23 @@
#include "lib/cookies/alg_containers.h"
#include "lib/cookies/alg_sha.h"
-const struct knot_cc_alg *const kr_cc_algs[] = {
- /* 0 */ &knot_cc_alg_fnv64,
- /* 1 */ &knot_cc_alg_hmac_sha256_64
-};
+const struct knot_cc_alg *kr_cc_alg_get(int id)
+{
+ /*
+ * Client algorithm identifiers are used to index this array of
+ * pointers.
+ */
+ static const struct knot_cc_alg *const cc_algs[] = {
+ /* 0 */ &knot_cc_alg_fnv64,
+ /* 1 */ &knot_cc_alg_hmac_sha256_64
+ };
+
+ if (id >= 0 && id < 2) {
+ return cc_algs[id];
+ }
+
+ return NULL;
+}
const knot_lookup_t kr_cc_alg_names[] = {
{ 0, "FNV-64" },
@@ -34,10 +47,23 @@ const knot_lookup_t kr_cc_alg_names[] = {
{ -1, NULL }
};
-const struct knot_sc_alg *const kr_sc_algs[] = {
- /* 0 */ &knot_sc_alg_fnv64,
- /* 1 */ &knot_sc_alg_hmac_sha256_64
-};
+const struct knot_sc_alg *kr_sc_alg_get(int id)
+{
+ /*
+ * Server algorithm identifiers are used to index this array of
+ * pointers.
+ */
+ static const struct knot_sc_alg *const sc_algs[] = {
+ /* 0 */ &knot_sc_alg_fnv64,
+ /* 1 */ &knot_sc_alg_hmac_sha256_64
+ };
+
+ if (id >= 0 && id < 2) {
+ return sc_algs[id];
+ }
+
+ return NULL;
+}
const knot_lookup_t kr_sc_alg_names[] = {
{ 0, "FNV-64" },
diff --git a/lib/cookies/alg_containers.h b/lib/cookies/alg_containers.h
index 17e1c5c91..f2aded1e6 100644
--- a/lib/cookies/alg_containers.h
+++ b/lib/cookies/alg_containers.h
@@ -22,17 +22,27 @@
#include "lib/defines.h"
-/** Client algorithm identifiers are used to index this array of pointers. */
+/**
+ * @brief Returns pointer to client cookie algorithm.
+ *
+ * @param id algorithm identifier as defined by lookup table
+ * @return pointer to algorithm structure with given id or NULL on error
+ */
KR_EXPORT
-extern const struct knot_cc_alg *const kr_cc_algs[];
+const struct knot_cc_alg *kr_cc_alg_get(int id);
/** Binds client algorithm identifiers onto names. */
KR_EXPORT
extern const knot_lookup_t kr_cc_alg_names[];
-/** Server algorithm identifiers are used to index this array of pointers. */
+/**
+ * @brief Returns pointer to server cookie algorithm.
+ *
+ * @param id algorithm identifier as defined by lookup table
+ * @return pointer to algorithm structure with given id or NULL on error
+ */
KR_EXPORT
-extern const struct knot_sc_alg *const kr_sc_algs[];
+const struct knot_sc_alg *kr_sc_alg_get(int id);
/** Binds server algorithm identifiers onto names. */
KR_EXPORT
diff --git a/lib/cookies/helper.c b/lib/cookies/helper.c
index af87616d6..00c4ce53e 100644
--- a/lib/cookies/helper.c
+++ b/lib/cookies/helper.c
@@ -135,9 +135,12 @@ int kr_request_put_cookie(const struct kr_cookie_comp *clnt_comp,
};
uint8_t cc[KNOT_OPT_COOKIE_CLNT];
uint16_t cc_len = KNOT_OPT_COOKIE_CLNT;
- assert((clnt_comp->alg_id >= 0) && kr_cc_algs[clnt_comp->alg_id] &&
- kr_cc_algs[clnt_comp->alg_id]->gen_func);
- cc_len = kr_cc_algs[clnt_comp->alg_id]->gen_func(&input, cc, cc_len);
+ const struct knot_cc_alg *cc_alg = kr_cc_alg_get(clnt_comp->alg_id);
+ if (!cc_alg) {
+ return kr_error(EINVAL);
+ }
+ assert(cc_alg->gen_func);
+ cc_len = cc_alg->gen_func(&input, cc, cc_len);
if (cc_len != KNOT_OPT_COOKIE_CLNT) {
return kr_error(EINVAL);
}
diff --git a/modules/cookies/cookiemonster.c b/modules/cookies/cookiemonster.c
index 8097f4102..30e3b0639 100644
--- a/modules/cookies/cookiemonster.c
+++ b/modules/cookies/cookiemonster.c
@@ -116,6 +116,8 @@ static int srvr_sockaddr_cc_check(const struct sockaddr **sockaddr,
const struct sockaddr *tmp_sockaddr = passed_server_sockaddr(req);
+ const struct knot_cc_alg *cc_alg = NULL;
+
/* The address must correspond with the client cookie. */
if (tmp_sockaddr) {
assert(clnt_sett->current.secr);
@@ -126,15 +128,17 @@ static int srvr_sockaddr_cc_check(const struct sockaddr **sockaddr,
.secret_data = clnt_sett->current.secr->data,
.secret_len = clnt_sett->current.secr->size
};
- int ret = knot_cc_check(cc, cc_len, &input,
- kr_cc_algs[clnt_sett->current.alg_id]);
+ cc_alg = kr_cc_alg_get(clnt_sett->current.alg_id);
+ if (!cc_alg) {
+ kr_error(EINVAL);
+ }
+ int ret = knot_cc_check(cc, cc_len, &input, cc_alg);
bool have_current = (ret == KNOT_EOK);
- if ((ret != KNOT_EOK) &&
- clnt_sett->recent.secr && (clnt_sett->recent.alg_id >= 0)) {
+ cc_alg = kr_cc_alg_get(clnt_sett->recent.alg_id);
+ if ((ret != KNOT_EOK) && clnt_sett->recent.secr && cc_alg) {
input.secret_data = clnt_sett->recent.secr->data;
input.secret_len = clnt_sett->recent.secr->size;
- ret = knot_cc_check(cc, cc_len, &input,
- kr_cc_algs[clnt_sett->recent.alg_id]);
+ ret = knot_cc_check(cc, cc_len, &input, cc_alg);
}
if (ret == KNOT_EOK) {
*sockaddr = tmp_sockaddr;
@@ -147,17 +151,20 @@ static int srvr_sockaddr_cc_check(const struct sockaddr **sockaddr,
"guessing response address from ns reputation");
/* Abusing name server reputation mechanism to guess IP addresses. */
+ cc_alg = kr_cc_alg_get(clnt_sett->current.alg_id);
+ if (!cc_alg) {
+ kr_error(EINVAL);
+ }
const struct kr_nsrep *ns = &qry->ns;
tmp_sockaddr = guess_server_addr(ns, cc, cc_len,
- clnt_sett->current.secr,
- kr_cc_algs[clnt_sett->current.alg_id]);
+ clnt_sett->current.secr, cc_alg);
bool have_current = (tmp_sockaddr != NULL);
- if (!tmp_sockaddr &&
- clnt_sett->recent.secr && (clnt_sett->recent.alg_id >= 0)) {
+ cc_alg = kr_cc_alg_get(clnt_sett->recent.alg_id);
+ if (!tmp_sockaddr && clnt_sett->recent.secr && cc_alg) {
/* Try recent client secret to check obtained cookie. */
tmp_sockaddr = guess_server_addr(ns, cc, cc_len,
clnt_sett->recent.secr,
- kr_cc_algs[clnt_sett->recent.alg_id]);
+ cc_alg);
}
if (tmp_sockaddr) {
*sockaddr = tmp_sockaddr;
@@ -385,9 +392,10 @@ int check_request(knot_layer_t *ctx, void *module_param)
bool ignore_badcookie = true; /* TODO -- Occasionally ignore? */
- if (!req->qsource.addr ||
- !srvr_sett->current.secr || (srvr_sett->current.alg_id < 0)) {
- DEBUG_MSG(NULL, "%s\n", "missing server cookie context");
+ const struct knot_sc_alg *current_sc_alg = kr_sc_alg_get(srvr_sett->current.alg_id);
+
+ if (!req->qsource.addr || !srvr_sett->current.secr || !current_sc_alg) {
+ DEBUG_MSG(NULL, "%s\n", "missing valid server cookie context");
return KNOT_STATE_FAIL;
}
@@ -428,18 +436,19 @@ int check_request(knot_layer_t *ctx, void *module_param)
/* Check server cookie obtained in request. */
- ret = knot_sc_check(KR_NONCE_LEN, &cookies, &srvr_data,
- kr_sc_algs[srvr_sett->current.alg_id]);
- if (ret == KNOT_EINVAL &&
- srvr_sett->recent.secr && (srvr_sett->recent.alg_id >= 0)) {
- /* Try recent algorithm. */
- struct knot_sc_private recent_srvr_data = {
- .clnt_sockaddr = req->qsource.addr,
- .secret_data = srvr_sett->recent.secr->data,
- .secret_len = srvr_sett->recent.secr->size
- };
- ret = knot_sc_check(KR_NONCE_LEN, &cookies, &recent_srvr_data,
- kr_sc_algs[srvr_sett->recent.alg_id]);
+ ret = knot_sc_check(KR_NONCE_LEN, &cookies, &srvr_data, current_sc_alg);
+ if (ret == KNOT_EINVAL && srvr_sett->recent.secr) {
+ const struct knot_sc_alg *recent_sc_alg = kr_sc_alg_get(srvr_sett->recent.alg_id);
+ if (recent_sc_alg) {
+ /* Try recent algorithm. */
+ struct knot_sc_private recent_srvr_data = {
+ .clnt_sockaddr = req->qsource.addr,
+ .secret_data = srvr_sett->recent.secr->data,
+ .secret_len = srvr_sett->recent.secr->size
+ };
+ ret = knot_sc_check(KR_NONCE_LEN, &cookies,
+ &recent_srvr_data, recent_sc_alg);
+ }
}
if (ret != KNOT_EOK) {
/* Invalid server cookie. */
@@ -467,9 +476,7 @@ int check_request(knot_layer_t *ctx, void *module_param)
answer_add_cookies:
/* Add server cookie into response. */
ret = kr_answer_write_cookie(&srvr_data, cookies.cc, cookies.cc_len,
- &nonce,
- kr_sc_algs[srvr_sett->current.alg_id],
- answer);
+ &nonce, current_sc_alg, answer);
if (ret != kr_ok()) {
return_state = KNOT_STATE_FAIL;
}
--
GitLab