-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support TsigProvider for Server and Transfer #1331
Conversation
/cc @Fattouche |
This is largely copied from github.com/miekg/pull/1323 by @Fattouche.
@@ -74,6 +74,24 @@ func (key tsigHMACProvider) Verify(msg []byte, t *TSIG) error { | |||
return nil | |||
} | |||
|
|||
type tsigSecretProvider map[string]string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is good for a default TsigProvider, however it still doesn't solve the problem described in
We need some way for Generate to be able to unwrap the Message so that we can see the query name. This is specifically useful in situations where multiple tsigs have the same name, but exist under a different zone.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That can be handled either in a separate PR or externally, but yes we do need to do something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How common a use case is that do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would say it's fairly common. I ran into this problem early on with this library when trying to serve multiple zones that had different primaries, therefore different TSIGs on each primary meaning they could not use the TSIG name as the zone Name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @tmthrgd have you thought any more about how we can best obtain the query name from the Generate function. I think having the opposite of TsigBuffer available in a function would make this possible. Alternatively we could consider passing in a context? It would be useful to have contexts everywhere anyway so that we could pass data around without needing to alter signatures.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
/merge |
Automatically submitted.
This adds support for
TsigProvider
to bothServer
andTransfer
. It also simplifies the implementation forClient
andConn
.Not sure why I didn't think to do this earlier. A miekg/dns/v2 could just push the
TsigSecret
code out into the callers.This also has the advantage that it doesn't add a bunch of extra code paths throughout, so it's safer to land.
Fixes #1300
Closes #1323
Fixes #1325