miekg / dns

DNS library in Go
https://miek.nl/2014/august/16/go-dns-package
BSD 3-Clause "New" or "Revised" License
8.09k stars 1.14k forks source link

Error in tsig.go when validating tsig.TimeSigned #153

Closed freb closed 10 years ago

freb commented 10 years ago

The line in question is: https://github.com/miekg/dns/blob/master/tsig.go#L228

I noticed one of the devices I'm working with is throwing "bad time" errors even though the clocks were only 1 second apart. After doing some print statements, when the error was thrown the ti variable value was equal to: 18446744073709551615

I'm assuming that is the maximum size of a uint64 and the number is wrapping when the signature on the msg is larger than the current time.

I'm new to Go and surprisingly haven't yet figure out a good way to find the difference.

Thanks,

andrewtj commented 10 years ago

Can you provide the message bytes? Is the device the FortiGate firewall you mentioned in the other thread? And that's sending Update requests to a server written in go?

freb commented 10 years ago

I used dns.Msg.Pack() on the Msg instance to get this, let me know if that isn't correct:

[207 235 40 0 0 1 0 0 0 2 0 1 7 101 120 97 109 112 108 101 3 99 111 109 0 0 6 0 1 1 104 7 101 120 97 109 112 108 101 3 99 111 109 0 0 1 0 255 0 0 0 0 0 0 1 104 7 101 120 97 109 112 108 101 3 99 111 109 0 0 1 0 1 0 0 0 69 0 4 10 1 0 1 3 107 101 121 0 0 250 0 255 0 0 0 0 0 58 8 104 109 97 99 45 109 100 53 7 115 105 103 45 97 108 103 3 114 101 103 3 105 110 116 0 0 0 84 98 70 67 1 44 0 16 183 80 231 212 167 18 46 120 148 157 214 173 19 169 12 21 207 235 0 0 0 0]

When parsing this request my system time was 1415726632, the msg time was 1415726659 and the value of the ti variable that gets compared to the request fudge was 18446744073709551589.

now: 1415726632
msg: 1415726659
ti: 18446744073709551589
fudge: 300

Its a little more difficult to test in that you will have to set your system time to before when the packet is sent in order to test it. This obviously would never occur on the same machine as the request will always be parsed at a time later than the msg was generated. However, in practice, it is very possible that clocks will be off by a few seconds.

Here is a snippet demonstrating the issue:

http://play.golang.org/p/-uTmmtXrq_

Its nice that the system time on the demo is always 1257894000 which is less that the time of the msg that I put in the demo. The output should be 157832659 but it is 18446744073551718957.

One way to fix it would be a if statement to determine which time is larger before doing the subtraction but it seems like there should be a better way.

miekg commented 10 years ago

Thanks for your detailed update. Checking which value is larger seems like an easy an sensible fix.

Testing this can be fixed by allowing the "current time" to be given to to TsigVerify() function, this would need some refactoring.