fix(rfc2136): add GSS-TSIG authentication to AXFR zone transfers#6528
fix(rfc2136): add GSS-TSIG authentication to AXFR zone transfers#6528carterpewpew wants to merge 1 commit into
Conversation
IncomeTransfer() skipped all TSIG authentication when gssTsig was enabled, causing AXFR requests to be sent unauthenticated. Servers requiring GSS-TSIG (e.g. Active Directory DNS) rejected these transfers, so Records() returned empty and the controller re-created all records every interval. Mirror the GSS-TSIG setup from SendMessage() into IncomeTransfer(): negotiate Kerberos credentials via KeyData(), set TsigProvider on the dns.Transfer, and sign the AXFR message. The GSS context is cleaned up after the transfer channel is fully drained. Add a spy stub so the GSS-TSIG test verifies List() defers TSIG setup to IncomeTransfer, and a direct IncomeTransfer test that exercises the production GSS-TSIG code path. Signed-off-by: Jathavedhan M <jathavedhan.m@ibm.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
kubernetes-prow
Bot
added
provider
|
Hi @carterpewpew. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Tip We noticed you've done this a few times! Consider joining the org to skip this step and gain Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
kubernetes-prow
Bot
added
size/L
kubernetes-prow
Bot
added
ok-to-test
Coverage Report for CI Build 28347292912Warning Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes. Coverage increased (+0.4%) to 83.218%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
vflaux
left a comment
vflaux
left a comment
There was a problem hiding this comment.
Could you add tests to fully cover IncomeTransfer()?
Also, could you add a test in List() to cover the case where the first nameserver fails and the next one succeeds?
| t.TsigProvider = handle | ||
| m.SetTsig(keyName, tsig.GSS, clockSkew, time.Now().Unix()) |
There was a problem hiding this comment.
| t.TsigProvider = handle | |
| m.SetTsig(keyName, tsig.GSS, clockSkew, time.Now().Unix()) | |
| t.TsigProvider = handle | |
| m = m.Copy() | |
| m.SetTsig(keyName, tsig.GSS, clockSkew, time.Now().Unix()) |
To avoid modifiying the provided dns.Msg.
| if !r.insecure { | ||
| if r.gssTsig { | ||
| keyName, handle, err := r.KeyData(nameserver) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to negotiate GSS-TSIG: %w", err) | ||
| } | ||
| t.TsigProvider = handle | ||
| m.SetTsig(keyName, tsig.GSS, clockSkew, time.Now().Unix()) | ||
| gssCleanup = func() { | ||
| handle.DeleteContext(keyName) | ||
| handle.Close() | ||
| } | ||
| } else { | ||
| t.TsigSecret = map[string]string{r.tsigKeyName: r.tsigSecret} | ||
| } | ||
| } |
There was a problem hiding this comment.
Could we move this right before t.In(m, nameserver) so we don’t need all the gssCleanup() calls in the error paths?
handle.Close() should already be enough, since it cleans up all keys associated with its context
|
when updates are secure and axfr is insecure at AD DNS, external-dns/rfc2136 fails: there should be an option for this case too, probably #6519 . Thanks |
4 participants
What does it do ?
Adds GSS-TSIG (Kerberos) authentication to AXFR zone transfers in the rfc2136 provider.
IncomeTransfer() previously skipped all TSIG authentication when gssTsig was enabled, causing AXFR requests to be sent unauthenticated. Servers requiring GSS-TSIG (e.g. Active Directory DNS) rejected these transfers, so Records() returned empty and the controller re-created all records every interval. This mirrors the GSS-TSIG setup already present in SendMessage() into IncomeTransfer(): negotiate Kerberos credentials via KeyData(), set TsigProvider on the dns.Transfer, and sign the AXFR message. The GSS context is cleaned up after the transfer channel is fully drained.
Motivation
Users running --rfc2136-gss-tsig with --rfc2136-tsig-axfr against Active Directory DNS see the controller continuously re-applying all records every interval because AXFR zone transfers fail silently (unauthenticated). This makes --rfc2136-tsig-axfr unusable for GSS-TSIG deployments.
Fixes: #6450
More