dnsjava-users

Bug in SIG(0) signature generation in 2.1.5

[Tekto <te...@ge...>]

Hi,

-- Description --

SIG0.signMessage in Version 2.1.5 is calculating a wrong signature. Sending
following update-request to a server will produce a SERVFAIL:

	SimpleResolver resolver = new SimpleResolver("somedns.example.invalid.");
	Name sig0zoneName = new Name("sig0.invalid.");
	Name sig0hostName = new Name("sometext.sig0.invalid.");

	KeyFactory keyFactory = KeyFactory.getInstance("RSA");
	RSAPrivateKey privKey = getPrivateZoneCrtKey(keyFactory);
	RSAPublicKey pubKey = getPublicZoneKey(keyFactory);

	KEYRecord keyRecord = new KEYRecord(sig0zoneName, DClass.IN, 0, Flags.HOST, Protocol.DNSSEC, Algorithm.RSASHA1, pubKey);		
	TXTRecord txtRecord = new TXTRecord(sig0hostName, DClass.IN, 0, "Hello World!");
	Update updateMessage = new Update(sig0zoneName);
	updateMessage.add(txtRecord);

	SIG0.signMessage(updateMessage, keyRecord, privKey, null);


-- How to reproduce --

Use SIG0.verifyMessage and try to verify updateMessage.toWire() which will
throw a DNSSEC.DNSSECException.

	SIG0.verifyMessage(updateMessage, updateMessage.toWire(), keyRecord, null);

-- Reason --
	
in DNSSEC.java signMessage(Message message, SIGRecord previous, KEYRecord key,
  PrivateKey privkey, Date inception, Date expiration)
