From ff4040006546eae4b070522f32033165f5672d0f Mon Sep 17 00:00:00 2001 From: Amila Senadheera Date: Sat, 9 Dec 2023 18:23:52 +0530 Subject: [PATCH] rewrite: fix multi request concurrency issue in cname rewrite (#6407) * fix concurrent issue with cname rewrite plugin Signed-off-by: amila * add nil check before deref, add AAAA type test case Signed-off-by: amila --------- Signed-off-by: amila --- plugin/rewrite/cname_target.go | 25 ++++++++++++++++--------- plugin/rewrite/cname_target_test.go | 21 +++++++++++++++++++-- 2 files changed, 35 insertions(+), 11 deletions(-) diff --git a/plugin/rewrite/cname_target.go b/plugin/rewrite/cname_target.go index 068f17fdd..d57bae3b2 100644 --- a/plugin/rewrite/cname_target.go +++ b/plugin/rewrite/cname_target.go @@ -25,11 +25,16 @@ type cnameTargetRule struct { paramFromTarget string paramToTarget string nextAction string - state request.Request - ctx context.Context Upstream UpstreamInt // Upstream for looking up external names during the resolution process. } +// cnameTargetRuleWithReqState is cname target rewrite rule state +type cnameTargetRuleWithReqState struct { + rule cnameTargetRule + state request.Request + ctx context.Context +} + func (r *cnameTargetRule) getFromAndToTarget(inputCName string) (from string, to string) { switch r.rewriteType { case ExactMatch: @@ -62,17 +67,17 @@ func (r *cnameTargetRule) getFromAndToTarget(inputCName string) (from string, to return "", "" } -func (r *cnameTargetRule) RewriteResponse(res *dns.Msg, rr dns.RR) { +func (r *cnameTargetRuleWithReqState) RewriteResponse(res *dns.Msg, rr dns.RR) { // logic to rewrite the cname target of dns response switch rr.Header().Rrtype { case dns.TypeCNAME: // rename the target of the cname response if cname, ok := rr.(*dns.CNAME); ok { - fromTarget, toTarget := r.getFromAndToTarget(cname.Target) + fromTarget, toTarget := r.rule.getFromAndToTarget(cname.Target) if cname.Target == fromTarget { // create upstream request with the new target with the same qtype r.state.Req.Question[0].Name = toTarget - upRes, err := r.Upstream.Lookup(r.ctx, r.state, toTarget, r.state.Req.Question[0].Qtype) + upRes, err := r.rule.Upstream.Lookup(r.ctx, r.state, toTarget, r.state.Req.Question[0].Qtype) if err != nil { log.Errorf("Error upstream request %v", err) @@ -133,10 +138,12 @@ func newCNAMERule(nextAction string, args ...string) (Rule, error) { // Rewrite rewrites the current request. func (r *cnameTargetRule) Rewrite(ctx context.Context, state request.Request) (ResponseRules, Result) { - if len(r.rewriteType) > 0 && len(r.paramFromTarget) > 0 && len(r.paramToTarget) > 0 { - r.state = state - r.ctx = ctx - return ResponseRules{r}, RewriteDone + if r != nil && len(r.rewriteType) > 0 && len(r.paramFromTarget) > 0 && len(r.paramToTarget) > 0 { + return ResponseRules{&cnameTargetRuleWithReqState{ + rule: *r, + state: state, + ctx: ctx, + }}, RewriteDone } return nil, RewriteIgnored } diff --git a/plugin/rewrite/cname_target_test.go b/plugin/rewrite/cname_target_test.go index 04ca01af4..9eee2b8de 100644 --- a/plugin/rewrite/cname_target_test.go +++ b/plugin/rewrite/cname_target_test.go @@ -21,8 +21,15 @@ func (u *MockedUpstream) Lookup(ctx context.Context, state request.Request, name m.Authoritative = true switch state.Req.Question[0].Name { case "xyz.example.com.": - m.Answer = []dns.RR{ - test.A("xyz.example.com. 3600 IN A 3.4.5.6"), + switch state.Req.Question[0].Qtype { + case dns.TypeA: + m.Answer = []dns.RR{ + test.A("xyz.example.com. 3600 IN A 3.4.5.6"), + } + case dns.TypeAAAA: + m.Answer = []dns.RR{ + test.AAAA("xyz.example.com. 3600 IN AAAA 3a01:7e00::f03c:91ff:fe79:234c"), + } } return m, nil case "bard.google.com.cdn.cloudflare.net.": @@ -94,6 +101,16 @@ func doTestCNameTargetTests(rules []Rule, t *testing.T) { test.A("xyz.example.com. 3600 IN A 3.4.5.6"), }, }, + {"abc.example.com", dns.TypeAAAA, + []dns.RR{ + test.CNAME("abc.example.com. 5 IN CNAME def.example.com."), + test.AAAA("def.example.com. 5 IN AAAA 2a01:7e00::f03c:91ff:fe79:234c"), + }, + []dns.RR{ + test.CNAME("abc.example.com. 5 IN CNAME xyz.example.com."), + test.AAAA("xyz.example.com. 3600 IN AAAA 3a01:7e00::f03c:91ff:fe79:234c"), + }, + }, {"chat.openai.com", dns.TypeA, []dns.RR{ test.CNAME("chat.openai.com. 20 IN CNAME chat.openai.com.cdn.cloudflare.net."),