From 5decd4971329fded1575e5a6c27efe1680843d64 Mon Sep 17 00:00:00 2001 From: Oliver Smith Date: Thu, 22 Dec 2022 17:06:22 +0100 Subject: [PATCH] mslookup: use apn functions from libosmocore Use the apn functions from libosmocore to encode and decode qnames to avoid code duplication and to avoid unnecessary dynamic allocation. The unit tests for encoding and decoding rfc_question / rfc_record are still passing and have the same output. Fixes: OS#5821 Change-Id: I09d3c617fd6eb4075084ee106d3f3c5803861d2f --- include/osmocom/mslookup/mdns_rfc.h | 3 - src/mslookup/mdns_rfc.c | 145 +++++++--------------------- tests/mslookup/mdns_test.c | 143 --------------------------- tests/mslookup/mdns_test.err | 82 ---------------- 4 files changed, 34 insertions(+), 339 deletions(-) diff --git a/include/osmocom/mslookup/mdns_rfc.h b/include/osmocom/mslookup/mdns_rfc.h index 9d6be5a2..4aaee962 100644 --- a/include/osmocom/mslookup/mdns_rfc.h +++ b/include/osmocom/mslookup/mdns_rfc.h @@ -99,9 +99,6 @@ struct osmo_mdns_rfc_record { uint8_t *rdata; }; -char *osmo_mdns_rfc_qname_encode(void *ctx, const char *domain); -char *osmo_mdns_rfc_qname_decode(void *ctx, const char *qname, size_t qname_len); - void osmo_mdns_rfc_header_encode(struct msgb *msg, const struct osmo_mdns_rfc_header *hdr); int osmo_mdns_rfc_header_decode(const uint8_t *data, size_t data_len, struct osmo_mdns_rfc_header *hdr); diff --git a/src/mslookup/mdns_rfc.c b/src/mslookup/mdns_rfc.c index eddba30d..7bfd3280 100644 --- a/src/mslookup/mdns_rfc.c +++ b/src/mslookup/mdns_rfc.c @@ -27,91 +27,9 @@ #include #include #include +#include #include -/* - * Encode/decode IEs - */ - -/*! Encode a domain string as qname (RFC 1035 4.1.2). - * \param[in] domain multiple labels separated by dots, e.g. "sip.voice.1234.msisdn". - * \returns allocated buffer with length-value pairs for each label (e.g. 0x03 "sip" 0x05 "voice" ...), NULL on error. - */ -char *osmo_mdns_rfc_qname_encode(void *ctx, const char *domain) -{ - char *domain_dup; - char *domain_iter; - char buf[OSMO_MDNS_RFC_MAX_NAME_LEN + 2] = ""; /* len(qname) is len(domain) +1 */ - struct osmo_strbuf sb = { .buf = buf, .len = sizeof(buf) }; - char *label; - - if (strlen(domain) > OSMO_MDNS_RFC_MAX_NAME_LEN) - return NULL; - - domain_iter = domain_dup = talloc_strdup(ctx, domain); - while ((label = strsep(&domain_iter, "."))) { - size_t len = strlen(label); - - /* Empty domain, dot at start, two dots in a row, or ending with a dot */ - if (!len) - goto error; - - OSMO_STRBUF_PRINTF(sb, "%c%s", (char)len, label); - } - - talloc_free(domain_dup); - return talloc_strdup(ctx, buf); - -error: - talloc_free(domain_dup); - return NULL; -} - -/*! Decode a domain string from a qname (RFC 1035 4.1.2). - * \param[in] qname buffer with length-value pairs for each label (e.g. 0x03 "sip" 0x05 "voice" ...) - * \param[in] qname_max_len amount of bytes that can be read at most from the memory location that qname points to. - * \returns allocated buffer with domain string, multiple labels separated by dots (e.g. "sip.voice.1234.msisdn"), - * NULL on error. - */ -char *osmo_mdns_rfc_qname_decode(void *ctx, const char *qname, size_t qname_max_len) -{ - const char *next_label, *qname_end = qname + qname_max_len; - char buf[OSMO_MDNS_RFC_MAX_NAME_LEN + 1]; - int i = 0; - - if (qname_max_len < 1) - return NULL; - - while (*qname) { - size_t len; - - if (i >= qname_max_len) - return NULL; - - len = *qname; - next_label = qname + len + 1; - - if (next_label >= qname_end || i + len > OSMO_MDNS_RFC_MAX_NAME_LEN) - return NULL; - - if (i) { - /* Two dots in a row is not allowed */ - if (buf[i - 1] == '.') - return NULL; - - buf[i] = '.'; - i++; - } - - memcpy(buf + i, qname + 1, len); - i += len; - qname = next_label; - } - buf[i] = '\0'; - - return talloc_strdup(ctx, buf); -} - /* * Encode/decode message sections */ @@ -153,18 +71,15 @@ int osmo_mdns_rfc_header_decode(const uint8_t *data, size_t data_len, struct osm */ int osmo_mdns_rfc_question_encode(void *ctx, struct msgb *msg, const struct osmo_mdns_rfc_question *qst) { - char *qname; - size_t qname_len; - uint8_t *qname_buf; + uint8_t *buf; + size_t buf_len; /* qname */ - qname = osmo_mdns_rfc_qname_encode(ctx, qst->domain); - if (!qname) + buf_len = strlen(qst->domain) + 1; + buf = msgb_put(msg, buf_len); + if (osmo_apn_from_str(buf, buf_len, qst->domain) < 0) return -EINVAL; - qname_len = strlen(qname) + 1; - qname_buf = msgb_put(msg, qname_len); - memcpy(qname_buf, qname, qname_len); - talloc_free(qname); + msgb_put_u8(msg, 0x00); /* qtype and qclass */ msgb_put_u16(msg, qst->qtype); @@ -182,21 +97,25 @@ struct osmo_mdns_rfc_question *osmo_mdns_rfc_question_decode(void *ctx, const ui if (data_len < 6) return NULL; - /* qname */ ret = talloc_zero(ctx, struct osmo_mdns_rfc_question); if (!ret) return NULL; - ret->domain = osmo_mdns_rfc_qname_decode(ret, (const char *)data, qname_len); - if (!ret->domain) { - talloc_free(ret); - return NULL; - } + + /* qname */ + ret->domain = talloc_size(ret, qname_len - 1); + if (!ret->domain) + goto error; + if (!osmo_apn_to_str(ret->domain, data, qname_len - 1)) + goto error; /* qtype and qclass */ ret->qtype = osmo_load16be(data + qname_len); ret->qclass = osmo_load16be(data + qname_len + 2); return ret; +error: + talloc_free(ret); + return NULL; } /* @@ -208,18 +127,15 @@ struct osmo_mdns_rfc_question *osmo_mdns_rfc_question_decode(void *ctx, const ui */ int osmo_mdns_rfc_record_encode(void *ctx, struct msgb *msg, const struct osmo_mdns_rfc_record *rec) { - char *name; - size_t name_len; uint8_t *buf; + size_t buf_len; /* name */ - name = osmo_mdns_rfc_qname_encode(ctx, rec->domain); - if (!name) + buf_len = strlen(rec->domain) + 1; + buf = msgb_put(msg, buf_len); + if (osmo_apn_from_str(buf, buf_len, rec->domain) < 0) return -EINVAL; - name_len = strlen(name) + 1; - buf = msgb_put(msg, name_len); - memcpy(buf, name, name_len); - talloc_free(name); + msgb_put_u8(msg, 0x00); /* type, class, ttl, rdlength */ msgb_put_u16(msg, rec->type); @@ -240,16 +156,23 @@ struct osmo_mdns_rfc_record *osmo_mdns_rfc_record_decode(void *ctx, const uint8_ struct osmo_mdns_rfc_record *ret; size_t name_len; + /* name length: represented as a series of labels, and terminated by a + * label with zero length (RFC 1035 3.3). A label with zero length is a + * NUL byte. */ + name_len = strnlen((const char *)data, data_len - 10) + 1; + if (data[name_len]) + return NULL; + + /* allocate ret + ret->domain */ ret = talloc_zero(ctx, struct osmo_mdns_rfc_record); if (!ret) return NULL; - - /* name */ - ret->domain = osmo_mdns_rfc_qname_decode(ret, (const char *)data, data_len - 10); + ret->domain = talloc_size(ctx, name_len - 1); if (!ret->domain) goto error; - name_len = strlen(ret->domain) + 2; - if (name_len + 10 > data_len) + + /* name */ + if (!osmo_apn_to_str(ret->domain, data, name_len - 1)) goto error; /* type, class, ttl, rdlength */ diff --git a/tests/mslookup/mdns_test.c b/tests/mslookup/mdns_test.c index 7b26f0f3..3e5a2306 100644 --- a/tests/mslookup/mdns_test.c +++ b/tests/mslookup/mdns_test.c @@ -32,148 +32,6 @@ struct qname_enc_dec_test { size_t qname_max_len; /* default: strlen(qname) + 1 */ }; -static const struct qname_enc_dec_test qname_enc_dec_test_data[] = { - { - /* OK: typical mslookup domain */ - .domain = "hlr.1234567.imsi", - .qname = "\x03" "hlr" "\x07" "1234567" "\x04" "imsi", - }, - { - /* Wrong format: double dot */ - .domain = "hlr..imsi", - .qname = NULL, - }, - { - /* Wrong format: double dot */ - .domain = "hlr", - .qname = "\x03hlr\0\x03imsi", - }, - { - /* Wrong format: dot at end */ - .domain = "hlr.", - .qname = NULL, - }, - { - /* Wrong format: dot at start */ - .domain = ".hlr", - .qname = NULL, - }, - { - /* Wrong format: empty */ - .domain = "", - .qname = NULL, - }, - { - /* OK: maximum length */ - .domain = - "123456789." "123456789." "123456789." "123456789." "123456789." - "123456789." "123456789." "123456789." "123456789." "123456789." - "123456789." "123456789." "123456789." "123456789." "123456789." - "123456789." "123456789." "123456789." "123456789." "123456789." - "123456789." "123456789." "123456789." "123456789." "123456789." - "12345" - , - .qname = - "\t123456789\t123456789\t123456789\t123456789\t123456789" - "\t123456789\t123456789\t123456789\t123456789\t123456789" - "\t123456789\t123456789\t123456789\t123456789\t123456789" - "\t123456789\t123456789\t123456789\t123456789\t123456789" - "\t123456789\t123456789\t123456789\t123456789\t123456789" - "\x05" "12345" - }, - { - /* Error: too long domain */ - .domain = - "123456789." "123456789." "123456789." "123456789." "123456789." - "123456789." "123456789." "123456789." "123456789." "123456789." - "123456789." "123456789." "123456789." "123456789." "123456789." - "123456789." "123456789." "123456789." "123456789." "123456789." - "123456789." "123456789." "123456789." "123456789." "123456789." - "12345toolong" - , - .qname = NULL, - }, - { - /* Error: too long qname */ - .domain = NULL, - .qname = - "\t123456789\t123456789\t123456789\t123456789\t123456789" - "\t123456789\t123456789\t123456789\t123456789\t123456789" - "\t123456789\t123456789\t123456789\t123456789\t123456789" - "\t123456789\t123456789\t123456789\t123456789\t123456789" - "\t123456789\t123456789\t123456789\t123456789\t123456789" - "\t123456789\t123456789\t123456789\t123456789\t123456789" - }, - { - /* Error: wrong token length in qname */ - .domain = NULL, - .qname = "\x03" "hlr" "\x07" "1234567" "\x05" "imsi", - }, - { - /* Error: wrong token length in qname */ - .domain = NULL, - .qname = "\x02" "hlr" "\x07" "1234567" "\x04" "imsi", - }, - { - /* Wrong format: token length at end of qname */ - .domain = NULL, - .qname = "\x03hlr\x03", - }, - { - /* Error: overflow in label length */ - .domain = NULL, - .qname = "\x03" "hlr" "\x07" "1234567" "\x04" "imsi", - .qname_max_len = 17, - }, -}; - -void test_enc_dec_rfc_qname(void *ctx) -{ - char quote_buf[300]; - int i; - - fprintf(stderr, "-- %s --\n", __func__); - - for (i = 0; i < ARRAY_SIZE(qname_enc_dec_test_data); i++) { - const struct qname_enc_dec_test *t = &qname_enc_dec_test_data[i]; - char *res; - - if (t->domain) { - fprintf(stderr, "domain: %s\n", osmo_quote_str_buf2(quote_buf, sizeof(quote_buf), t->domain, -1)); - fprintf(stderr, "exp: %s\n", osmo_quote_str_buf2(quote_buf, sizeof(quote_buf), t->qname, -1)); - res = osmo_mdns_rfc_qname_encode(ctx, t->domain); - fprintf(stderr, "res: %s\n", osmo_quote_str_buf2(quote_buf, sizeof(quote_buf), res, -1)); - if (t->qname == res || (t->qname && res && strcmp(t->qname, res) == 0)) - fprintf(stderr, "=> OK\n"); - else - fprintf(stderr, "=> ERROR\n"); - if (res) - talloc_free(res); - fprintf(stderr, "\n"); - } - - if (t->qname) { - size_t qname_max_len = t->qname_max_len; - if (qname_max_len) - fprintf(stderr, "qname_max_len: %lu\n", qname_max_len); - else - qname_max_len = strlen(t->qname) + 1; - - fprintf(stderr, "qname: %s\n", osmo_quote_str_buf2(quote_buf, sizeof(quote_buf), t->qname, -1)); - fprintf(stderr, "exp: %s\n", osmo_quote_str_buf2(quote_buf, sizeof(quote_buf), t->domain, -1)); - res = osmo_mdns_rfc_qname_decode(ctx, t->qname, qname_max_len); - fprintf(stderr, "res: %s\n", osmo_quote_str_buf2(quote_buf, sizeof(quote_buf), res, -1)); - if (t->domain == res || (t->domain && res && strcmp(t->domain, res) == 0)) - fprintf(stderr, "=> OK\n"); - else - fprintf(stderr, "=> ERROR\n"); - if (res) - talloc_free(res); - fprintf(stderr, "\n"); - } - } -} - #define PRINT_HDR(hdr, name) \ fprintf(stderr, "header %s:\n" \ ".id = %i\n" \ @@ -589,7 +447,6 @@ int main(int argc, char **argv) log_set_print_category_hex(osmo_stderr_target, 0); log_set_use_color(osmo_stderr_target, 0); - test_enc_dec_rfc_qname(ctx); test_enc_dec_rfc_header(); test_enc_dec_rfc_header_einval(); test_enc_dec_rfc_question(ctx); diff --git a/tests/mslookup/mdns_test.err b/tests/mslookup/mdns_test.err index 51e5afe6..0d650be2 100644 --- a/tests/mslookup/mdns_test.err +++ b/tests/mslookup/mdns_test.err @@ -1,85 +1,3 @@ --- test_enc_dec_rfc_qname -- -domain: "hlr.1234567.imsi" -exp: "\3hlr\a1234567\4imsi" -res: "\3hlr\a1234567\4imsi" -=> OK - -qname: "\3hlr\a1234567\4imsi" -exp: "hlr.1234567.imsi" -res: "hlr.1234567.imsi" -=> OK - -domain: "hlr..imsi" -exp: NULL -res: NULL -=> OK - -domain: "hlr" -exp: "\3hlr" -res: "\3hlr" -=> OK - -qname: "\3hlr" -exp: "hlr" -res: "hlr" -=> OK - -domain: "hlr." -exp: NULL -res: NULL -=> OK - -domain: ".hlr" -exp: NULL -res: NULL -=> OK - -domain: "" -exp: NULL -res: NULL -=> OK - -domain: "123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.12345" -exp: "\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\512345" -res: "\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\512345" -=> OK - -qname: "\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\512345" -exp: "123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.12345" -res: "123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.12345" -=> OK - -domain: "123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.123456789.12345toolong" -exp: NULL -res: NULL -=> OK - -qname: "\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\t123456789\ -exp: NULL -res: NULL -=> OK - -qname: "\3hlr\a1234567\5imsi" -exp: NULL -res: NULL -=> OK - -qname: "\2hlr\a1234567\4imsi" -exp: NULL -res: NULL -=> OK - -qname: "\3hlr\3" -exp: NULL -res: NULL -=> OK - -qname_max_len: 17 -qname: "\3hlr\a1234567\4imsi" -exp: NULL -res: NULL -=> OK - -- test_enc_dec_rfc_header -- header in: .id = 1337