3671. [bug] Don't allow dnssec-importkey overwrite a existing

non-imported private key.

CHANGES

@@ -1,3 +1,6 @@
+3671.	[bug]		Don't allow dnssec-importkey overwrite a existing
+			non-imported private key.
+
 3670.	[bug]		Address read after free in server side of
 			lwres_getrrsetbyname. [RT #29075]
 

bin/dnssec/dnssec-importkey.c

@@ -61,7 +61,9 @@ static dns_fixedname_t	fixed;
 static dns_name_t	*name = NULL;
 static isc_mem_t	*mctx = NULL;
 static isc_boolean_t	setpub = ISC_FALSE, setdel = ISC_FALSE;
+static isc_boolean_t	setttl = ISC_FALSE;
 static isc_stdtime_t	pub = 0, del = 0;
+static dns_ttl_t	ttl = 0;
 
 static isc_result_t
 initname(char *setname) {
@@ -190,9 +192,10 @@ static void
 emit(const char *dir, dns_rdata_t *rdata) {
 	isc_result_t result;
 	char keystr[DST_KEY_FORMATSIZE];
-	char newname[1024];
+	char pubname[1024];
+	char priname[1024];
 	isc_buffer_t buf;
-	dst_key_t *key = NULL;
+	dst_key_t *key = NULL, *tmp = NULL;
 
 	isc_buffer_init(&buf, rdata->data, rdata->length);
 	isc_buffer_add(&buf, rdata->length);
@@ -201,18 +204,36 @@ emit(const char *dir, dns_rdata_t *rdata) {
 		fatal("dst_key_fromdns: %s", isc_result_totext(result));
 	}
 
-	dst_key_setexternal(key, ISC_TRUE);
-	if (setpub)
-		dst_key_settime(key, DST_TIME_PUBLISH, pub);
-	if (setdel)
-		dst_key_settime(key, DST_TIME_DELETE, del);
-
-	isc_buffer_init(&buf, newname, sizeof(newname));
+	isc_buffer_init(&buf, pubname, sizeof(pubname));
 	result = dst_key_buildfilename(key, DST_TYPE_PUBLIC, dir, &buf);
 	if (result != ISC_R_SUCCESS) {
 		fatal("Failed to build public key filename: %s",
 		      isc_result_totext(result));
 	}
+	isc_buffer_init(&buf, priname, sizeof(priname));
+	result = dst_key_buildfilename(key, DST_TYPE_PRIVATE, dir, &buf);
+	if (result != ISC_R_SUCCESS) {
+		fatal("Failed to build private key filename: %s",
+		      isc_result_totext(result));
+	}
+
+	result = dst_key_fromfile(dst_key_name(key), dst_key_id(key),
+				  dst_key_alg(key),
+				  DST_TYPE_PUBLIC | DST_TYPE_PRIVATE,
+				  dir, mctx, &tmp);
+	if (result == ISC_R_SUCCESS) {
+		if (dst_key_isprivate(tmp) && !dst_key_isexternal(tmp))
+			fatal("Private key already exists in %s", priname);
+		dst_key_free(&tmp);
+	}
+
+	dst_key_setexternal(key, ISC_TRUE);
+	if (setpub)
+		dst_key_settime(key, DST_TIME_PUBLISH, pub);
+	if (setdel)
+		dst_key_settime(key, DST_TIME_DELETE, del);
+	if (setttl)
+		dst_key_setttl(key, ttl);
 
 	result = dst_key_tofile(key, DST_TYPE_PUBLIC|DST_TYPE_PRIVATE,
 				dir);
@@ -221,8 +242,7 @@ emit(const char *dir, dns_rdata_t *rdata) {
 		fatal("Failed to write key %s: %s", keystr,
 		      isc_result_totext(result));
 	}
-
-	printf("%s\n", newname);
+	printf("%s\n", pubname);
 
 	isc_buffer_clear(&buf);
 	result = dst_key_buildfilename(key, DST_TYPE_PRIVATE, dir, &buf);
@@ -230,7 +250,7 @@ emit(const char *dir, dns_rdata_t *rdata) {
 		fatal("Failed to build private key filename: %s",
 		      isc_result_totext(result));
 	}
-	printf("%s\n", newname);
+	printf("%s\n", priname);
 	dst_key_free(&key);
 }
 
@@ -240,13 +260,21 @@ usage(void) ISC_PLATFORM_NORETURN_POST;
 static void
 usage(void) {
 	fprintf(stderr, "Usage:\n");
-	fprintf(stderr,	"    %s options [-K dir] file\n\n", program);
+	fprintf(stderr,	"    %s options [-K dir] keyfile\n\n", program);
+	fprintf(stderr, "    %s options -f file [keyname]\n\n", program);
 	fprintf(stderr, "Version: %s\n", VERSION);
 	fprintf(stderr, "Options:\n");
-	fprintf(stderr, "    -v <verbose level>\n");
+	fprintf(stderr, "    -f file: read key from zone file\n");
 	fprintf(stderr, "    -K <directory>: directory in which to store "
-			"the keyset files\n");
-	fprintf(stderr, "    -f file: read keyset from zone file\n");
+				"the key files\n");
+	fprintf(stderr, "    -L ttl:             set default key TTL\n");
+	fprintf(stderr, "    -v <verbose level>\n");
+	fprintf(stderr, "    -h: print usage and exit\n");
+	fprintf(stderr, "Timing options:\n");
+	fprintf(stderr, "    -P date/[+-]offset/none: set/unset key "
+						     "publication date\n");
+	fprintf(stderr, "    -D date/[+-]offset/none: set/unset key "
+						     "deletion date\n");
 
 	exit (-1);
 }
@@ -278,7 +306,8 @@ main(int argc, char **argv) {
 
 	isc_commandline_errprint = ISC_FALSE;
 
-	while ((ch = isc_commandline_parse(argc, argv, "D:f:hK:P:v:")) != -1) {
+#define CMDLINE_FLAGS "D:f:hK:L:P:v:"
+	while ((ch = isc_commandline_parse(argc, argv, CMDLINE_FLAGS)) != -1) {
 		switch (ch) {
 		case 'D':
 			if (setdel)
@@ -292,6 +321,13 @@ main(int argc, char **argv) {
 			if (strlen(dir) == 0U)
 				fatal("directory must be non-empty string");
 			break;
+		case 'L':
+			if (strcmp(isc_commandline_argument, "none") == 0)
+				ttl = 0;
+			else
+				ttl = strtottl(isc_commandline_argument);
+			setttl = ISC_TRUE;
+			break;
 		case 'P':
 			if (setpub)
 				fatal("-P specified more than once");
@@ -346,8 +382,8 @@ main(int argc, char **argv) {
 	dns_rdataset_init(&rdataset);
 
 	if (filename != NULL) {
-		if (argc < isc_commandline_index + 1 && filename != NULL) {
-			/* using zone name as the zone file name */
+		if (argc < isc_commandline_index + 1) {
+			/* using filename as zone name */
 			namestr = filename;
 		} else
 			namestr = argv[isc_commandline_index];

bin/dnssec/dnssec-importkey.docbook

@@ -44,22 +44,45 @@
   <refsynopsisdiv>
     <cmdsynopsis>
       <command>dnssec-importkey</command>
-      <arg><option>-f <replaceable class="parameter">filename</replaceable></option></arg>
       <arg><option>-K <replaceable class="parameter">directory</replaceable></option></arg>
+      <arg><option>-L <replaceable class="parameter">ttl</replaceable></option></arg>
       <arg><option>-P <replaceable class="parameter">date/offset</replaceable></option></arg>
       <arg><option>-D <replaceable class="parameter">date/offset</replaceable></option></arg>
       <arg><option>-h</option></arg>
       <arg><option>-v <replaceable class="parameter">level</replaceable></option></arg>
-      <arg><option>keyname</option></arg>
+      <arg choice="req"><option>keyfile</option></arg>
+    </cmdsynopsis>
+    <cmdsynopsis>
+      <command>dnssec-importkey</command>
+      <arg choice="req"><option>-f <replaceable class="parameter">filename</replaceable></option></arg>
+      <arg><option>-K <replaceable class="parameter">directory</replaceable></option></arg>
+      <arg><option>-L <replaceable class="parameter">ttl</replaceable></option></arg>
+      <arg><option>-P <replaceable class="parameter">date/offset</replaceable></option></arg>
+      <arg><option>-D <replaceable class="parameter">date/offset</replaceable></option></arg>
+      <arg><option>-h</option></arg>
+      <arg><option>-v <replaceable class="parameter">level</replaceable></option></arg>
+      <arg><option>dnsname</option></arg>
     </cmdsynopsis>
   </refsynopsisdiv>
 
   <refsect1>
     <title>DESCRIPTION</title>
     <para><command>dnssec-importkey</command>
-      read a DNSKEY record and generated a .key/.private key pair.
-      Publication (<option>-P</option>) and deletions (<option>-D</option>)
-      times can be set for the key.
+      reads a public DNSKEY record and generates a pair of
+      .key/.private files.  The DNSKEY record may be read from an
+      existing .key file, in which case a corresponding .private file
+      will be generated, or it may be read from any other file or
+      from the standard input, in which case both .key and .private
+      files will be generated.
+    </para>
+    <para>
+      The newly-created .private file does <emphasis>not</command>
+      contain private key data, and cannot be used for signing.
+      However, having a .private file makes it possible to set
+      publication (<option>-P</option>) and deletion
+      (<option>-D</option>) times for the key, which means the
+      public key can be added to and removed from the DNSKEY RRset
+      on schedule even if the true private key is stored offline.
     </para>
   </refsect1>
 
@@ -70,9 +93,16 @@
       <varlistentry>
 	<term>-f <replaceable class="parameter">filename</replaceable></term>
         <listitem>
-	  <para>
-	    Filename to read the key from.
-	  </para>
+          <para>
+            Zone file mode: instead of a public keyfile name, the argument
+	    is the DNS domain name of a zone master file, which can be read
+            from <option>file</option>.  If the domain name is the same as
+            <option>file</option>, then it may be omitted.
+          </para>
+          <para>
+            If <option>file</option> is set to <literal>"-"</literal>, then
+            the zone data is read from the standard input.
+          </para>
         </listitem>
       </varlistentry>
   
@@ -93,7 +123,7 @@
             into a DNSKEY RR.  If the key is imported into a zone,
             this is the TTL that will be used for it, unless there was
             already a DNSKEY RRset in place, in which case the existing TTL
-            would take precedence.  importkey the default TTL to
+            would take precedence.  Setting the default TTL to
             <literal>0</literal> or <literal>none</literal> removes it.
           </para>
         </listitem>
@@ -159,6 +189,16 @@
     </variablelist>
   </refsect1>
 
+  <refsect1>
+    <title>FILES</title>
+    <para>
+      A keyfile can be designed by the key identification
+      <filename>Knnnn.+aaa+iiiii</filename> or the full file name
+      <filename>Knnnn.+aaa+iiiii.key</filename> as generated by
+      <refentrytitle>dnssec-keygen</refentrytitle><manvolnum>8</manvolnum>.
+    </para>
+  </refsect1>
+
   <refsect1>
     <title>SEE ALSO</title>
     <para><citerefentry>

bin/tests/system/inline/clean.sh

@@ -82,3 +82,4 @@ rm -f */*.nzf
 rm -f ns3/test-?.bk
 rm -f ns3/test-?.bk.signed
 rm -f ns3/test-?.bk.signed.jnl
+rm -f import.key Kimport*

bin/tests/system/inline/ns3/sign.sh

@@ -128,7 +128,9 @@ rm -f ${k3}.* ${k4}.*
 #
 # Convert k1 and k2 in to External Keys.
 rm -f $k1.private 
-$IMPORTKEY -P now -D now+3600 -f $k1.key $zone > /dev/null 2>&1
+mv $k1.key a-file
+$IMPORTKEY -P now -D now+3600 -f a-file $zone > /dev/null 2>&1
 rm -f $k2.private 
-$IMPORTKEY -f $k2.key $zone > /dev/null 2>&1
+mv $k2.key a-file
+$IMPORTKEY -f a-file $zone > /dev/null 2>&1
 done

bin/tests/system/inline/tests.sh

@@ -847,4 +847,19 @@ do
 done
 status=`expr $status + $ret`
 
+n=`expr $n + 1`
+echo "I:testing imported key won't overwrite a private key ($n)"
+ret=0
+key=`$KEYGEN -r $RANDFILE -q import.example`
+cp ${key}.key import.key
+# import should fail
+$IMPORTKEY -f import.key import.example > /dev/null 2>&1 && ret=1
+rm -f ${key}.private
+# private key removed; import should now succeed
+$IMPORTKEY -f import.key import.example > /dev/null 2>&1 || ret=1
+# now that it's an external key, re-import should succeed
+$IMPORTKEY -f import.key import.example > /dev/null 2>&1 || ret=1
+if [ $ret != 0 ]; then echo "I:failed"; fi
+status=`expr $status + $ret`
+
 exit $status