Package: libbind9-160 Version: 1:9.11.4.P2+dfsg-1 Severity: grave Tags: security upstream patch Forwarded: security-offi...@isc.org Control: found -1 1:9.11.4.P1+dfsg-1 Control: found -1 1:9.11.4+dfsg-4
Hi, I discovered the following security bug in bind9 a few weeks ago, and responsibly disclosed it to the ISC security officer. Unfortunately, until today they did not acknowledge it is a security issue - in contrast, they proved that they do not fully understand the issue, and now have added a new feature in the 9.11.4.P2 release which wrongly addresses this security issue. The issue is with DDNS update policies using Kerberos, namely the krb5-subdomain and ms-subdomain update policies for TKEY-GSSAPI. The documentation states, and has always stated, the following: krb5-self --------- This rule takes a Kerberos machine principal (host/machine@REALM) for machine in REALM and and converts it machine.realm allowing the machine to update machine.realm. The REALM to be matched is specified in the identity field. The name field should be set to "." krb5-subdomain -------------- This rule takes a Kerberos machine principal (host/machine@REALM) for machine in REALM and converts it to machine.realm allowing the machine to update subdomains of machine.realm. The REALM to be matched is specified in the identity field. The name field should be set to "." https://ftp.isc.org/isc/bind9/9.11.4-P1/doc/arm/Bv9ARM.ch06.html#dynamic_update_policies (I am referring to both krb5-* and ms-* when saying krb5-* in the following.) Now the issue is the following (at least in all revisions of bind 9.10 and 9.11, *including the recently released 9.11.4.P1 and .P2*: krb5-self works a documented, and allows any client showing a valid Kerberos ticket for machine.realm to update exactly machine.realm. However, krb5-subdomain is missing the documented check completely - *it allows updating all records*. To be more precise, it checks the name of the record to be updated against the name field, instead of, as documented, against the machine name from the Kerberos identity. If a BIND TKEY update policy is configured as described in the manual, with the administrator intending to allow machines to update records below their own hostname, they are indeed granting full access to the whole zonem because the documented Kerberos name check is missing. The ISC security officer claims the following: The documented update policy was never intended to work like documented. It was intended to work like th ecode does, only checking against the configured name field instead of the Kerberos machine name. This is not a security issue - it is a documentation bug. I have raised concerns about these views with the ISC because it has some implications that I will not believe to be correct: There already is a check called subdomai n(without krb5-) that allows updating subdomains of the configured name. It can also take a Kerberos principal name as TKEY identity - in that case it allows updating the subdomain given in the name field if a client shows a valid Kerberos ticket for the principal in the identity field. This is what the ISC claims to be the intention of krb5-subdomain - I do, however, doubt that the person adding the krb5-subdomain check intended to add a new check that does the same as the existing subdomain check. (I also doubt that the person intended to duplicate an existing check, then accidentally added documentation stating the contrary.) The code for krb5-subdomain *does* go through all the hassle to extract the machine name from the given Kerberos identity - it then ignores the result and always returns TRUE instead of checking the record to be updated against the machine name it just got hold of with so much work. I do doubt that the person writing the original code, in addition to the above, intentionally added this code handling a Kerberos identity, then always returning TRUE, resulting in the check to do the same as the normal subdomain check - only with 100 lines of string manipulation resulting in a no-op wrapped around it. Even after explaining all this to the ISC, they decided to not fix these checks to do what they are documented (and obviously intended) to do. Instead, in 9.11.4.P1, they added *new* checks called krb5-subself and ms-subself that do what krb5-subdomain and ms-subdomain were intended to do, and sold this as a new feature (they also did not mention my research and patching efforts doing so, they just told the world they added a cool new feature - THANK YOU 💖!). The broken -subdomain checks were kept as they are, including the broken documentation. Even if the ISC intends to keep the broken checks (which, again, are duplicates of a more simple check, with tons of string manipulation code wrapped around), I still consider this a serious security issue because with the checks not doing what the documentation says for many years will surely have left system administrators to believe that they had implemented security measures for their DDNS updates, and now it turns out their expectations were wrong all the time. Even *if* the original author of these checks had intended them to not check the Kerberos machine name (which, as laid out above, I strongly doubt), there was a security promise that was not true for years. I am posting this bug report to document the issue, because the ISC security officer has been unresponsive for more than a week, and the concerns raised were obviously ignored. I leave it up to the Debian BIND maintainers to do whatever they deem necessary, or also ignore the issue as a whole. In any case, attached are my original patches, which the ISC partially took over in their "new feature" (again without any credits). Kind regards, Nik
diff --git a/lib/dns/gssapictx.c b/lib/dns/gssapictx.c
index 35cfb902ae..df0901c0ec 100644
--- a/lib/dns/gssapictx.c
+++ b/lib/dns/gssapictx.c
@@ -344,7 +344,7 @@ cleanup:
isc_boolean_t
dst_gssapi_identitymatchesrealmkrb5(dns_name_t *signer, dns_name_t *name,
- dns_name_t *realm)
+ dns_name_t *realm, isc_boolean_t with_subdomains)
{
#ifdef GSSAPI
char sbuf[DNS_NAME_FORMATSIZE];
@@ -354,6 +354,7 @@ dst_gssapi_identitymatchesrealmkrb5(dns_name_t *signer, dns_name_t *name,
char *rname;
isc_buffer_t buffer;
isc_result_t result;
+ dns_fixedname_t signer_name;
/*
* It is far, far easier to write the names we are looking at into
@@ -363,8 +364,7 @@ dst_gssapi_identitymatchesrealmkrb5(dns_name_t *signer, dns_name_t *name,
result = dns_name_toprincipal(signer, &buffer);
RUNTIME_CHECK(result == ISC_R_SUCCESS);
isc_buffer_putuint8(&buffer, 0);
- if (name != NULL)
- dns_name_format(name, nbuf, sizeof(nbuf));
+ dns_name_format(name, nbuf, sizeof(nbuf));
dns_name_format(realm, rbuf, sizeof(rbuf));
/*
@@ -395,16 +395,49 @@ dst_gssapi_identitymatchesrealmkrb5(dns_name_t *signer, dns_name_t *name,
return (isc_boolean_false);
/*
- * Now, we do a simple comparison between the name and the realm.
+ * At this point in time, when we assume
+ * - Realm: EXAMPLE.COM
+ * - Signer:
+ * * KRB5: host/foo.example....@example.com
+ * * MS : FOO$@EXAMPLE.COM
+ * - Record to be updated: bar.example.com
+ * then we have:
+ * - sname: foo.Example.com (krb5) / foo (MS)
+ * - nbuf : bar.example.com
+ * - rname: EXAMPLE.COM
+ * So, for MS we have to reconstruct the full signer name.
*/
- if (name != NULL) {
- if ((strcasecmp(sname, nbuf) == 0)
- && (strcmp(rname, rbuf) == 0))
- return (isc_boolean_true);
- } else {
- if (strcmp(rname, rbuf) == 0)
- return (isc_boolean_true);
- }
+
+ /*
+ * Now, compare the signer name with the name of the
+ * record to be updated. For subdomain updates, we do
+ * a subdomain checks, otherwise a simple comparison.
+ */
+ if (with_subdomains) {
+ /*
+ * Convert the signer's name back to a dns_name_t
+ * for later subdomain comparison.
+ */
+ dns_fixedname_init(&signer_name);
+ isc_buffer_constinit(&buffer, sname, strlen(sname));
+ isc_buffer_add(&buffer, strlen(sname));
+ result = dns_name_fromtext(dns_fixedname_name(&signer_name), &buffer,
+ dns_rootname, 0, NULL);
+ if (result != ISC_R_SUCCESS)
+ return (isc_boolean_false);
+ /*
+ * Now we compare the signer's name with the requested name.
+ */
+ if (!dns_name_issubdomain(name, dns_fixedname_name(&signer_name)))
+ return (isc_boolean_false);
+ } else if (strcasecmp(sname, nbuf) != 0)
+ return (isc_boolean_false);
+
+ /*
+ * Lastly, we compare the realm.
+ */
+ if (strcmp(rname, rbuf) == 0)
+ return (isc_boolean_true);
return (isc_boolean_false);
#else
@@ -417,17 +450,17 @@ dst_gssapi_identitymatchesrealmkrb5(dns_name_t *signer, dns_name_t *name,
isc_boolean_t
dst_gssapi_identitymatchesrealmms(dns_name_t *signer, dns_name_t *name,
- dns_name_t *realm)
+ dns_name_t *realm, isc_boolean_t with_subdomains)
{
#ifdef GSSAPI
char sbuf[DNS_NAME_FORMATSIZE];
char nbuf[DNS_NAME_FORMATSIZE];
char rbuf[DNS_NAME_FORMATSIZE];
char *sname;
- char *nname;
char *rname;
isc_buffer_t buffer;
isc_result_t result;
+ dns_fixedname_t signer_name;
/*
* It is far, far easier to write the names we are looking at into
@@ -437,8 +470,7 @@ dst_gssapi_identitymatchesrealmms(dns_name_t *signer, dns_name_t *name,
result = dns_name_toprincipal(signer, &buffer);
RUNTIME_CHECK(result == ISC_R_SUCCESS);
isc_buffer_putuint8(&buffer, 0);
- if (name != NULL)
- dns_name_format(name, nbuf, sizeof(nbuf));
+ dns_name_format(name, nbuf, sizeof(nbuf));
dns_name_format(realm, rbuf, sizeof(rbuf));
/*
@@ -473,29 +505,54 @@ dst_gssapi_identitymatchesrealmms(dns_name_t *signer, dns_name_t *name,
sname = sbuf;
/*
- * Find the first . in the target name, and make it the end of
- * the string. The rest of the name has to match the realm.
+ * At this point in time, when we assume
+ * - Realm: EXAMPLE.COM
+ * - Signer:
+ * * KRB5: host/foo.example....@example.com
+ * * MS : FOO$@EXAMPLE.COM
+ * - Record to be updated: bar.example.com
+ * then we have:
+ * - sname: foo.Example.com (krb5) / foo (MS)
+ * - nbuf : bar.example.com
+ * - rname: EXAMPLE.COM
+ * So, for MS we have to reconstruct the full signer name.
*/
- if (name != NULL) {
- nname = strchr(nbuf, '.');
- if (nname == NULL)
- return (isc_boolean_false);
- *nname++ = '\0';
- }
+
+ *sname++ = '.';
+ /* fits because foo.EXAMPLE.COM is shorter than FOO$@EXAMPLE.COM */
+ memmove(sname, rname, strlen(rname) + /* NUL */ 1);
+ sname = sbuf;
/*
- * Now, we do a simple comparison between the name and the realm.
+ * Now, compare the signer name with the name of the
+ * record to be updated. For subdomain updates, we do
+ * a subdomain checks, otherwise a simple comparison.
*/
- if (name != NULL) {
- if ((strcasecmp(sname, nbuf) == 0)
- && (strcmp(rname, rbuf) == 0)
- && (strcasecmp(nname, rbuf) == 0))
- return (isc_boolean_true);
- } else {
- if (strcmp(rname, rbuf) == 0)
- return (isc_boolean_true);
- }
+ if (with_subdomains) {
+ /*
+ * Convert the signer's name back to a dns_name_t
+ * for later subdomain comparison.
+ */
+ dns_fixedname_init(&signer_name);
+ isc_buffer_constinit(&buffer, sname, strlen(sname));
+ isc_buffer_add(&buffer, strlen(sname));
+ result = dns_name_fromtext(dns_fixedname_name(&signer_name), &buffer,
+ dns_rootname, 0, NULL);
+ if (result != ISC_R_SUCCESS)
+ return (isc_boolean_false);
+ /*
+ * Now we compare the signer's name with the requested name.
+ */
+ if (!dns_name_issubdomain(name, dns_fixedname_name(&signer_name)))
+ return (isc_boolean_false);
+ } else if (strcasecmp(sname, nbuf) != 0)
+ return (isc_boolean_false);
+ /*
+ * Lastly, we compare the realm.
+ */
+ if (strcmp(rname, rbuf) == 0)
+ return (isc_boolean_true);
return (isc_boolean_false);
#else
diff --git a/lib/dns/include/dst/gssapi.h b/lib/dns/include/dst/gssapi.h
index 99da6292a1..a1f65d37d1 100644
--- a/lib/dns/include/dst/gssapi.h
+++ b/lib/dns/include/dst/gssapi.h
@@ -185,7 +185,7 @@ gss_error_tostring(isc_uint32_t major, isc_uint32_t minor,
isc_boolean_t
dst_gssapi_identitymatchesrealmkrb5(dns_name_t *signer, dns_name_t *name,
- dns_name_t *realm);
+ dns_name_t *realm, isc_boolean_t with_subdomains);
/*
* Compare a "signer" (in the format of a Kerberos-format Kerberos5
* principal: host/example....@example.com) to the realm name stored
@@ -195,7 +195,7 @@ dst_gssapi_identitymatchesrealmkrb5(dns_name_t *signer, dns_name_t *name,
isc_boolean_t
dst_gssapi_identitymatchesrealmms(dns_name_t *signer, dns_name_t *name,
- dns_name_t *realm);
+ dns_name_t *realm, isc_boolean_t with_subdomains);
/*
* Compare a "signer" (in the format of a Kerberos-format Kerberos5
* principal: host/example....@example.com) to the realm name stored
diff --git a/lib/dns/ssu.c b/lib/dns/ssu.c
index adc1505707..5ed6d70d49 100644
--- a/lib/dns/ssu.c
+++ b/lib/dns/ssu.c
@@ -462,26 +462,30 @@ dns_ssutable_checkrules2(dns_ssutable_t *table, dns_name_t *signer,
break;
case DNS_SSUMATCHTYPE_SELFKRB5:
if (!dst_gssapi_identitymatchesrealmkrb5(signer, name,
- rule->identity))
+ rule->identity,
+ ISC_FALSE))
continue;
break;
case DNS_SSUMATCHTYPE_SELFMS:
if (!dst_gssapi_identitymatchesrealmms(signer, name,
- rule->identity))
+ rule->identity,
+ ISC_FALSE))
continue;
break;
case DNS_SSUMATCHTYPE_SUBDOMAINKRB5:
if (!dns_name_issubdomain(name, rule->name))
continue;
- if (!dst_gssapi_identitymatchesrealmkrb5(signer, NULL,
- rule->identity))
+ if (!dst_gssapi_identitymatchesrealmkrb5(signer, name,
+ rule->identity,
+ ISC_TRUE))
continue;
break;
case DNS_SSUMATCHTYPE_SUBDOMAINMS:
if (!dns_name_issubdomain(name, rule->name))
continue;
- if (!dst_gssapi_identitymatchesrealmms(signer, NULL,
- rule->identity))
+ if (!dst_gssapi_identitymatchesrealmms(signer, name,
+ rule->identity,
+ ISC_TRUE))
continue;
break;
case DNS_SSUMATCHTYPE_TCPSELF:
diff --git a/lib/dns/gssapictx.c b/lib/dns/gssapictx.c
index 3a3af34a98..d31341438f 100644
--- a/lib/dns/gssapictx.c
+++ b/lib/dns/gssapictx.c
@@ -351,7 +351,7 @@ cleanup:
isc_boolean_t
dst_gssapi_identitymatchesrealmkrb5(dns_name_t *signer, dns_name_t *name,
- dns_name_t *realm)
+ dns_name_t *realm, isc_boolean_t with_subdomains)
{
#ifdef GSSAPI
char sbuf[DNS_NAME_FORMATSIZE];
@@ -361,6 +361,7 @@ dst_gssapi_identitymatchesrealmkrb5(dns_name_t *signer, dns_name_t *name,
char *rname;
isc_buffer_t buffer;
isc_result_t result;
+ dns_fixedname_t signer_name;
/*
* It is far, far easier to write the names we are looking at into
@@ -370,8 +371,7 @@ dst_gssapi_identitymatchesrealmkrb5(dns_name_t *signer, dns_name_t *name,
result = dns_name_toprincipal(signer, &buffer);
RUNTIME_CHECK(result == ISC_R_SUCCESS);
isc_buffer_putuint8(&buffer, 0);
- if (name != NULL)
- dns_name_format(name, nbuf, sizeof(nbuf));
+ dns_name_format(name, nbuf, sizeof(nbuf));
dns_name_format(realm, rbuf, sizeof(rbuf));
/*
@@ -402,16 +402,49 @@ dst_gssapi_identitymatchesrealmkrb5(dns_name_t *signer, dns_name_t *name,
return (isc_boolean_false);
/*
- * Now, we do a simple comparison between the name and the realm.
+ * At this point in time, when we assume
+ * - Realm: EXAMPLE.COM
+ * - Signer:
+ * * KRB5: host/foo.example....@example.com
+ * * MS : FOO$@EXAMPLE.COM
+ * - Record to be updated: bar.example.com
+ * then we have:
+ * - sname: foo.Example.com (krb5) / foo (MS)
+ * - nbuf : bar.example.com
+ * - rname: EXAMPLE.COM
+ * So, for MS we have to reconstruct the full signer name.
*/
- if (name != NULL) {
- if ((strcasecmp(sname, nbuf) == 0)
- && (strcmp(rname, rbuf) == 0))
- return (isc_boolean_true);
- } else {
- if (strcmp(rname, rbuf) == 0)
- return (isc_boolean_true);
- }
+
+ /*
+ * Now, compare the signer name with the name of the
+ * record to be updated. For subdomain updates, we do
+ * a subdomain checks, otherwise a simple comparison.
+ */
+ if (with_subdomains) {
+ /*
+ * Convert the signer's name back to a dns_name_t
+ * for later subdomain comparison.
+ */
+ dns_fixedname_init(&signer_name);
+ isc_buffer_constinit(&buffer, sname, strlen(sname));
+ isc_buffer_add(&buffer, strlen(sname));
+ result = dns_name_fromtext(dns_fixedname_name(&signer_name), &buffer,
+ dns_rootname, 0, NULL);
+ if (result != ISC_R_SUCCESS)
+ return (isc_boolean_false);
+ /*
+ * Now we compare the signer's name with the requested name.
+ */
+ if (!dns_name_issubdomain(name, dns_fixedname_name(&signer_name)))
+ return (isc_boolean_false);
+ } else if (strcasecmp(sname, nbuf) != 0)
+ return (isc_boolean_false);
+
+ /*
+ * Lastly, we compare the realm.
+ */
+ if (strcmp(rname, rbuf) == 0)
+ return (isc_boolean_true);
return (isc_boolean_false);
#else
@@ -424,17 +457,17 @@ dst_gssapi_identitymatchesrealmkrb5(dns_name_t *signer, dns_name_t *name,
isc_boolean_t
dst_gssapi_identitymatchesrealmms(dns_name_t *signer, dns_name_t *name,
- dns_name_t *realm)
+ dns_name_t *realm, isc_boolean_t with_subdomains)
{
#ifdef GSSAPI
char sbuf[DNS_NAME_FORMATSIZE];
char nbuf[DNS_NAME_FORMATSIZE];
char rbuf[DNS_NAME_FORMATSIZE];
char *sname;
- char *nname;
char *rname;
isc_buffer_t buffer;
isc_result_t result;
+ dns_fixedname_t signer_name;
/*
* It is far, far easier to write the names we are looking at into
@@ -444,8 +477,7 @@ dst_gssapi_identitymatchesrealmms(dns_name_t *signer, dns_name_t *name,
result = dns_name_toprincipal(signer, &buffer);
RUNTIME_CHECK(result == ISC_R_SUCCESS);
isc_buffer_putuint8(&buffer, 0);
- if (name != NULL)
- dns_name_format(name, nbuf, sizeof(nbuf));
+ dns_name_format(name, nbuf, sizeof(nbuf));
dns_name_format(realm, rbuf, sizeof(rbuf));
/*
@@ -480,29 +512,54 @@ dst_gssapi_identitymatchesrealmms(dns_name_t *signer, dns_name_t *name,
sname = sbuf;
/*
- * Find the first . in the target name, and make it the end of
- * the string. The rest of the name has to match the realm.
+ * At this point in time, when we assume
+ * - Realm: EXAMPLE.COM
+ * - Signer:
+ * * KRB5: host/foo.example....@example.com
+ * * MS : FOO$@EXAMPLE.COM
+ * - Record to be updated: bar.example.com
+ * then we have:
+ * - sname: foo.Example.com (krb5) / foo (MS)
+ * - nbuf : bar.example.com
+ * - rname: EXAMPLE.COM
+ * So, for MS we have to reconstruct the full signer name.
*/
- if (name != NULL) {
- nname = strchr(nbuf, '.');
- if (nname == NULL)
- return (isc_boolean_false);
- *nname++ = '\0';
- }
+
+ *sname++ = '.';
+ /* fits because foo.EXAMPLE.COM is shorter than FOO$@EXAMPLE.COM */
+ memmove(sname, rname, strlen(rname) + /* NUL */ 1);
+ sname = sbuf;
/*
- * Now, we do a simple comparison between the name and the realm.
+ * Now, compare the signer name with the name of the
+ * record to be updated. For subdomain updates, we do
+ * a subdomain checks, otherwise a simple comparison.
*/
- if (name != NULL) {
- if ((strcasecmp(sname, nbuf) == 0)
- && (strcmp(rname, rbuf) == 0)
- && (strcasecmp(nname, rbuf) == 0))
- return (isc_boolean_true);
- } else {
- if (strcmp(rname, rbuf) == 0)
- return (isc_boolean_true);
- }
+ if (with_subdomains) {
+ /*
+ * Convert the signer's name back to a dns_name_t
+ * for later subdomain comparison.
+ */
+ dns_fixedname_init(&signer_name);
+ isc_buffer_constinit(&buffer, sname, strlen(sname));
+ isc_buffer_add(&buffer, strlen(sname));
+ result = dns_name_fromtext(dns_fixedname_name(&signer_name), &buffer,
+ dns_rootname, 0, NULL);
+ if (result != ISC_R_SUCCESS)
+ return (isc_boolean_false);
+ /*
+ * Now we compare the signer's name with the requested name.
+ */
+ if (!dns_name_issubdomain(name, dns_fixedname_name(&signer_name)))
+ return (isc_boolean_false);
+ } else if (strcasecmp(sname, nbuf) != 0)
+ return (isc_boolean_false);
+ /*
+ * Lastly, we compare the realm.
+ */
+ if (strcmp(rname, rbuf) == 0)
+ return (isc_boolean_true);
return (isc_boolean_false);
#else
diff --git a/lib/dns/include/dst/gssapi.h b/lib/dns/include/dst/gssapi.h
index 53c594e6b2..a1a037e7b9 100644
--- a/lib/dns/include/dst/gssapi.h
+++ b/lib/dns/include/dst/gssapi.h
@@ -191,7 +191,7 @@ gss_error_tostring(isc_uint32_t major, isc_uint32_t minor,
isc_boolean_t
dst_gssapi_identitymatchesrealmkrb5(dns_name_t *signer, dns_name_t *name,
- dns_name_t *realm);
+ dns_name_t *realm, isc_boolean_t with_subdomains);
/*
* Compare a "signer" (in the format of a Kerberos-format Kerberos5
* principal: host/example....@example.com) to the realm name stored
@@ -201,7 +201,7 @@ dst_gssapi_identitymatchesrealmkrb5(dns_name_t *signer, dns_name_t *name,
isc_boolean_t
dst_gssapi_identitymatchesrealmms(dns_name_t *signer, dns_name_t *name,
- dns_name_t *realm);
+ dns_name_t *realm, isc_boolean_t with_subdomains);
/*
* Compare a "signer" (in the format of a Kerberos-format Kerberos5
* principal: host/example....@example.com) to the realm name stored
diff --git a/lib/dns/ssu.c b/lib/dns/ssu.c
index 7adb769cf3..abf8bb5c33 100644
--- a/lib/dns/ssu.c
+++ b/lib/dns/ssu.c
@@ -436,26 +436,30 @@ dns_ssutable_checkrules(dns_ssutable_t *table, dns_name_t *signer,
break;
case DNS_SSUMATCHTYPE_SELFKRB5:
if (!dst_gssapi_identitymatchesrealmkrb5(signer, name,
- rule->identity))
+ rule->identity,
+ ISC_FALSE))
continue;
break;
case DNS_SSUMATCHTYPE_SELFMS:
if (!dst_gssapi_identitymatchesrealmms(signer, name,
- rule->identity))
+ rule->identity,
+ ISC_FALSE))
continue;
break;
case DNS_SSUMATCHTYPE_SUBDOMAINKRB5:
if (!dns_name_issubdomain(name, rule->name))
continue;
- if (!dst_gssapi_identitymatchesrealmkrb5(signer, NULL,
- rule->identity))
+ if (!dst_gssapi_identitymatchesrealmkrb5(signer, name,
+ rule->identity,
+ ISC_TRUE))
continue;
break;
case DNS_SSUMATCHTYPE_SUBDOMAINMS:
if (!dns_name_issubdomain(name, rule->name))
continue;
- if (!dst_gssapi_identitymatchesrealmms(signer, NULL,
- rule->identity))
+ if (!dst_gssapi_identitymatchesrealmms(signer, name,
+ rule->identity,
+ ISC_TRUE))
continue;
break;
case DNS_SSUMATCHTYPE_TCPSELF:
diff --git a/lib/dns/gssapictx.c b/lib/dns/gssapictx.c
index bca9d3646b..2cf84ca0fd 100644
--- a/lib/dns/gssapictx.c
+++ b/lib/dns/gssapictx.c
@@ -347,7 +347,8 @@ cleanup:
bool
dst_gssapi_identitymatchesrealmkrb5(const dns_name_t *signer,
const dns_name_t *name,
- const dns_name_t *realm)
+ const dns_name_t *realm,
+ bool with_subdomains)
{
#ifdef GSSAPI
char sbuf[DNS_NAME_FORMATSIZE];
@@ -357,6 +358,7 @@ dst_gssapi_identitymatchesrealmkrb5(const dns_name_t *signer,
char *rname;
isc_buffer_t buffer;
isc_result_t result;
+ dns_fixedname_t signer_name;
/*
* It is far, far easier to write the names we are looking at into
@@ -366,8 +368,7 @@ dst_gssapi_identitymatchesrealmkrb5(const dns_name_t *signer,
result = dns_name_toprincipal(signer, &buffer);
RUNTIME_CHECK(result == ISC_R_SUCCESS);
isc_buffer_putuint8(&buffer, 0);
- if (name != NULL)
- dns_name_format(name, nbuf, sizeof(nbuf));
+ dns_name_format(name, nbuf, sizeof(nbuf));
dns_name_format(realm, rbuf, sizeof(rbuf));
/*
@@ -401,18 +402,49 @@ dst_gssapi_identitymatchesrealmkrb5(const dns_name_t *signer,
}
/*
- * Now, we do a simple comparison between the name and the realm.
+ * At this point in time, when we assume
+ * - Realm: EXAMPLE.COM
+ * - Signer:
+ * * KRB5: host/foo.example....@example.com
+ * * MS : FOO$@EXAMPLE.COM
+ * - Record to be updated: bar.example.com
+ * then we have:
+ * - sname: foo.Example.com (krb5) / foo (MS)
+ * - nbuf : bar.example.com
+ * - rname: EXAMPLE.COM
+ * So, for MS we have to reconstruct the full signer name.
*/
- if (name != NULL) {
- if ((strcasecmp(sname, nbuf) == 0)
- && (strcmp(rname, rbuf) == 0)) {
- return (true);
- }
- } else {
- if (strcmp(rname, rbuf) == 0) {
- return (true);
- }
- }
+
+ /*
+ * Now, compare the signer name with the name of the
+ * record to be updated. For subdomain updates, we do
+ * a subdomain checks, otherwise a simple comparison.
+ */
+ if (with_subdomains) {
+ /*
+ * Convert the signer's name back to a dns_name_t
+ * for later subdomain comparison.
+ */
+ dns_fixedname_init(&signer_name);
+ isc_buffer_constinit(&buffer, sname, strlen(sname));
+ isc_buffer_add(&buffer, strlen(sname));
+ result = dns_name_fromtext(dns_fixedname_name(&signer_name), &buffer,
+ dns_rootname, 0, NULL);
+ if (result != ISC_R_SUCCESS)
+ return (false);
+ /*
+ * Now we compare the signer's name with the requested name.
+ */
+ if (!dns_name_issubdomain(name, dns_fixedname_name(&signer_name)))
+ return (false);
+ } else if (strcasecmp(sname, nbuf) != 0)
+ return (false);
+
+ /*
+ * Lastly, we compare the realm.
+ */
+ if (strcmp(rname, rbuf) == 0)
+ return (true);
return (false);
#else
@@ -426,17 +458,18 @@ dst_gssapi_identitymatchesrealmkrb5(const dns_name_t *signer,
bool
dst_gssapi_identitymatchesrealmms(const dns_name_t *signer,
const dns_name_t *name,
- const dns_name_t *realm)
+ const dns_name_t *realm,
+ bool with_subdomains)
{
#ifdef GSSAPI
char sbuf[DNS_NAME_FORMATSIZE];
char nbuf[DNS_NAME_FORMATSIZE];
char rbuf[DNS_NAME_FORMATSIZE];
char *sname;
- char *nname;
char *rname;
isc_buffer_t buffer;
isc_result_t result;
+ dns_fixedname_t signer_name;
/*
* It is far, far easier to write the names we are looking at into
@@ -446,8 +479,7 @@ dst_gssapi_identitymatchesrealmms(const dns_name_t *signer,
result = dns_name_toprincipal(signer, &buffer);
RUNTIME_CHECK(result == ISC_R_SUCCESS);
isc_buffer_putuint8(&buffer, 0);
- if (name != NULL)
- dns_name_format(name, nbuf, sizeof(nbuf));
+ dns_name_format(name, nbuf, sizeof(nbuf));
dns_name_format(realm, rbuf, sizeof(rbuf));
/*
@@ -485,32 +517,54 @@ dst_gssapi_identitymatchesrealmms(const dns_name_t *signer,
sname = sbuf;
/*
- * Find the first . in the target name, and make it the end of
- * the string. The rest of the name has to match the realm.
+ * At this point in time, when we assume
+ * - Realm: EXAMPLE.COM
+ * - Signer:
+ * * KRB5: host/foo.example....@example.com
+ * * MS : FOO$@EXAMPLE.COM
+ * - Record to be updated: bar.example.com
+ * then we have:
+ * - sname: foo.Example.com (krb5) / foo (MS)
+ * - nbuf : bar.example.com
+ * - rname: EXAMPLE.COM
+ * So, for MS we have to reconstruct the full signer name.
*/
- if (name != NULL) {
- nname = strchr(nbuf, '.');
- if (nname == NULL) {
- return (false);
- }
- *nname++ = '\0';
- }
+
+ *sname++ = '.';
+ /* fits because foo.EXAMPLE.COM is shorter than FOO$@EXAMPLE.COM */
+ memmove(sname, rname, strlen(rname) + /* NUL */ 1);
+ sname = sbuf;
/*
- * Now, we do a simple comparison between the name and the realm.
+ * Now, compare the signer name with the name of the
+ * record to be updated. For subdomain updates, we do
+ * a subdomain checks, otherwise a simple comparison.
*/
- if (name != NULL) {
- if ((strcasecmp(sname, nbuf) == 0)
- && (strcmp(rname, rbuf) == 0)
- && (strcasecmp(nname, rbuf) == 0)) {
- return (true);
- }
- } else {
- if (strcmp(rname, rbuf) == 0) {
- return (true);
- }
- }
+ if (with_subdomains) {
+ /*
+ * Convert the signer's name back to a dns_name_t
+ * for later subdomain comparison.
+ */
+ dns_fixedname_init(&signer_name);
+ isc_buffer_constinit(&buffer, sname, strlen(sname));
+ isc_buffer_add(&buffer, strlen(sname));
+ result = dns_name_fromtext(dns_fixedname_name(&signer_name), &buffer,
+ dns_rootname, 0, NULL);
+ if (result != ISC_R_SUCCESS)
+ return (false);
+ /*
+ * Now we compare the signer's name with the requested name.
+ */
+ if (!dns_name_issubdomain(name, dns_fixedname_name(&signer_name)))
+ return (false);
+ } else if (strcasecmp(sname, nbuf) != 0)
+ return (false);
+ /*
+ * Lastly, we compare the realm.
+ */
+ if (strcmp(rname, rbuf) == 0)
+ return (true);
return (false);
#else
diff --git a/lib/dns/include/dst/gssapi.h b/lib/dns/include/dst/gssapi.h
index e08dee2801..5c49cd3a5e 100644
--- a/lib/dns/include/dst/gssapi.h
+++ b/lib/dns/include/dst/gssapi.h
@@ -189,7 +189,8 @@ gss_error_tostring(uint32_t major, uint32_t minor,
bool
dst_gssapi_identitymatchesrealmkrb5(const dns_name_t *signer,
const dns_name_t *name,
- const dns_name_t *realm);
+ const dns_name_t *realm,
+ bool with_subdomains);
/*
* Compare a "signer" (in the format of a Kerberos-format Kerberos5
* principal: host/example....@example.com) to the realm name stored
@@ -200,7 +201,8 @@ dst_gssapi_identitymatchesrealmkrb5(const dns_name_t *signer,
bool
dst_gssapi_identitymatchesrealmms(const dns_name_t *signer,
const dns_name_t *name,
- const dns_name_t *realm);
+ const dns_name_t *realm,
+ bool with_subdomains);
/*
* Compare a "signer" (in the format of a Kerberos-format Kerberos5
* principal: host/example....@example.com) to the realm name stored
diff --git a/lib/dns/ssu.c b/lib/dns/ssu.c
index 7cd664c75e..985f77b835 100644
--- a/lib/dns/ssu.c
+++ b/lib/dns/ssu.c
@@ -458,26 +458,30 @@ dns_ssutable_checkrules(dns_ssutable_t *table, const dns_name_t *signer,
break;
case dns_ssumatchtype_selfkrb5:
if (!dst_gssapi_identitymatchesrealmkrb5(signer, name,
- rule->identity))
+ rule->identity,
+ ISC_FALSE))
continue;
break;
case dns_ssumatchtype_selfms:
if (!dst_gssapi_identitymatchesrealmms(signer, name,
- rule->identity))
+ rule->identity,
+ ISC_FALSE))
continue;
break;
case dns_ssumatchtype_subdomainkrb5:
if (!dns_name_issubdomain(name, rule->name))
continue;
- if (!dst_gssapi_identitymatchesrealmkrb5(signer, NULL,
- rule->identity))
+ if (!dst_gssapi_identitymatchesrealmkrb5(signer, name,
+ rule->identity,
+ ISC_TRUE))
continue;
break;
case dns_ssumatchtype_subdomainms:
if (!dns_name_issubdomain(name, rule->name))
continue;
- if (!dst_gssapi_identitymatchesrealmms(signer, NULL,
- rule->identity))
+ if (!dst_gssapi_identitymatchesrealmms(signer, name,
+ rule->identity,
+ ISC_TRUE))
continue;
break;
case dns_ssumatchtype_tcpself:
signature.asc
Description: PGP signature
