Fix TSIG time-validation overflow and enforce zero Error field on requests

[zbalkan]

What

This PR makes two related fixes to TSIG (RFC 8945) message verification in DnsDatagram.cs.

First, it replaces the arithmetic used to check a TSIG record's time window. The previous code built DateTime values by calling DateTime.UnixEpoch.AddSeconds(tsig.TimeSigned - tsig.Fudge) and the +tsig.Fudge equivalent, then compared them against DateTime.UtcNow. Because TimeSigned is an unsigned 48-bit value carried in a ulong and Fudge is a ushort, a crafted or corrupted combination of the two can push the computed seconds-since-epoch outside the range DateTime can represent, which makes AddSeconds throw ArgumentOutOfRangeException. This is now a private helper, IsTsigTimeValid, that does the whole comparison in double arithmetic (Math.Abs((utcNow - DateTime.UnixEpoch).TotalSeconds - timeSigned) <= fudge) and can't overflow. The three call sites — request verification, response verification, and the intermediate-message verification loop for multi-message TSIG chains — all now go through this one helper instead of duplicating the old inline logic.

Second, it adds a check in VerifySignedRequest that rejects a request whose TSIG Error field is non-zero. RFC 8945 §4.2 is explicit here: "Error: in responses, an unsigned 16-bit integer containing the extended RCODE covering TSIG processing. In requests, this MUST be zero." A non-zero Error on an inbound request is therefore a malformed message, and the code now returns a signed FORMERR response for it, the same way it already does for BADTIME/BADTRUNC.

Why

The time-validation change closes a remote crash vector: any signed request or response whose TimeSigned/Fudge pair produces an out-of-range DateTime add can throw before the record's authenticity has even been checked, i.e. before the library knows whether the sender holds a valid key. That's a denial-of-service condition triggerable by anyone who can reach the TSIG-verification code path, not just an authenticated peer.

The Error-field change brings request verification in line with what the RFC actually specifies, rather than silently accepting a field value the spec reserves for responses only. It's a small conformance gap, not a security hole on its own, but it's the kind of gap that lets a malformed or buggy client interoperate today and fail unpredictably against a stricter implementation later.

How

IsTsigTimeValid(ulong timeSigned, ushort fudge, DateTime utcNow) is added as a private static method and used to replace three duplicated inline blocks (request verification, response verification, and the chained-message loop for TCP AXFR-style multi-message TSIG). No public API surface changes — the three call sites keep the exact same branching and error responses they had before (BADTIME signed responses, etc.); only the comparison itself changed.

The Error != NoError check is inserted into VerifySignedRequest after the MAC, time, and truncation checks succeed, so it only fires once the request has already been authenticated with a valid key. That ordering matters: it means the rejection is deliberately signed (errorResponse.SignResponse(this, keys)), consistent with how BADTIME and BADTRUNC are already handled in that method, rather than falling back to the unsigned path used for key/signature failures where the key hasn't yet been proven valid.

I deliberately did not add a companion check on OtherData. RFC 8945 §4.2 says "[Other Data] This document assigns no meaning to its contents in requests" — unlike Error, there's no MUST-be-empty requirement for OtherData on the request side, so enforcing one would be a spec violation in the other direction.

Testing

• dotnet build TechnitiumLibrary.Net/TechnitiumLibrary.Net.csproj -c Release — builds clean, 0 warnings/errors.
• Manually traced all three IsTsigTimeValid call sites against the original inequality semantics to confirm no behavioral drift versus the pre-existing logic (same bounds, same inclusive <=/>= comparison, just no overflow path).
• Cross-repo compatibility check against the downstream consumer zbalkan/DnsServer: built this branch's TechnitiumLibrary.Net.dll and rebuilt DnsServerCore.csproj and the full DnsServerApp.csproj against it — both succeed with 0 errors. The sole call site of VerifySignedRequest in DnsServer.cs already branches generically on the boolean result and logs errorResponse.RCODE/TsigError, so the new FormatError path requires no change there. DnsServer's own outbound signed requests (zone transfer, refresh, notify) go through the library's TsigResolveAsync → SignRequest, which always hardcodes Error = NoError, so this server can't trip its own new check.
• No unit test project exists in this repository yet, so this change ships without automated regression coverage, consistent with the rest of the codebase.

Anything Else

Not blocking this PR, but worth a follow-up issue: there's no test project in TechnitiumLibrary at all. TSIG verification is exactly the kind of logic (bit-width edge cases, RFC-mandated field checks) that benefits from a small table-driven test suite — this PR would have been a good candidate to introduce one, but that's a larger, separate undertaking than these two bug fixes.

[ShreyasZare]

Thanks for the PR. Will check it soon in detail.