diff --git a/CHANGES b/CHANGES index 8812bb6b8e..04305d1a74 100644 --- a/CHANGES +++ b/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] diff --git a/bin/dnssec/dnssec-importkey.c b/bin/dnssec/dnssec-importkey.c index 0a195c4ea9..563dc09948 100644 --- a/bin/dnssec/dnssec-importkey.c +++ b/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 \n"); + fprintf(stderr, " -f file: read key from zone file\n"); fprintf(stderr, " -K : 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 \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]; diff --git a/bin/dnssec/dnssec-importkey.docbook b/bin/dnssec/dnssec-importkey.docbook index e10ecb4b2f..f9b322c884 100644 --- a/bin/dnssec/dnssec-importkey.docbook +++ b/bin/dnssec/dnssec-importkey.docbook @@ -44,22 +44,45 @@ dnssec-importkey - + - + + + + dnssec-importkey + + + + + + + + DESCRIPTION dnssec-importkey - read a DNSKEY record and generated a .key/.private key pair. - Publication () and deletions () - 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. + + + The newly-created .private file does not + contain private key data, and cannot be used for signing. + However, having a .private file makes it possible to set + publication () and deletion + () 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. @@ -70,9 +93,16 @@ -f filename - - Filename to read the key from. - + + 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 . If the domain name is the same as + , then it may be omitted. + + + If is set to "-", then + the zone data is read from the standard input. + @@ -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 0 or none removes it. @@ -159,6 +189,16 @@ + + FILES + + A keyfile can be designed by the key identification + Knnnn.+aaa+iiiii or the full file name + Knnnn.+aaa+iiiii.key as generated by + dnssec-keygen8. + + + SEE ALSO diff --git a/bin/tests/system/inline/clean.sh b/bin/tests/system/inline/clean.sh index 412031ace7..4913c733b3 100644 --- a/bin/tests/system/inline/clean.sh +++ b/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* diff --git a/bin/tests/system/inline/ns3/sign.sh b/bin/tests/system/inline/ns3/sign.sh index 515c58913a..d606fe75db 100644 --- a/bin/tests/system/inline/ns3/sign.sh +++ b/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 diff --git a/bin/tests/system/inline/tests.sh b/bin/tests/system/inline/tests.sh index 9b3bf05ad5..f975c26ae7 100644 --- a/bin/tests/system/inline/tests.sh +++ b/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