From 08b02a50570250f9891baef8c10fa7ee843afe59 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Sat, 25 Apr 2020 11:02:08 +1200 Subject: [PATCH 1/6] ndr_dns: move ndr_push_dns_string_list core into sharable function This is because ndr_nbt.c does almost exactly the same thing with almost exactly the same code, and they both do it wrong. Soon they will both be using this better version. Signed-off-by: Douglas Bagnall --- librpc/ndr/ndr_dns.c | 79 +++------------------------------- librpc/ndr/ndr_dns_utils.c | 88 ++++++++++++++++++++++++++++++++++++++ librpc/ndr/ndr_dns_utils.h | 5 +++ librpc/wscript_build | 2 +- 4 files changed, 99 insertions(+), 75 deletions(-) create mode 100644 librpc/ndr/ndr_dns_utils.c create mode 100644 librpc/ndr/ndr_dns_utils.h diff --git a/librpc/ndr/ndr_dns.c b/librpc/ndr/ndr_dns.c index d37c8cc2ece..68a3c9de782 100644 --- a/librpc/ndr/ndr_dns.c +++ b/librpc/ndr/ndr_dns.c @@ -33,6 +33,7 @@ #include "librpc/gen_ndr/ndr_dnsp.h" #include "system/locale.h" #include "lib/util/util_net.h" +#include "ndr_dns_utils.h" /* don't allow an unlimited number of name components */ #define MAX_COMPONENTS 128 @@ -159,80 +160,10 @@ _PUBLIC_ enum ndr_err_code ndr_push_dns_string(struct ndr_push *ndr, int ndr_flags, const char *s) { - if (!(ndr_flags & NDR_SCALARS)) { - return NDR_ERR_SUCCESS; - } - - while (s && *s) { - enum ndr_err_code ndr_err; - char *compname; - size_t complen; - uint32_t offset; - - if (!(ndr->flags & LIBNDR_FLAG_NO_COMPRESSION)) { - /* see if we have pushed the remaining string already, - * if so we use a label pointer to this string - */ - ndr_err = ndr_token_retrieve_cmp_fn(&ndr->dns_string_list, s, - &offset, - (comparison_fn_t)strcmp, - false); - if (NDR_ERR_CODE_IS_SUCCESS(ndr_err)) { - uint8_t b[2]; - - if (offset > 0x3FFF) { - return ndr_push_error(ndr, NDR_ERR_STRING, - "offset for dns string " \ - "label pointer " \ - "%u[%08X] > 0x00003FFF", - offset, offset); - } - - b[0] = 0xC0 | (offset>>8); - b[1] = (offset & 0xFF); - - return ndr_push_bytes(ndr, b, 2); - } - } - - complen = strcspn(s, "."); - - /* we need to make sure the length fits into 6 bytes */ - if (complen > 0x3F) { - return ndr_push_error(ndr, NDR_ERR_STRING, - "component length %u[%08X] > " \ - "0x0000003F", - (unsigned)complen, - (unsigned)complen); - } - - compname = talloc_asprintf(ndr, "%c%*.*s", - (unsigned char)complen, - (unsigned char)complen, - (unsigned char)complen, s); - NDR_ERR_HAVE_NO_MEMORY(compname); - - /* remember the current component + the rest of the string - * so it can be reused later - */ - if (!(ndr->flags & LIBNDR_FLAG_NO_COMPRESSION)) { - NDR_CHECK(ndr_token_store(ndr, &ndr->dns_string_list, s, - ndr->offset)); - } - - /* push just this component into the blob */ - NDR_CHECK(ndr_push_bytes(ndr, (const uint8_t *)compname, - complen+1)); - talloc_free(compname); - - s += complen; - if (*s == '.') s++; - } - - /* if we reach the end of the string and have pushed the last component - * without using a label pointer, we need to terminate the string - */ - return ndr_push_bytes(ndr, (const uint8_t *)"", 1); + return ndr_push_dns_string_list(ndr, + &ndr->dns_string_list, + ndr_flags, + s); } _PUBLIC_ enum ndr_err_code ndr_pull_dns_txt_record(struct ndr_pull *ndr, int ndr_flags, struct dns_txt_record *r) diff --git a/librpc/ndr/ndr_dns_utils.c b/librpc/ndr/ndr_dns_utils.c new file mode 100644 index 00000000000..2d9b5f1bc1e --- /dev/null +++ b/librpc/ndr/ndr_dns_utils.c @@ -0,0 +1,88 @@ +#include "includes.h" +#include "../librpc/ndr/libndr.h" +#include "ndr_dns_utils.h" + + +/** + push a dns/nbt string list to the wire +*/ +enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr, + struct ndr_token_list *string_list, + int ndr_flags, + const char *s) +{ + if (!(ndr_flags & NDR_SCALARS)) { + return NDR_ERR_SUCCESS; + } + + while (s && *s) { + enum ndr_err_code ndr_err; + char *compname; + size_t complen; + uint32_t offset; + + if (!(ndr->flags & LIBNDR_FLAG_NO_COMPRESSION)) { + /* see if we have pushed the remaining string already, + * if so we use a label pointer to this string + */ + ndr_err = ndr_token_retrieve_cmp_fn(string_list, s, + &offset, + (comparison_fn_t)strcmp, + false); + if (NDR_ERR_CODE_IS_SUCCESS(ndr_err)) { + uint8_t b[2]; + + if (offset > 0x3FFF) { + return ndr_push_error(ndr, NDR_ERR_STRING, + "offset for dns string " \ + "label pointer " \ + "%u[%08X] > 0x00003FFF", + offset, offset); + } + + b[0] = 0xC0 | (offset>>8); + b[1] = (offset & 0xFF); + + return ndr_push_bytes(ndr, b, 2); + } + } + + complen = strcspn(s, "."); + + /* we need to make sure the length fits into 6 bytes */ + if (complen > 0x3F) { + return ndr_push_error(ndr, NDR_ERR_STRING, + "component length %u[%08X] > " \ + "0x0000003F", + (unsigned)complen, + (unsigned)complen); + } + + compname = talloc_asprintf(ndr, "%c%*.*s", + (unsigned char)complen, + (unsigned char)complen, + (unsigned char)complen, s); + NDR_ERR_HAVE_NO_MEMORY(compname); + + /* remember the current component + the rest of the string + * so it can be reused later + */ + if (!(ndr->flags & LIBNDR_FLAG_NO_COMPRESSION)) { + NDR_CHECK(ndr_token_store(ndr, string_list, s, + ndr->offset)); + } + + /* push just this component into the blob */ + NDR_CHECK(ndr_push_bytes(ndr, (const uint8_t *)compname, + complen+1)); + talloc_free(compname); + + s += complen; + if (*s == '.') s++; + } + + /* if we reach the end of the string and have pushed the last component + * without using a label pointer, we need to terminate the string + */ + return ndr_push_bytes(ndr, (const uint8_t *)"", 1); +} diff --git a/librpc/ndr/ndr_dns_utils.h b/librpc/ndr/ndr_dns_utils.h new file mode 100644 index 00000000000..823e3201112 --- /dev/null +++ b/librpc/ndr/ndr_dns_utils.h @@ -0,0 +1,5 @@ + +enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr, + struct ndr_token_list *string_list, + int ndr_flags, + const char *s); diff --git a/librpc/wscript_build b/librpc/wscript_build index 928c96dbae4..c6b0c5bb786 100644 --- a/librpc/wscript_build +++ b/librpc/wscript_build @@ -31,7 +31,7 @@ bld.SAMBA_SUBSYSTEM('NDR_DNSSERVER', ) bld.SAMBA_SUBSYSTEM('NDR_DNS', - source='gen_ndr/ndr_dns.c ndr/ndr_dns.c', + source='gen_ndr/ndr_dns.c ndr/ndr_dns.c ndr/ndr_dns_utils.c', public_deps='ndr NDR_DNSP' ) -- 2.17.1 From 167f7319fbf44f3d516b3e4bde857c3d9836d979 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Sat, 25 Apr 2020 11:03:30 +1200 Subject: [PATCH 2/6] ndr/dns_utils: correct a comment Signed-off-by: Douglas Bagnall --- librpc/ndr/ndr_dns_utils.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/librpc/ndr/ndr_dns_utils.c b/librpc/ndr/ndr_dns_utils.c index 2d9b5f1bc1e..2ce300863bc 100644 --- a/librpc/ndr/ndr_dns_utils.c +++ b/librpc/ndr/ndr_dns_utils.c @@ -49,7 +49,7 @@ enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr, complen = strcspn(s, "."); - /* we need to make sure the length fits into 6 bytes */ + /* the length must fit into 6 bits (i.e. <= 63) */ if (complen > 0x3F) { return ndr_push_error(ndr, NDR_ERR_STRING, "component length %u[%08X] > " \ -- 2.17.1 From b0c260fea4ebb7c69d3ae1fd02d9dbcbde169e24 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Sat, 25 Apr 2020 11:10:18 +1200 Subject: [PATCH 3/6] ndr_dns: do not allow consecutive dots The empty subdomain component is reserved for the root domain, which we should only (and always) see at the end of the list. That is, we expect "example.com.", but not "example..com". Signed-off-by: Douglas Bagnall --- librpc/ndr/ndr_dns_utils.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/librpc/ndr/ndr_dns_utils.c b/librpc/ndr/ndr_dns_utils.c index 2ce300863bc..6931dac422d 100644 --- a/librpc/ndr/ndr_dns_utils.c +++ b/librpc/ndr/ndr_dns_utils.c @@ -58,6 +58,12 @@ enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr, (unsigned)complen); } + if (complen == 0 && s[complen] == '.') { + return ndr_push_error(ndr, NDR_ERR_STRING, + "component length is 0 " + "(consecutive dots)"); + } + compname = talloc_asprintf(ndr, "%c%*.*s", (unsigned char)complen, (unsigned char)complen, -- 2.17.1 From 19e85736369ac86a301abe10017611b12e5a4e45 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Fri, 15 May 2020 00:06:08 +1200 Subject: [PATCH 4/6] push: do not allow strings longer than 255 bytes --- librpc/ndr/ndr_dns_utils.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/librpc/ndr/ndr_dns_utils.c b/librpc/ndr/ndr_dns_utils.c index 6931dac422d..b7f11dbab4e 100644 --- a/librpc/ndr/ndr_dns_utils.c +++ b/librpc/ndr/ndr_dns_utils.c @@ -11,6 +11,8 @@ enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr, int ndr_flags, const char *s) { + const char *start = s; + if (!(ndr_flags & NDR_SCALARS)) { return NDR_ERR_SUCCESS; } @@ -84,7 +86,13 @@ enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr, talloc_free(compname); s += complen; - if (*s == '.') s++; + if (*s == '.') { + s++; + } + if (s - start > 255) { + return ndr_push_error(ndr, NDR_ERR_STRING, + "name > 255 character long"); + } } /* if we reach the end of the string and have pushed the last component -- 2.17.1 From 66316b04b60e30106c6b268c96e651ab33eecce3 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Fri, 15 May 2020 10:52:45 +1200 Subject: [PATCH 5/6] ndr/dns-utils: prepare for NBT compatibility NBT has a funny thing where it sometimes needs to send a trailing dot as part of the last component, because the string representation is a user name. In DNS, "example.com", and "example.com." are the same, both having three components ("example", "com", ""); in NBT, we want to treat them differently, with the second form having the three components ("example", "com.", ""). This is based on e6e2ec0001fe3c010445e26cc0efddbc1f73416b. Also DNS compression cannot be turned off for NBT. Signed-off-by: Douglas Bagnall --- librpc/ndr/ndr_dns.c | 3 ++- librpc/ndr/ndr_dns_utils.c | 21 ++++++++++++++++++--- librpc/ndr/ndr_dns_utils.h | 3 ++- 3 files changed, 22 insertions(+), 5 deletions(-) diff --git a/librpc/ndr/ndr_dns.c b/librpc/ndr/ndr_dns.c index 68a3c9de782..966e0b59786 100644 --- a/librpc/ndr/ndr_dns.c +++ b/librpc/ndr/ndr_dns.c @@ -163,7 +163,8 @@ _PUBLIC_ enum ndr_err_code ndr_push_dns_string(struct ndr_push *ndr, return ndr_push_dns_string_list(ndr, &ndr->dns_string_list, ndr_flags, - s); + s, + false); } _PUBLIC_ enum ndr_err_code ndr_pull_dns_txt_record(struct ndr_pull *ndr, int ndr_flags, struct dns_txt_record *r) diff --git a/librpc/ndr/ndr_dns_utils.c b/librpc/ndr/ndr_dns_utils.c index b7f11dbab4e..b054091c46c 100644 --- a/librpc/ndr/ndr_dns_utils.c +++ b/librpc/ndr/ndr_dns_utils.c @@ -9,9 +9,16 @@ enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr, struct ndr_token_list *string_list, int ndr_flags, - const char *s) + const char *s, + bool is_nbt) { const char *start = s; + bool use_compression; + if (is_nbt) { + use_compression = true; + } else { + use_compression = !(ndr->flags & LIBNDR_FLAG_NO_COMPRESSION); + } if (!(ndr_flags & NDR_SCALARS)) { return NDR_ERR_SUCCESS; @@ -23,7 +30,7 @@ enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr, size_t complen; uint32_t offset; - if (!(ndr->flags & LIBNDR_FLAG_NO_COMPRESSION)) { + if (use_compression) { /* see if we have pushed the remaining string already, * if so we use a label pointer to this string */ @@ -66,6 +73,14 @@ enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr, "(consecutive dots)"); } + if (is_nbt && s[complen] == '.' && s[complen + 1] == '\0') { + /* nbt names are sometimes usernames, and we need to + * keep a trailing dot to ensure it is byte-identical, + * (not just semantically identical given DNS + * semantics). */ + complen++; + } + compname = talloc_asprintf(ndr, "%c%*.*s", (unsigned char)complen, (unsigned char)complen, @@ -75,7 +90,7 @@ enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr, /* remember the current component + the rest of the string * so it can be reused later */ - if (!(ndr->flags & LIBNDR_FLAG_NO_COMPRESSION)) { + if (use_compression) { NDR_CHECK(ndr_token_store(ndr, string_list, s, ndr->offset)); } diff --git a/librpc/ndr/ndr_dns_utils.h b/librpc/ndr/ndr_dns_utils.h index 823e3201112..71a65433bbb 100644 --- a/librpc/ndr/ndr_dns_utils.h +++ b/librpc/ndr/ndr_dns_utils.h @@ -2,4 +2,5 @@ enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr, struct ndr_token_list *string_list, int ndr_flags, - const char *s); + const char *s, + bool is_nbt); -- 2.17.1 From e2a493ff004d7bae4002115fafeb45204958a6fa Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Fri, 15 May 2020 11:48:27 +1200 Subject: [PATCH 6/6] ndr/nbt_push_string: use ndr_dns_utils common push function Signed-off-by: Douglas Bagnall --- librpc/ndr/ndr_nbt.c | 72 +++++--------------------------------------- librpc/wscript_build | 2 +- 2 files changed, 8 insertions(+), 66 deletions(-) diff --git a/librpc/ndr/ndr_nbt.c b/librpc/ndr/ndr_nbt.c index b15dc5c06e5..8ed9f0a5f05 100644 --- a/librpc/ndr/ndr_nbt.c +++ b/librpc/ndr/ndr_nbt.c @@ -25,6 +25,8 @@ #include "includes.h" #include "../libcli/nbt/libnbt.h" #include "../libcli/netlogon/netlogon.h" +#include "ndr_dns_utils.h" + /* don't allow an unlimited number of name components */ #define MAX_COMPONENTS 128 @@ -141,71 +143,11 @@ _PUBLIC_ enum ndr_err_code ndr_pull_nbt_string(struct ndr_pull *ndr, int ndr_fla */ _PUBLIC_ enum ndr_err_code ndr_push_nbt_string(struct ndr_push *ndr, int ndr_flags, const char *s) { - if (!(ndr_flags & NDR_SCALARS)) { - return NDR_ERR_SUCCESS; - } - - while (s && *s) { - enum ndr_err_code ndr_err; - char *compname; - size_t complen; - uint32_t offset; - - /* see if we have pushed the remaining string already, - * if so we use a label pointer to this string - */ - ndr_err = ndr_token_retrieve_cmp_fn(&ndr->nbt_string_list, s, &offset, (comparison_fn_t)strcmp, false); - if (NDR_ERR_CODE_IS_SUCCESS(ndr_err)) { - uint8_t b[2]; - - if (offset > 0x3FFF) { - return ndr_push_error(ndr, NDR_ERR_STRING, - "offset for nbt string label pointer %u[%08X] > 0x00003FFF", - offset, offset); - } - - b[0] = 0xC0 | (offset>>8); - b[1] = (offset & 0xFF); - - return ndr_push_bytes(ndr, b, 2); - } - - complen = strcspn(s, "."); - - /* we need to make sure the length fits into 6 bytes */ - if (complen > 0x3F) { - return ndr_push_error(ndr, NDR_ERR_STRING, - "component length %u[%08X] > 0x0000003F", - (unsigned)complen, (unsigned)complen); - } - - if (s[complen] == '.' && s[complen+1] == '\0') { - complen++; - } - - compname = talloc_asprintf(ndr, "%c%*.*s", - (unsigned char)complen, - (unsigned char)complen, - (unsigned char)complen, s); - NDR_ERR_HAVE_NO_MEMORY(compname); - - /* remember the current componemt + the rest of the string - * so it can be reused later - */ - NDR_CHECK(ndr_token_store(ndr, &ndr->nbt_string_list, s, ndr->offset)); - - /* push just this component into the blob */ - NDR_CHECK(ndr_push_bytes(ndr, (const uint8_t *)compname, complen+1)); - talloc_free(compname); - - s += complen; - if (*s == '.') s++; - } - - /* if we reach the end of the string and have pushed the last component - * without using a label pointer, we need to terminate the string - */ - return ndr_push_bytes(ndr, (const uint8_t *)"", 1); + return ndr_push_dns_string_list(ndr, + &ndr->dns_string_list, + ndr_flags, + s, + true); } diff --git a/librpc/wscript_build b/librpc/wscript_build index c6b0c5bb786..e9963474de0 100644 --- a/librpc/wscript_build +++ b/librpc/wscript_build @@ -415,7 +415,7 @@ bld.SAMBA_SUBSYSTEM('NDR_NBT', bld.SAMBA_LIBRARY('ndr_nbt', source='gen_ndr/ndr_nbt.c ndr/ndr_nbt.c', - public_deps='ndr NDR_NBT_BUF NDR_SECURITY', + public_deps='ndr NDR_NBT_BUF NDR_SECURITY NDR_DNS', public_headers='gen_ndr/nbt.h gen_ndr/ndr_nbt.h ndr/ndr_nbt.h', header_path=[ ('gen_ndr*', 'gen_ndr'), ('ndr*', 'ndr')], pc_files='ndr_nbt.pc', -- 2.17.1