Recursor would shuffle CNAME records until after the name they referred to. #4340

Merged
merged 2 commits into from Aug 25, 2016

Projects

None yet

2 participants

@ahupowerdns
Member

The PowerDNS Recursor shuffles answers randomly, so no single A record gets overloaded. This logic also took care not to shuffle a CNAME record until after the name it points to, because we theorized this would upset some resolvers.

Our logic however assumed all the CNAMEs would initially be at the front of the packet. We'd start our shuffling after skipping all the CNAMEs up front. It now turns out that sometimes we end up with a 'CNAME A CNAME A' packet to shuffle.
This would happily shuffle the last three records. With this PR, we put the CNAMEs up front explicitly before commencing the shuffle. Closes #4339.

Still to be investigated: why didn't this bite us before?

ahupowerdns added some commits Aug 21, 2016
@ahupowerdns ahupowerdns The PowerDNS Recursor shuffles answers randomly, so no single A recor…
…d gets overloaded. This logic also took care not to shuffle a CNAME record until after the name it points to, because we theorized this would upset some resolvers.

Our logic however assumed all the CNAMEs would initially be at the front of the packet. We'd start our shuffling after skipping all the CNAMEs up front. It now turns out that sometimes we end up with a 'CNAME A CNAME A' packet to shuffle.
This would happily shuffle the last three records. With this PR, we put the CNAMEs up front explicitly before commencing the shuffle. Closes #4339.

Still to be investigated: why didn't this bite us before?
27b85f2
@ahupowerdns ahupowerdns so what broke the ordering was the RRSIGs interspersed with the answe…
…rs. With this commit, RRSIGs get explicitly put at the back, and also not shuffled

within the actual answer records.
2cccc37
@rgacogne rgacogne added the rec label Aug 24, 2016
@ahupowerdns ahupowerdns merged commit d16349b into PowerDNS:master Aug 25, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment