inet_dns support for UPDATE, NOTIFY and TSIG

[jimdigriz]

Some proposed enhancements to inet_dns:

• guard to catch when there is more than one EDNS(0) option present as discussed
• support for NOTIFY and UPDATE
  • UPDATE is a little more complicated as zero length data is now allowed and a new NONE class
  • removed the UPDATE{A,D,DA,M,MA} to kick up a conversation if these should still exist
  • includes the defines for IXFR and a few other rcodes
• Initial support for TSIG suggested some time back that there would be interest to have this baked into inet_dns.erl
  • Not yet added support for TSIG over TCP as described by RFC8945, section 5.3.1; this is on my list of things to resolve though - this has now been added though mostly untested

I suspect none of this is particularly controversial but the TSIG API does need to be thrashed out.

I am submitting what I see as version zero (0) and finally worth sharing around. I really would prefer the focus be on the API as the implementation will need burning and redoing a few more times from scratch as we carve out the interface.

In the spirit of inet_dns, this is an undocumented API in that it expects the user to handle policy whilst it only sets out to handles the encoding/decoding/verification.

I tried a few interface/API iterations and found having settled on passing in #dns_rec{} ended up being the least offensive and made the most sense from a possible user perspective but this is my view only. Similarly I tried putting this all in inet_dns.erl but after a while the sign/verify functionality just felt more correct in its own module, again this is my view, but it did mean I had to export the name encode/decode routines from inet_dns so maybe this should be folded back into inet_dns.

Broken out of the commit so to hack out the conversation here, this is what it smells like (for a simple client expecting a single UDP response packet to its request):

Zone = "example.com".
Name = "cheese".
Request = inet_dns:make_msg([
        {header,inet_dns:make_header([
                {id,rand:uniform(65536) - 1},
                {opcode,update}
        ])},
        % RFC2136, section 2
        {qdlist,[       % Zone
                inet_dns:make_dns_query([
                        {domain,Zone},
                        {class,in},
                        {type,soa}
                ])
        ]},
        {nslist,[       % Update
                inet_dns:make_rr([ 
                        {domain,Name ++ "." ++ Zone},
                        {ttl,300},
                        {class,in},
                        {type,a},
                        {data,{192,0,2,1}}
                ])
        ]}
]).

% my thinking here is to be like crypto:mac_init as we need rolling state especially for TSIG over TCP
TSigState0 = inet_dns_tsig:init([{keys,[{"mykey","moocowmoocow"}]}]).

{ok,RequestSigned,TSigState1} = inet_dns_tsig:sign(Request, TSigState0).
RequestPkt = inet_dns:encode(RequestSigned).

{ok,Sock} = gen_udp:open(0, [binary,{active,false}]).
gen_udp:send(Sock, {{127,0,0,1},5354}, RequestPkt).
{ok,{_,_,ResponsePkt}} = gen_udp:recv(Sock, 0).

{ok,ResponseSigned} = inet_dns:decode(ResponsePkt).

% will return {error,...} if something is wrong
% like inet_dns:decode, will throw formerr if something is wrong with the packet...personally not a fan of this behaviour
{ok,TSigState2} = inet_dns_tsig:verify(ResponseSigned, TSigState1).

Response = inet_dns:msg(ResponseSigned).

Usage for a client (shown above and mostly tested):

1. tsig:init/1
   • configuration algs really is only useful as a single item as we use the first algorithm (default: sha256) provided
   • configuration keys really is only useful as a single item as we use the first name/secret entry
2. tsig:sign/2 the request and send it
3. for each response packet (there can be one or typically for TCP more than one) call tsig:verify/2
4. ...remember to consider the gotchas/warnings in the comments at the top of inet_dns_tsig.erl

Usage for a server (not tested as I am still in the middle of building my DNS server...will get back to you...):

1. tsig:init/1
   • configuration algs lists all the algorithms we are willing to support (default: only sha256, maybe the default for 'server' should be changed to all the algorithms)
   • configuration keys lists all the name/secret combinations that are accepted
2. call tsig:verify/2 on the request packet
3. construct the response and call tsig:sign/2, sending it to the client; repeat this until the response is completely sent

[github-actions]

CT Test Results

       2 files       65 suites   1h 1m 7s ⏱️
1 495 tests 1 241 ✔️ 254 💤 0 ❌
1 686 runs  1 384 ✔️ 302 💤 0 ❌

Results for commit 2473a41.

♻️ This comment has been updated with latest results.

[jimdigriz]

Okay, the name compression tripped me up[1] so the API now requires the user to pass in the original binary packet.

TSIG over TCP now works.

So I have tested this against Knot DNS, but still lacking testing with OTP as the 'server'...I'll let you know once I have tested it, may be some time though.

Would be great to have feedback on the API/interface as if I need to rework it to make it less offensive, it would really help to know sooner rather than later.

An example of a TSIG signed DNS update is as shown:

Zone = "example.com".
Name = "cheese".
RequestPkt = inet_dns:encode(inet_dns:make_msg([
        {header,inet_dns:make_header([
                {id,rand:uniform(65536) - 1}, 
                {opcode,update}
        ])},
        % RFC2136, section 2
        {qdlist,[       % Zone
                inet_dns:make_dns_query([
                        {domain,Zone},
                        {class,in},
                        {type,soa}
                ])  
        ]}, 
        {nslist,[       % Update
                inet_dns:make_rr([
                        {domain,Name ++ "." ++ Zone},
                        {ttl,300},
                        {class,in},
                        {type,a},
                        {data,{192,0,2,1}}
                ])  
        ]}  
])).
TSigState0 = inet_dns_tsig:init([{keys,[{"mykey","moocowmoocow"}]}]).
{ok,RequestPktSigned,TSigState1} = inet_dns_tsig:sign(RequestPkt, TSigState0).
{ok,Sock} = gen_udp:open(0, [binary,{active,false}]).
ok = gen_udp:send(Sock, {{127,0,0,1},5354}, RequestPktSigned).
{ok,{_,_,ResponsePktSigned}} = gen_udp:recv(Sock, 0). 
ok = gen_udp:close(Sock),
{ok,TSigState2} = inet_dns_tsig:verify(ResponsePktSigned, TSigState1).
{ok,ResponseSigned} = inet_dns:decode(ResponsePktSigned).
Response = inet_dns:msg(ResponseSigned).

An example of a TSIG signed DNS AXFR query is as shown:

ZoneAXFR = "com".
RequestAXFRPkt = inet_dns:encode(inet_dns:make_msg([
        {header,inet_dns:make_header([
                {id,rand:uniform(65536) - 1}, 
                {opcode,query}
        ])},
        {qdlist,[
                inet_dns:make_dns_query([
                        {domain,ZoneAXFR},
                        {class,in},
                        {type,axfr}
                ])
        ]}  
])).
TSigStateAXFR0 = inet_dns_tsig:init([{keys,[{"mykey","moocowmoocow"}]}]).
{ok,RequestAXFRPktSigned,TSigStateAXFR1} = inet_dns_tsig:sign(RequestAXFRPkt, TSigStateAXFR0).
{ok,SockAXFR} = gen_tcp:connect({127,0,0,1}, 5354, [binary,{active,false},{nodelay,true},{packet,2}]).
ok = gen_tcp:send(SockAXFR, RequestAXFRPktSigned).
Recv = fun(Recv, A) ->
        case gen_tcp:recv(SockAXFR, 0) of
                {ok,Pkt} ->
                        Recv(Recv, [Pkt|A]);
                {error,closed} ->
                        lists:reverse(A)
        end 
end.
ResponseAXFRPktsSigned = Recv(Recv, []),
ok = gen_tcp:close(SockAXFR).
TSigStateAXFR2 = lists:foldl(fun(P, TS0) ->
        {ok,TS} = inet_dns_tsig:verify(P, TS0), TS
end, TSigStateAXFR1, ResponseAXFRPktsSigned).
[ case inet_dns:decode(P) of {ok,M} -> inet_dns:msg(M) end || P <- ResponseAXFRPktsSigned ].

[1] we need to do an HMAC on the original DNS packet and every DNS name compression algorithm is different so by decoding and re-encoding the packet we end up with a different output

[jimdigriz]

Would be helpful to receive a summary of either "this is a pile of garbage, please haul it away" or "we like it in principle, we are currently too busy to chew through the sharp edges".

Just depressing for a contributor to watch other PRs being worked through whilst receiving radio silence and being left in limbo.

Really it is absolutely zero problem if this is considered a hot steaming pile of garbage, more than happy to just move it into my own private library.

For my purposes it does not need to be in OTP, I only submitted as it was mentioned there was some interest in getting something like this into OTP.

[RaimoNiskanen]

I will try to find the time to look at this now (tomorrow (...)).
Sorry about the silence!
It is a lot to read up on, to be able to have an opinion...

[jimdigriz]

I think the biggest bit here is that the decision needs to be made to either:

1. have one (the existing) complete DNS decoder and a second one (for TSIG) that breaks each RR section into a list of binaries
2. rework the existing DNS decoder to be a two pass affair, the first one breaks the everything up into a list of binaries where then TSIG can do its thing, whilst the main decoder then immediately goes into the binary to record parser

Most of my PR is just a reimplementation of the DNS decoder; I did it this way so you could see what is needed rather than just jumping in an butchering the existing decoder.

Of course happy to do this work, but need the old thumbs up or thumbs down on if this would ever be accepted.

[RaimoNiskanen]

I am happy you have made the effort to understand the RFC:s, and to write code that works. From here we can look at details and structure.

I have read RFC 8945 (could have been more thorough) and now try to understand how your code implements it...

[jimdigriz]

Let me know if I can help in any way, even if it is "hey, we think we want the DNS decoder to work like this, can you refactor things for us and we'll review"

Thanks!

[RaimoNiskanen]

I'll write down things in the order I stumble on them...

I belong to the camp that limits lines to 80 characters, but we have people from the other camp in my team... If you have a strong opinion about not limiting code to 80 characters I will accept that, but I would really like the line limit to be 80 characters...

I do not see the point of converting to Gregorian time for the user interface format. Why not simply have Posix time there. Furthermore I think using os:system_time(seconds) would be less of a surprise to system administrators. erlang:system_time/* might be quite a bit off from the OS Posix time after changing the system clock. I believe os:system_time/* is the best value we have for the Posix time that the sysadmin has set.

I have a hard time understanding the code in inet_dns_tsig:do_verify/2 regarding #tsig_state.qr as in when it is the request, the first response, an unsigned intermediate response, the peculiar mac update for QR < 3 - when and why does that happen. The interval 1 < QR, QR < 100 should be 98 messages, not 99, etc. More comments would be nice, and perhaps it is possible to have a flatter or in some other way clearer decision tree?

There was something in the RFC about at least 10 bytes for a truncated MAC, but I cannot find code for that.

Should #tsig_state.key be {string(), iodata()} | #{string() := iodata()}, and .alg be tsig_alg() | #{tsig_alg() := []} to facilitate lookup for the server. The client can take aConfig that specifies one key and alg, but the server knows all supported algs and gets all keys it should know of. .alg can have a default of all supported.

I think the abstraction border between inet_dns and inet_dns_tsig is basically right, but it would be nice if we could get the separation more like what is indicated in the RFC by adding specialized helper functions in inet_dns:

• For encoding: Introduce inet_dns:encode_comp(Q) -> {Binary, Comp}, sign that Binary with a function in inet_dns_tsig to get a TsigRR, and use a new inet_dns:encode_tsig(Binary, Comp, TsigRR) -> NewBinary. With this it would be possible to append a TSIG record to Binary, and patch ARCOUNT without unpacking Binary, right?
• Would it be possible to decode first, extract the TSIG record, then verify? Use the normal inet_dns:decode(Buffer), work on the RR level to find the TSIG record, then implement inet_dns:decode_tsig(Buffer, TsigRR) -> NewBuffer where NewBuffer is the original not signed binary message that can be verified with a function in inet_dns_tsig.

Is this a feasible API, or is there something in the RFC:s that I have missed? Is this any better?

I think the duplicated functions in inet_dns_tsig such as encode_name/1 and decode_name/1 miss some tests, for example for maximum length, for compression pointer, etc, so as you say; it would be good to use the ones in inet_dns to have the encode/decode details in one module.

[jimdigriz]

I belong to the camp that limits lines to 80 characters, but we have people from the other camp in my team... If you have a strong opinion about not limiting code to 80 characters I will accept that, but I would really like the line limit to be 80 characters...

Sure, I don't mind either way, but if you asked me to s/\t/ /g we might have a fight on our hands!

I do not see the point of converting to Gregorian time for the user interface format. Why not simply have Posix time there.

Was only personal taste, mostly as my brain does not come with the epoch converter upgrade. I am not overly bothered either way.

I have a hard time understanding the code in inet_dns_tsig:do_verify/2 regarding #tsig_state.qr as in when it is the request, the first response, an unsigned intermediate response, the peculiar mac update for QR < 3 - when and why does that happen. The interval 1 < QR, QR < 100 should be 98 messages, not 99, etc.

Right, so part of the problem (I like to see it as a feature) is that I have overloaded qr to mean two things.

• 0 (false): request
• 1 (true): response
• >1: response but multi-packet following the original response
  • the fun part is this effectively wraps at 100 to coincide with legacy support for when TSIG only need to be applied at least once every 99 packets (as well as the first/final)
  • when we wrap, we wrap to 2 (hence the test for 99), not 1 (which would make the test 98)

To preserve the sanity of others, should I remove the overloading of qr and have counter to run alongside it? Alternatively better documentation?

More comments would be nice, and perhaps it is possible to have a flatter or in some other way clearer decision tree?

Agreed.

There was something in the RFC about at least 10 bytes for a truncated MAC, but I cannot find code for that.

It is somewhat subtly implemented, the lower limit is referred to by MAC_SIZE_MIN (RFC8945, section 5.2.2.1), gets used as a lower limit for MACSizeMin, latched in the state and used in the later verification step.

Should #tsig_state.key be {string(), iodata()} | #{string() := iodata()}, and .alg be tsig_alg() | #{tsig_alg() := []} to facilitate lookup for the server. The client can take aConfig that specifies one key and alg, but the server knows all supported algs and gets all keys it should know of. .alg can have a default of all supported.

It is guess work at the moment on what I think is appropriate, but completely untested. I have be poking my DNS product codebase on and off around my other work...I only have had a chance to work on the client side as I am still working through implementing DNS-SD registration/lookups hence why the client side TSIG work is tested so far.

Personally I would have the server side be non-configurable on alg as the only reason you would care here (and on the MAC length) is if you want to implement a policy on the server on what is considered acceptable.

I do not see any difference in effect punting this check post-verification compared to being forced to verify MACs from hostile actors but where they are using 'correct' values of alg.

I think the abstraction border between inet_dns and inet_dns_tsig is basically right, but it would be nice if we could get the separation more like what is indicated in the RFC by adding specialized helper functions in inet_dns:

• For encoding: Introduce inet_dns:encode_comp(Q) -> {Binary, Comp}, sign that Binary with a function in inet_dns_tsig to get a TsigRR, and use a new inet_dns:encode_tsig(Binary, Comp, TsigRR) -> NewBinary. With this it would be possible to append a TSIG record to Binary, and patch ARCOUNT without unpacking Binary, right?

I quite like this. My reason for doing things the way they are is that I assumed there may be some push back about exporting anything more from inet_dns...as it is completely undocumented, but maybe that undocumented'ness is a good reason why this is not a problem?

• Would it be possible to decode first, extract the TSIG record, then verify? Use the normal inet_dns:decode(Buffer), work on the RR level to find the TSIG record, then implement inet_dns:decode_tsig(Buffer, TsigRR) -> NewBuffer where NewBuffer is the original not signed binary message that can be verified with a function in inet_dns_tsig.

Decoding is a complete pain as you need to remove the TSIG RR (decrementing ARCOUNT is straight forward) from the original binary and make that available; which is why I re-implemented an DNS decoder.

Annoyingly we know it is the last RR in the entire packet (anything else is invalid), but I cannot see a way to 100% reliably scan for the start of the RR from the end of the packet as name compression is allowed here (it is only forbidden on the algorithm name) and it is pretty much impossible to determine a length header in there.

Only way I see it is we have to decode the whole thing.

One idea I have is we could update #dns_rr{} to include the byte offset of where the RR starts in the binary has helpfully the usual decoder already tracks this for the purposes of name compression. This information can then supply the needed information (in a single pass) on where to truncate the original packet. If updating #dns_rr{} is unappealing for some reason then we could just somehow carry this offset through all the decoding functions (ewwwww...) and communicate this back to the caller (double ewwww...).

Personally I prefer the idea of being able to call something like inet_dns:dns_rr(RR, offset), but I am not the maintainers here, you get to make this call :)

[RaimoNiskanen]

Sure, I don't mind either way, but if you asked me to s/\t/ /g we might have a fight on our hands!

Now that you mention it ;-) ... no, but we prefer to use space only indent for new modules. However, if this is already written with tabs I will not insist on changing that. But I would be happy if you can rewrite to keep the line length under 80 (76) characters.

And use Posix seconds in the user data, also use os:system_time(seconds) for that - you had no comment on os:system_time vs. erlang:system_time?

To preserve the sanity of others, should I remove the overloading of .qr and have counter to run alongside it? Alternatively better documentation?

I think better comments and clearer code will be good enough. I still think that 2..99 would be 98 intermediate non-signed messages, not 99. For integer ranges I find it often clearer to consistently use =< which demonstrates the interval end.

Would using request | response | 1..99 for #tsig_state.qr cause too clumsy code, or could it be a clarification?

I missed ?MAC_SIZE_MIN. Now it is clear.

... but maybe that undocumented'ness is a good reason why this is not a problem?

We can somewhat treat it as an internal module and modify it as we like, but we know we have external users, so we want to only do backwards compatible changes, if possible. Adding specialized functions for TSIG would be fine.

Only way I see it is we have to decode the whole thing.

Then it would be preferable to not duplicate code getting a parallel decoder implementation.

One idea I have is we could update #dns_rr{} to include the byte offset of where the RR starts in the binary ...

I think it would be enough to add a byte offset field to the TSIG RR #dns_rr.data term.
Or maybe like this...:

• Change inet_dns:decode_rr_section/5 to have special treatment for ?S_TSIG so it verifies that Rest is <<>> and returns {{TsigRR, Offset}, reverse(RRs), boolean()}.
• Change inet_dns:do_decode/1 to return {#dns_rec, {TsigRR,Offset} | undefined}.
• Change inet_dns:decode/1 to append the TsigRR to #dns_rec.arlist unless undefined.
• Add a function inet_dns:decode_tsig/1 that returns {ok, {#dns_rec{}, TsigRR, BinaryPatchedToOriginal}.

Offset can be part of TsigRR, or as suggested above, be in a wrapping tuple.

[jimdigriz]

Sure, I don't mind either way, but if you asked me to s/\t/ /g we might have a fight on our hands!

Now that you mention it ;-) ... no, but we prefer to use space only indent for new modules. However, if this is already written with tabs I will not insist on changing that. But I would be happy if you can rewrite to keep the line length under 80 (76) characters.

Only a suggestion, it is useful at times when committing code to purposely merge incorrectly whitespaced segments.

The theory is that at some later point that someone with OCD will come along later, fix it up...and then git blame not longer points to you :)

...also use os:system_time(seconds) for that - you had no comment on os:system_time vs. erlang:system_time?

Did not see any need to discuss it, took this as a signal on what the OTP team want to see in a v1 rework.

One idea I have is we could update #dns_rr{} to include the byte offset of where the RR starts in the binary ...

I think it would be enough to add a byte offset field to the TSIG RR #dns_rr.data term. Or maybe like this...:

• Change inet_dns:decode_rr_section/5 to have special treatment for ?S_TSIG so it verifies that Rest is <<>> and returns {{TsigRR, Offset}, reverse(RRs), boolean()}.

I suspect it will never occur, mostly due to the contract around TSIG, but treating things 'special' feels like something that will burn someone later on; also to date 100% of OTP only Erlang processed DNS packets are TSIG less :)

Offset can be part of TsigRR, or as suggested above, be in a wrapping tuple.

I think I would prefer to put it in the #dns_rr{} record.

It is easier to reason about (no RR is special, I hate them all equally!) plus it will help others working on decoding problems where they can now tie back each RRs to where they appeared in the originating binary; my thinking here is when you are inspecting your PCAPs in Wireshark.

I think I have everything I need from a first review to rework the TSIG component.

Did you want to cherry pick out the EDNS(0) guard commit from this PR? Also what do you want me to do (if anything) with the NOTIFY/UPDATE commit...cherry pick or does it need some massaging in places?

[RaimoNiskanen]

I suspect it will never occur, mostly due to the contract around TSIG, but treating things 'special' feels like something that will burn someone later on; also to date 100% of OTP only Erlang processed DNS packets are TSIG less :)

Well, since inet_dns parses DNS messages, and we are adding TSIG parsing, then I do not see why that parsing should not handle TSIG like the special record it is, just like the OPT record has special treatment. (We could even add a #dns_rr_tsig{} record, but no real point in that, I guess) A TSIG record has to be last otherwise it is a parse error, and part of the parsing result is how large the original message was. This behaviour is separate from signature verification.

I think I would prefer to put it in the #dns_rr{} record.

I am afraid we much prefer to not change the #dns_rr{} record. Some customers use the record definitions form the .hrl file, instead of the existing (also undocumented) functional abstraction, so adding fields would force them to recompile, which is not popular (and these are internal corporate customers that pay our salaries). Such a change would have to be for a major release, and have warnings attached to it (even though this module is "unsupported").

I understand your reasoning about decoding, and debugging the wire format, but unfortunately ... the above.

Did you want to cherry pick out the EDNS(0) guard commit from this PR? Also what do you want me to do (if anything) with the NOTIFY/UPDATE commit...cherry pick or does it need some massaging in places?

If I should take the first two commits, then you would have to wait until I have merged to master for you to rebase on that, to avoid merge conflicts, furthermore, it seems UPDATE/NOTIFY are the major use cases you have for TSIG. So I do not mind keeping these commits in this PR.

But, there seem to be two diff hunks in the 3:rd commit (inet_dns: support for TSIG requests), moving A, AAAA and WSIG records from class IN RRs to any class RRs, that maybe should be in the 2:nd commit (inet_dns: support for UPDATE and NOTIFY DNS packets). If so those diff hunks should be moved.

[jimdigriz]

I think I would prefer to put it in the #dns_rr{} record.

I am afraid we much prefer to not change the #dns_rr{} record. Some customers use the record definitions form the .hrl file, instead of the existing (also undocumented) functional abstraction, so adding fields would force them to recompile, which is not popular (and these are internal corporate customers that pay our salaries).

Such a change would have to be for a major release, and have warnings attached to it (even though this module is "unsupported").

I understand your reasoning about decoding, and debugging the wire format, but unfortunately ... the above.

I am 100% okay with waiting till OTP 28 for this; not the first time I have waited on a PR to be merged.

For now I am happy to slum the solution at my end for my use case with double parsing and passing a binary blob into the DNS encoder as I used to for CAA RRs.

If though the preference is to get this done sooner rather than later, then I'll start the work to drag that byte offset kicking'n'screaming through the decoder. I assume this is what you want unless I hear otherwise.

Did you want to cherry pick out the EDNS(0) guard commit from this PR? Also what do you want me to do (if anything) with the NOTIFY/UPDATE commit...cherry pick or does it need some massaging in places?

If I should take the first two commits, then you would have to wait until I have merged to master for you to rebase on that, to avoid merge conflicts, furthermore, it seems UPDATE/NOTIFY are the major use cases you have for TSIG. So I do not mind keeping these commits in this PR.

I suspect if the TSIG stuff was ready today for merging, it would not make it into OTP 26.x.

Rebasing and tracking master is really little to no effort, and the TSIG work is not tangibly impacted by it.

Only asking as whatever is easier for you? If you prefer to have the PR retain all three features, that works for me...the cost at my side is no different carrying one or three features out of tree.

But, there seem to be two diff hunks in the 3:rd commit (inet_dns: support for TSIG requests), moving A, AAAA and WSIG records from class IN RRs to any class RRs, that maybe should be in the 2:nd commit (inet_dns: support for UPDATE and NOTIFY DNS packets). If so those diff hunks should be moved.

Good catch, looks like I crossed the streams, I'll get that fixed.

[RaimoNiskanen]

If though the preference is to get this done sooner rather than later, ...

It would be nice to get this finished while I have not gotten too deep into to much other stuff and forgotten about the details. Other than that there is no hurry.

... then I'll start the work to drag that byte offset kicking'n'screaming through the decoder. I assume this is what you want unless I hear otherwise.

To just get the byte offset out there would not be much difference to have it as a new field in the #dns_rr{} record compared to in the #dns_rr.data field for a TSIG record. But I think the DNS decoder should be aware of the TSIG record semantics and verify that it is last in the AR section.

Adding an API function that also removes the TSIG record and returns it separated from the DNS message would be an optimization to avoid completing a DNS message you do not want and then having to parse the DNS whole message again to do exactly that.

I suspect if the TSIG stuff was ready today for merging, it would not make it into OTP 26.x.

Maybe in theory, but, ... no.

If you prefer to have the PR retain all three features, that works for me...

I think, then, I would prefer to keep all 3 in the same branch, with the 2 first as separate commits, as it is now.

[jimdigriz]

I think, then, I would prefer to keep all 3 in the same branch, with the 2 first as separate commits, as it is now.

...I'll get to work then.

Might take a week or two for me to find the time, so "watch this space".

Thanks for the review and the pointers!

[RaimoNiskanen]

I removed the 'testing' label, but the branch is still part of our daily builds. I pull it in and add it internally...

[jimdigriz]

I managed to clear the backlog of my other work today, hence the rebasing that slipped out last night, and hope to have the squashing sorted by today. sorry it took a while to get back to this.

[RaimoNiskanen]

No worries :-)

[jimdigriz]

Okay, here is the first cut, I'm now running through the unit tests at each commit to make sure it all is okay.

Original unsquashed branch at https://github.com/jimdigriz/otp/tree/dns.unsquashed and there is no diff between the two.

[jimdigriz]

Punted the nsd->knotd commit up the chain so we get better testing...typing apt install nsd is too much effort...

Ran the following at each commit and they pass:

make kernel_test MAKEFLAGS="$MAKEFLAGS" ARGS="-suite inet_res_SUITE"

Let me know if you need anything else, but I think at the moment there is nothing else I need to do?

[RaimoNiskanen]

For the sake of the procedures I will let it run a few nights in its squashed variant before merging. I also think it is ready to merge.

[RaimoNiskanen]

Thank you for your pull request and hard work!

[jimdigriz]

Thanks for the guidance on beating this PR into shape.

See you when I need to implement SIG(0) :)

[RaimoNiskanen]

I will read up on that when it falls on my table... ;-)