dnssec-signzone and jitter bug... still

[Paul Wouters]

On Tue, 1 Nov 2011, Paul Wouters wrote:

>> There have been discussions in the past over this, but we were once again 
>> bitten by this dnssec-signzone bug:
>> 
>> Tue Nov  1 12:11:28 2011 signDomain: sign command: 
>> /usr/sbin/dnssec-signzone -C -u -r /dev/random -t -o openswan.org  -f 
>> /var/tmp/openswan.org.sign.tmp -i 1296000  -e +2592000 -j 1296000  -k 
>> Kopenswan.org.+005+07398 /var/tmp/dnsx_sign_domain_openswan.org.31202 
>> Kopenswan.org.+005+64562
>
>> Error: DNSSEC signature has expired for openswan.org.	AAAA
>
>> This signature expires at Nov 4, in three(!) days. The signature was 
>> generated on Oct 8,
>> and all this time dnssec-signzone thinks it is valid to retain it, while 
>> clearly being
>> outside the -i interval window.
>> 
>> I again have to conclude that jittering is not correctly implemented. It 
>> seems jittering
>> is done AFTER determining the valid start/end times via the -s/-e/-i 
>> options.
>> 
>> To me, the above signing commands means "all RRSIGs should be valid from 
>> -1h to at the
>> very least +1296000 seconds and at most +2592000 seconds, spread out"
>> 
>> Currently however, it seems a valid end time is (minimum lifetime) - 
>> (jitter), which in my
>> case means "0" if you want to jitter between +2w and +4w.
>> 
>> This is dnssec-signzone from 9.7.3
>
> I just confirmed this bug with 9.9.0a3 as well.

My issue is resolved with the following patch. (the cycle variable contains
the contents of the dnssec-signzone -i option)

--- ./bin/dnssec/dnssec-signzone.c	2011-11-01 18:39:53.000000000 -0400
+++ ./bin/dnssec/dnssec-signzone.c.new	2011-11-01 18:41:16.000000000 -0400
@@ -537,10 +537,7 @@

  		key = keythatsigned(&rrsig);
  		sig_format(&rrsig, sigstr, sizeof(sigstr));
-		if (key != NULL && issigningkey(key))
-			expired = isc_serial_gt(now + cycle, rrsig.timeexpire);
-		else
-			expired = isc_serial_gt(now, rrsig.timeexpire);
+		expired = isc_serial_gt(now + cycle, rrsig.timeexpire);

  		if (isc_serial_gt(rrsig.timesigned, rrsig.timeexpire)) {
  			/* rrsig is dropped and not replaced */

I don't understand exactly what the code was trying to do. Perhaps it tried to
roll keys faster? In which case it might need now + cycle + something to keep
that logic intact compared to non-key records.

Paul