update message is added to the same DNSOutput-Buffer as the introducing SIG-RDATA.
Therefore name compression calculation in update-section will refer to a wrong
offset (Start of to-sign data block instead of the start of the message will be
truly send over the wire. Verification on the server counterpart will fail.
(tested against BIND 9.7.3)

-- How to fix it --

Changing line 955 (in Version 2.1.5) in DNSSEC.java from

	message.toWire(out);

to
	
	out.writeByteArray(message.toWire());

will fix this behaviour since it uses a second, independent DNSOutput-buffer for
calculation.


Regards,
	Adam

Re: Bug in SIG(0) signature generation in 2.1.5

[Brian W. <bwe...@xb...>]

On Jul 26, 2013, at 9:56 AM, Tekto <te...@ge...> wrote:

> Hi,
> 
> -- Description --
> 
> SIG0.signMessage in Version 2.1.5 is calculating a wrong signature. Sending
> following update-request to a server will produce a SERVFAIL:
> 
> 	SimpleResolver resolver = new SimpleResolver("somedns.example.invalid.");
> 	Name sig0zoneName = new Name("sig0.invalid.");
> 	Name sig0hostName = new Name("sometext.sig0.invalid.");
> 
> 	KeyFactory keyFactory = KeyFactory.getInstance("RSA");
> 	RSAPrivateKey privKey = getPrivateZoneCrtKey(keyFactory);
> 	RSAPublicKey pubKey = getPublicZoneKey(keyFactory);
> 
> 	KEYRecord keyRecord = new KEYRecord(sig0zoneName, DClass.IN, 0, Flags.HOST, Protocol.DNSSEC, Algorithm.RSASHA1, pubKey);		
> 	TXTRecord txtRecord = new TXTRecord(sig0hostName, DClass.IN, 0, "Hello World!");
> 	Update updateMessage = new Update(sig0zoneName);
> 	updateMessage.add(txtRecord);
> 
> 	SIG0.signMessage(updateMessage, keyRecord, privKey, null);
> 
> 
> -- How to reproduce --
> 
> Use SIG0.verifyMessage and try to verify updateMessage.toWire() which will
> throw a DNSSEC.DNSSECException.
> 
> 	SIG0.verifyMessage(updateMessage, updateMessage.toWire(), keyRecord, null);

I assume there's some missing code between the signMessage and verifyMessage?  Passing the same Message object to both signMessage and verifyMessage doesn't work, since some of the internal Message state needed to verify is only filled in when the Message is parsed from wire format; that is, something like:

SIG0.signMessage(updateMessage, keyRecord, privKey, null);
byte [] wire = msg.toWire();
Message msg = new Message(wire);
SIG0.verifyMessage(msg, msg.toWire(), keyRecord, null);

Without that, the code throws an ArrayIndexOutOfBoundsException, because it's badly handling the fact that the Message doesn't know where the start of the SIG0 record is (I've locally updated the code to throw a more useful NoSignatureException).

With code like that added, I'm seeing the failure, although I'm not sure why I'm not seeing the same failure in the existing SIG0 test code I have.

I'm going to try and adapt this sample code into a unit test, and commit the changes in the next few days.

Thanks!

Brian

Re: Bug in SIG(0) signature generation in 2.1.5

[Tekto <te...@ge...>]

Hi Brian,

On 07/26/2013 09:06 PM, Brian Wellington wrote:>
 >
 > I assume there's some missing code between the signMessage and 
verifyMessage?
 > Passing the same Message object to both signMessage and verifyMessage
 > doesn't work, since some of the internal Message state needed to
 > verify is only filled in when the Message is parsed from wire format;
 > that is, something like:

Yes, you're right. I'd tried misc. thing to fiddle that out (including 
injecting captured requests with nsupdate) so I didn't remember
for sure the last steps.

 > SIG0.signMessage(updateMessage, keyRecord, privKey, null);
 > byte [] wire = msg.toWire();
 > Message msg = new Message(wire);
 > SIG0.verifyMessage(msg, msg.toWire(), keyRecord, null);
 >
 > Without that, the code throws an ArrayIndexOutOfBoundsException, 
because it's badly handling the fact that the Message doesn't know where 
the start of the SIG0 record is (I've locally updated the code to throw 
a more useful NoSignatureException).
 >
 > With code like that added, I'm seeing the failure, although I'm not 
sure why I'm not seeing the same failure in the existing SIG0 test code 
I have.
 >
 > I'm going to try and adapt this sample code into a unit test, and 
commit the changes in the next few days.

If you like, I can send you the test-key and host a used.

 >
 > Thanks!
 >
 > Brian
 >

Bye, Adam

Re: Bug in SIG(0) signature generation in 2.1.5

[Brian W. <bwe...@xb...>]

On Jul 31, 2013, at 5:09 AM, Tekto <te...@ge...> wrote:

> Hi Brian,
> 
> On 07/26/2013 09:06 PM, Brian Wellington wrote:>
> >
> > I assume there's some missing code between the signMessage and verifyMessage?
> > Passing the same Message object to both signMessage and verifyMessage
> > doesn't work, since some of the internal Message state needed to
> > verify is only filled in when the Message is parsed from wire format;
> > that is, something like:
> 
> Yes, you're right. I'd tried misc. thing to fiddle that out (including injecting captured requests with nsupdate) so I didn't remember
> for sure the last steps.

No problem.

> > SIG0.signMessage(updateMessage, keyRecord, privKey, null);
> > byte [] wire = msg.toWire();
> > Message msg = new Message(wire);
> > SIG0.verifyMessage(msg, msg.toWire(), keyRecord, null);
> >
> > Without that, the code throws an ArrayIndexOutOfBoundsException, because it's badly handling the fact that the Message doesn't know where the start of the SIG0 record is (I've locally updated the code to throw a more useful NoSignatureException).
> >
> > With code like that added, I'm seeing the failure, although I'm not sure why I'm not seeing the same failure in the existing SIG0 test code I have.
> >
> > I'm going to try and adapt this sample code into a unit test, and commit the changes in the next few days.
> 
> If you like, I can send you the test-key and host a used.

I committed the fix and a unit test yesterday.  Hopefully I'll have time to do a new release sometime in the next few days.

Thanks again!

Brian