Allow all valid key names

TSIG keys names need to be able to be set to any valid name so that
update self rules can work for any valid name.  Restore this ability
to the key generating tool while preventing rndc.conf and named.conf
from being compromised due to specially crafted key names.

diff --git a/bin/confgen/keygen.c b/bin/confgen/keygen.c
index e23bd92fb680619727f0ec266766321db8ceb470..40123b15b8ad342e4e1c0dc5dadd462bd7eec493 100644 (file)
--- a/bin/confgen/keygen.c
+++ b/bin/confgen/keygen.c
@@ -91,23 +91,30 @@ alg_bits(dns_secalg_t alg) {
 }
 
 /*%
- * Reject key names that would not embed safely into a named.conf
- * 'key "<name>" { ... };' clause. Allowed: alphanumerics, '.', '-', '_'.
+ * Reject invalid key names and make them safe for embedding into
+ * rndc.conf and named.conf.
  */
 void
-validate_keyname(const char *keyname) {
+makesafe_keyname(const char *keyname, char *namebuf, size_t length) {
        dns_fixedname_t fixed;
        dns_name_t *name = dns_fixedname_initname(&fixed);
        isc_result_t result;
+       isc_buffer_t b;
 
        if (keyname == NULL || keyname[0] == '\0') {
                fatal("key name must not be empty");
        }
-
        result = dns_name_fromstring(name, keyname, dns_rootname, 0, NULL);
        if (result != ISC_R_SUCCESS) {
                fatal("invalid key name: %s", isc_result_totext(result));
        }
+
+       isc_buffer_init(&b, namebuf, length);
+       result = dns_name_totext(name, DNS_NAME_QUOTED | DNS_NAME_OMITFINALDOT,
+                                &b);
+       if (result != ISC_R_SUCCESS) {
+               fatal("invalid key name: %s", isc_result_totext(result));
+       }
 }
 
 /*%

diff --git a/bin/confgen/keygen.h b/bin/confgen/keygen.h
index 711ec195e0e51ed3bbf5048107251625c8801e70..97be9b4e1ccc773051cdb41f2da01fa0a565e12c 100644 (file)
--- a/bin/confgen/keygen.h
+++ b/bin/confgen/keygen.h
@@ -21,7 +21,7 @@
 #include <dns/secalg.h>
 
 void
-validate_keyname(const char *keyname);
+makesafe_keyname(const char *keyname, char *namebuf, size_t length);
 
 void
 generate_key(isc_mem_t *mctx, dns_secalg_t alg, int keysize,

diff --git a/bin/confgen/rndc-confgen.c b/bin/confgen/rndc-confgen.c
index 470b7fe8082ec01812aca9f008b1cdeabb197bcd..5ddede48ad96a1d6ee9c3c41512ba98804e6c114 100644 (file)
--- a/bin/confgen/rndc-confgen.c
+++ b/bin/confgen/rndc-confgen.c
@@ -87,6 +87,7 @@ main(int argc, char **argv) {
        bool show_final_mem = false;
        isc_buffer_t key_txtbuffer;
        char key_txtsecret[256];
+       char namebuf[DNS_NAME_FORMATSIZE];
        const char *keyname = NULL;
        const char *serveraddr = NULL;
        dns_secalg_t alg;
@@ -207,7 +208,7 @@ main(int argc, char **argv) {
                usage(EXIT_FAILURE);
        }
 
-       validate_keyname(keyname);
+       makesafe_keyname(keyname, namebuf, sizeof(namebuf));
 
        if (alg == DST_ALG_HMACMD5) {
                fprintf(stderr, "warning: use of hmac-md5 for RNDC keys "
@@ -226,7 +227,7 @@ main(int argc, char **argv) {
 
        if (keyonly) {
                write_key_file(keyfile, chrootdir == NULL ? user : NULL,
-                              keyname, &key_txtbuffer, alg);
+                              namebuf, &key_txtbuffer, alg);
                if (!quiet) {
                        printf("wrote key file \"%s\"\n", keyfile);
                }
@@ -238,7 +239,7 @@ main(int argc, char **argv) {
                        snprintf(buf, len, "%s%s%s", chrootdir,
                                 (*keyfile != '/') ? "/" : "", keyfile);
 
-                       write_key_file(buf, user, keyname, &key_txtbuffer, alg);
+                       write_key_file(buf, user, namebuf, &key_txtbuffer, alg);
                        if (!quiet) {
                                printf("wrote key file \"%s\"\n", buf);
                        }
@@ -270,13 +271,13 @@ options {\n\
 #              allow { %s; } keys { \"%s\"; };\n\
 # };\n\
 # End of named.conf\n",
-                      keyname, algname,
+                      namebuf, algname,
                       (int)isc_buffer_usedlength(&key_txtbuffer),
-                      (char *)isc_buffer_base(&key_txtbuffer), keyname,
-                      serveraddr, port, keyname, algname,
+                      (char *)isc_buffer_base(&key_txtbuffer), namebuf,
+                      serveraddr, port, namebuf, algname,
                       (int)isc_buffer_usedlength(&key_txtbuffer),
                       (char *)isc_buffer_base(&key_txtbuffer), serveraddr,
-                      port, serveraddr, keyname);
+                      port, serveraddr, namebuf);
        }
 
        if (show_final_mem) {

diff --git a/bin/confgen/rndc-confgen.rst b/bin/confgen/rndc-confgen.rst
index 0a91489c48923bcc09477e6845c31277dec1977a..3864ec4474dd3df820056597bcb924e6faae675f 100644 (file)
--- a/bin/confgen/rndc-confgen.rst
+++ b/bin/confgen/rndc-confgen.rst
@@ -72,8 +72,10 @@ Options
 
 .. option:: -k keyname
 
-   This option specifies the key name of the :iscman:`rndc` authentication key. This must be a
-   valid domain name. The default is ``rndc-key``.
+   This option specifies the key name of the :iscman:`rndc`
+   authentication key. This must be a valid domain name and will
+   be sanitized using the domain name semantics. The default is
+   ``rndc-key``.
 
 .. option:: -p port
 

diff --git a/bin/confgen/tsig-keygen.c b/bin/confgen/tsig-keygen.c
index 9e11b22eb8c2f8ebcd2786750f0c134198448fb5..c6af0ed7b6a1e87b8154fe4fbd900cfb3d482832 100644 (file)
--- a/bin/confgen/tsig-keygen.c
+++ b/bin/confgen/tsig-keygen.c
@@ -84,6 +84,7 @@ main(int argc, char **argv) {
        bool quiet = false;
        isc_buffer_t key_txtbuffer;
        char key_txtsecret[256];
+       char namebuf[DNS_NAME_FORMATSIZE];
        const char *keyname = NULL;
        const char *zone = NULL;
        const char *self_domain = NULL;
@@ -212,7 +213,7 @@ main(int argc, char **argv) {
                }
        }
 
-       validate_keyname(keyname);
+       makesafe_keyname(keyname, namebuf, sizeof(namebuf));
 
        isc_buffer_init(&key_txtbuffer, &key_txtsecret, sizeof(key_txtsecret));
 
@@ -230,7 +231,7 @@ key \"%s\" {\n\
        algorithm %s;\n\
        secret \"%.*s\";\n\
 };\n",
-              keyname, algname, (int)isc_buffer_usedlength(&key_txtbuffer),
+              namebuf, algname, (int)isc_buffer_usedlength(&key_txtbuffer),
               (char *)isc_buffer_base(&key_txtbuffer));
 
        if (!quiet) {
@@ -242,7 +243,7 @@ key \"%s\" {\n\
 update-policy {\n\
          grant %s name %s ANY;\n\
 };\n",
-                              self_domain, keyname, self_domain);
+                              self_domain, namebuf, self_domain);
                } else if (zone != NULL) {
                        printf("\n\
 # Then, in the \"zone\" definition statement for \"%s\",\n\
@@ -251,7 +252,7 @@ update-policy {\n\
 update-policy {\n\
          grant %s zonesub ANY;\n\
 };\n",
-                              zone, keyname);
+                              zone, namebuf);
                } else {
                        printf("\n\
 # Then, in the \"zone\" statement for each zone you wish to dynamically\n\
@@ -261,7 +262,7 @@ update-policy {\n\
 update-policy {\n\
        grant %s zonesub ANY;\n\
 };\n",
-                              keyname);
+                              namebuf);
                }
 
                printf("\n\