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

GeoIP backend - Hit on service's default network matching poisons the cache #6584

Closed
blop opened this Issue May 8, 2018 · 13 comments

Comments

Projects
None yet
5 participants
@blop

blop commented May 8, 2018

  • Program: Authoritative
  • Issue type: Bug report

Short description

When using the GeoIP backend, as described on https://doc.powerdns.com/md/authoritative/backend-geoip/, we can configure services with subnet matching rules and a default matching rule.

I can send queries from all the configured subnets and I always get the correct answer.
If we send a query that matches no subnet, it returns the default value configured for the service, which is normal.

But then, after receiving that "default matched" query, it always answer with that value for everyone, for 20 seconds (query-cache-ttl delay I guess), even for clients that should be matched against one of the configured subnet.

Environment

  • Operating system: Docker
  • Software version: PowerDNS Authoritative Server 4.1.1
  • Software source: PowerDNS repository

Configuration

Command line : --config-dir=/powerdns/ --resolver=1.2.3.4:53

/powerdns/pdns.conf :

expand-alias=yes
launch=geoip
geoip-zones-file=/powerdns/zones.yml

/powerdns/zones.yml

domains:
- domain: example.com
  ttl: 30
  records:
    example.com:
      - soa: ns-aaa.example.com dns.example.com 2018050100 7200 3600 1209600 300
      - ns: ns-aaa.example.com
      - ns: ns-bbb.example.com
    ns-aaa.example.com:
      - a: 10.1.0.1
    ns-bbb.example.com:
      - a: 10.2.0.1
    foo-aaa.example.com:
      - alias: aaa.another.domain.com
    foo-bbb.example.com:
      - alias: bbb.another.domain.com
  services:
    foo.example.com:
      default: 'foo-aaa.example.com'
      10.1.0.0/16: 'foo-aaa.example.com'
      10.2.0.0/16: 'foo-bbb.example.com'

Expected behaviour

The service's subnet mapping should always be respected.
The query cache should be involved only after resolving the service based on the subnet.

Actual behaviour

Query Cache is filled only when hitting the default service mapping, and the cached value is then wrongly used for the duration of query-cache-ttl.

Usecase

It's the whole point of the GeoIP backend to vary the answered value based on the client's IP address.
One query from an unknown IP should not poison the cache.

I will try to look into the implementation code if I have some time.
Thanks already.

@pieterlexis

This comment has been minimized.

Member

pieterlexis commented May 8, 2018

What is the source IP of the query? Is this a recursor in each network, or one single recursor shared between these two networks?

@blop

This comment has been minimized.

blop commented May 8, 2018

There is no recursor involved.

I test by sending queries using dig in command line directly to the PowerDNS Authoritative Server (from different routed network).

cmouse added a commit to cmouse/pdns that referenced this issue May 8, 2018

geoipbackend: Make sure initial netmask is not too wide
Otherwise cache will be "poisoned" with too wide value, rendering
the geoip functionality useless.

Fixes PowerDNS#6584, found by @blop

@cmouse cmouse referenced this issue May 8, 2018

Merged

Geoip #6585

3 of 7 tasks complete
@cmouse

This comment has been minimized.

Contributor

cmouse commented May 8, 2018

@blop can you test if this fixes your issue?

@blop

This comment has been minimized.

blop commented May 8, 2018

Thank you @cmouse ! That was quick ;-) I will perform some testing.

@Habbie

This comment has been minimized.

Member

Habbie commented May 8, 2018

@blop do you need packages for testing?

@blop

This comment has been minimized.

blop commented May 8, 2018

Thank you @Habbie

Currently, I use a fork of tcely/dockerhub-powerdns in which I added geoip in the modules list.
I'll try to build the version of @cmouse in that.

@Habbie

This comment has been minimized.

Member

Habbie commented May 8, 2018

ok! let us know if we can help

@blop

This comment has been minimized.

blop commented May 8, 2018

I adapted the Dockerfile to build the geoip branch.

However I get stuck further down the compilation with these issues :

pollmplexer.cc:11:52: error: no 'FDMultiplexer* FDMultiplexer::getMultiplexerSilent()' member function declared in class 'FDMultiplexer'
 FDMultiplexer* FDMultiplexer::getMultiplexerSilent()
                                                    ^
pollmplexer.cc:38:16: error: 'virtual void PollFDMultiplexer::getAvailableFDs(std::vector<int>&, int)' marked 'override', but does not override
   virtual void getAvailableFDs(std::vector<int>& fds, int timeout) override;
                ^~~~~~~~~~~~~~~
pollmplexer.cc:43:10: error: 'std::__cxx11::string PollFDMultiplexer::getName() const' marked 'override', but does not override
   string getName() const override
          ^~~~~~~
pollmplexer.cc: In function 'FDMultiplexer* make()':
pollmplexer.cc:53:32: error: invalid new-expression of abstract class type 'PollFDMultiplexer'
   return new PollFDMultiplexer();
                                ^
pollmplexer.cc:28:7: note:   because the following virtual functions are pure within 'PollFDMultiplexer':
 class PollFDMultiplexer : public FDMultiplexer
       ^~~~~~~~~~~~~~~~~
In file included from pollmplexer.cc:4:0:
mplexer.hh:133:23: note:        virtual std::__cxx11::string FDMultiplexer::getName()
   virtual std::string getName() = 0;
                       ^~~~~~~

If you have a working docker image or amd64 binaries/libs (with geoip) that I can use it'd be awesome.
Otherwise I'll continue to work on this build.

;-)

@blop

This comment has been minimized.

blop commented May 10, 2018

@cmouse Is your branch compiling properly ?
@Habbie If you have that package I'd be happy to test :-)

@Habbie

This comment has been minimized.

Member

Habbie commented May 10, 2018

https://builder.powerdns.com/#/builders/8/builds/2288 - in 30-40 minutes you should be able to get packages for various distributions here.

@cmouse

This comment has been minimized.

Contributor

cmouse commented May 10, 2018

@blop, yes and CI agrees. Is there a reason you ask?

@blop

This comment has been minimized.

blop commented May 10, 2018

@cmouse Because of these errors I mentionned above. But it's probably my fault! ;-)
I will test using the version provided by @Habbie

@blop

This comment has been minimized.

blop commented May 11, 2018

\o/ I can confirm the patch made by @cmouse is fixing this issue.

@rgacogne rgacogne modified the milestones: auth-4.2.0, auth-4.1.x May 11, 2018

pieterlexis added a commit to pieterlexis/pdns that referenced this issue May 15, 2018

geoipbackend: Make sure initial netmask is not too wide
Otherwise cache will be "poisoned" with too wide value, rendering
the geoip functionality useless.

Fixes PowerDNS#6584, found by @blop

(cherry picked from commit 86f6a8e)

cmouse added a commit to cmouse/pdns that referenced this issue May 15, 2018

geoipbackend: Make sure initial netmask is not too wide
Otherwise cache will be "poisoned" with too wide value, rendering
the geoip functionality useless.

Fixes PowerDNS#6584, found by @blop

cmouse added a commit to cmouse/pdns that referenced this issue May 15, 2018

geoipbackend: Make sure initial netmask is not too wide
Otherwise cache will be "poisoned" with too wide value, rendering
the geoip functionality useless.

Fixes PowerDNS#6584, found by @blop
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment