Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Refactor: use wire-formatted byte arrays to represent names internally, no more strings #90

Open
wants to merge 29 commits into
base: master
Choose a base branch
from
+9,332 −9,586
Open

Refactor: use wire-formatted byte arrays to represent names internally, no more strings #90

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
80ff7b8
src: remove unused and uneeded code
pinheadmz Feb 25, 2022
498e116
dns: change all names in structs and types from strings to uint arrays
pinheadmz Feb 25, 2022
99081df
dns: refactor name_parse(): read wire format and keep in wire format
pinheadmz Feb 25, 2022
9f9f33f
dns: refactor name_serialize(): read and keep in wire format
pinheadmz Feb 25, 2022
081fedd
dns: use dns-name-based hash map for label compression in serialize()
pinheadmz Feb 25, 2022
e7d7c4b
dns: implement to/from string (DNS presentation format) for names
pinheadmz Feb 25, 2022
58fc525
dns: repurpose name_verify() to match hsd rules.verifyName()
pinheadmz Feb 25, 2022
5d12bc9
dns: refactor label split/from/get for byte array instead of string
pinheadmz Feb 25, 2022
7184186
ns: fix synth parsing in onrecv() and refactor for byte arrays
pinheadmz Feb 25, 2022
0dde592
resource: update synth record processing for byte arrays
pinheadmz Feb 25, 2022
15018c0
pool: rename "name" to "tld" and update HNS namehash for byte arrays
pinheadmz Feb 25, 2022
272f216
utils: refactor to_lower for byte arrays
pinheadmz Feb 25, 2022
da4f3bb
cache: goodbye char *name!
pinheadmz Feb 25, 2022
9d011d9
dns: goodbye char *name!
pinheadmz Feb 25, 2022
406cd62
dnssec: goodbye char *name!
pinheadmz Feb 25, 2022
c28676c
ns: goodbye char *name!
pinheadmz Feb 25, 2022
48e9ddd
pool: goodbye char *name!
pinheadmz Feb 25, 2022
0b323c1
req: goodbye char *name!
pinheadmz Feb 25, 2022
40099a9
resource: goodbye char *name!
pinheadmz Feb 25, 2022
62ab53b
rs: goodbye char *name!
pinheadmz Feb 25, 2022
60f7a23
test: cover names-as-arrays-not-strings refactor
pinheadmz Feb 25, 2022
d63f2e1
dns: remove label compression from some record types data
pinheadmz Feb 28, 2022
df6820a
dns: fix max_name overflow and test invalid names read/write
pinheadmz Mar 1, 2022
f47ba8d
dns: prevent memory leak when reading invalid record data
pinheadmz Mar 1, 2022
e33e9c3
dns: fix hsk_dns_is_subdomain
pinheadmz Sep 20, 2022
f2a7445
src: address multiple minor review comments from #90
pinheadmz Sep 20, 2022
ca447c8
dns: clean up loop in hsk_dns_name_from_string()
pinheadmz Sep 21, 2022
bf6108b
dns: read domain names from tld.h as bytes, not strings
pinheadmz Sep 21, 2022
e307bb7
req: don't compute presentation format until it's needed by unbound
pinheadmz Sep 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 12 additions & 42 deletions src/cache.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ hsk_cache_prune(hsk_cache_t *c) {
bool
hsk_cache_insert_data(
hsk_cache_t *c,
const char *name,
const uint8_t *name,
uint16_t type,
uint8_t *wire,
size_t wire_len
Expand All @@ -75,8 +75,7 @@ hsk_cache_insert_data(
hsk_cache_key_t ck;
hsk_cache_key_init(&ck);

if (!hsk_cache_key_set(&ck, name, type))
return false;
hsk_cache_key_set(&ck, name, type);

hsk_cache_item_t *cache = hsk_map_get(&c->map, &ck);

Expand Down Expand Up @@ -142,7 +141,7 @@ hsk_cache_insert(
bool
hsk_cache_get_data(
hsk_cache_t *c,
const char *name,
const uint8_t *name,
uint16_t type,
uint8_t **wire,
size_t *wire_len
Expand All @@ -152,8 +151,7 @@ hsk_cache_get_data(
hsk_cache_key_t ck;
hsk_cache_key_init(&ck);

if (!hsk_cache_key_set(&ck, name, type))
return false;
hsk_cache_key_set(&ck, name, type);

hsk_cache_item_t *cache = hsk_map_get(&c->map, &ck);

Expand Down Expand Up @@ -181,7 +179,9 @@ hsk_cache_get(hsk_cache_t *c, const hsk_dns_req_t *req) {
if (!hsk_cache_get_data(c, req->name, req->type, &data, &data_len))
return NULL;

hsk_cache_log(c, "cache hit for: %s\n", req->name);
char namestr[HSK_DNS_MAX_NAME_STRING] = {0};
assert(hsk_dns_name_to_string(req->name, namestr));
hsk_cache_log(c, "cache hit for: %s\n", namestr);

if (!hsk_dns_msg_decode(data, data_len, &msg)) {
hsk_cache_log(c, "could not deserialize cached item\n");
Expand Down Expand Up @@ -255,16 +255,10 @@ hsk_cache_key_equal(const void *a, const void *b) {
return true;
}

bool
hsk_cache_key_set(hsk_cache_key_t *ck, const char *name, uint16_t type) {
void
hsk_cache_key_set(hsk_cache_key_t *ck, const uint8_t *name, uint16_t type) {
assert(ck);

if (!hsk_dns_name_verify(name))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we still need to verify if tld is correct ?

Otherwise this method will never return false.
I guess that could make this more precise and instead move tld check to the hsk_cache_insert_data and hsk_cache_get_data before the cache_key_set.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method will become void and instead those two methods will return early.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I think we can remove the return value. We check hsk_dns_name_verify() in hsk_ns_onrecv() which is the entry point for the user's request and is checked before attempting name resolution and before caching. So I think throughout hnsd we can assume that req->tld is already valid.

so I changed hsk_cache_key_set() to void -- what did you mean by "two methods" ?

fixed in f5200ab

return false;

if (hsk_dns_name_dirty(name))
return false;

int labels = hsk_dns_label_count(name);
bool ref = false;

Expand All @@ -276,30 +270,6 @@ hsk_cache_key_set(hsk_cache_key_t *ck, const char *name, uint16_t type) {
case 2:
ref = !hsk_resource_is_ptr(name);
break;
case 3:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed with @pinheadmz, this should have been removed even before this update. As root wont parse/recognize SRV/TLSA//SMIMEA and OPENPGPKEY records at all.

switch (type) {
case HSK_DNS_SRV: {
ref = !hsk_dns_label_is_srv(name);
break;
}
case HSK_DNS_TLSA: {
ref = !hsk_dns_label_is_tlsa(name);
break;
}
case HSK_DNS_SMIMEA: {
ref = !hsk_dns_label_is_smimea(name);
break;
}
case HSK_DNS_OPENPGPKEY: {
ref = !hsk_dns_label_is_openpgpkey(name);
break;
}
default: {
ref = true;
break;
}
}
break;
default:
ref = true;
break;
Expand All @@ -308,12 +278,12 @@ hsk_cache_key_set(hsk_cache_key_t *ck, const char *name, uint16_t type) {
if (ref)
labels = 1;

ck->name_len = hsk_dns_label_from(name, -labels, (char *)ck->name);
hsk_to_lower((char *)ck->name);
ck->name_len = hsk_dns_label_from(name, -labels, ck->name);
hsk_to_lower(ck->name);
ck->ref = ref;
ck->type = type;

return true;
return;
}

void
Expand Down
10 changes: 5 additions & 5 deletions src/cache.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ typedef struct hsk_cache_s {
} hsk_cache_t;

typedef struct hsk_cache_key_s {
uint8_t name[HSK_DNS_MAX_NAME + 1];
uint8_t name[HSK_DNS_MAX_NAME];
size_t name_len;
uint16_t type;
bool ref;
Expand Down Expand Up @@ -44,7 +44,7 @@ hsk_cache_free(hsk_cache_t *c);
bool
hsk_cache_insert_data(
hsk_cache_t *c,
const char *name,
const uint8_t *name,
uint16_t type,
uint8_t *wire,
size_t wire_len
Expand All @@ -60,7 +60,7 @@ hsk_cache_insert(
bool
hsk_cache_get_data(
hsk_cache_t *c,
const char *name,
const uint8_t *name,
uint16_t type,
uint8_t **wire,
size_t *wire_len
Expand All @@ -87,8 +87,8 @@ hsk_cache_key_hash(const void *key);
bool
hsk_cache_key_equal(const void *a, const void *b);

bool
hsk_cache_key_set(hsk_cache_key_t *ck, const char *name, uint16_t type);
void
hsk_cache_key_set(hsk_cache_key_t *ck, const uint8_t *name, uint16_t type);

void
hsk_cache_item_init(hsk_cache_item_t *ci);
Expand Down
Loading