rewrite: fix multi request concurrency issue in cname rewrite (#6407)

* fix concurrent issue with cname rewrite plugin

Signed-off-by: amila <amila.15@cse.mrt.ac.lk>

* add nil check before deref, add AAAA type test case

Signed-off-by: amila <amila.15@cse.mrt.ac.lk>

---------

Signed-off-by: amila <amila.15@cse.mrt.ac.lk>
This commit is contained in:
Amila Senadheera 2023-12-09 18:23:52 +05:30 committed by GitHub
parent 92ec849acb
commit ff40400065
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 35 additions and 11 deletions

View file

@ -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
}

View file

@ -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."),