Skip to content
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

transfer to must include * destination #469

Closed
Isolus opened this issue Jan 4, 2017 · 17 comments
Closed

transfer to must include * destination #469

Isolus opened this issue Jan 4, 2017 · 17 comments

Comments

@Isolus
Copy link
Collaborator

Isolus commented Jan 4, 2017

I want to restrict zone transfers (with the file middleware) to specific secondary servers. At the moment I can only allow all servers (* is present in the list) or none (* is not present). The function TransferAllowed in zone.go seems to be incomplete.

I think rewriting it shouldn't be that hard but the TransferTo slice of Zone includes only strings so I have to parse them again. That seems unnecessary. So wouldn't it be better to have a slice of structs with IP, IPNet (to support CIDR notation) and Port?

@miekg
Copy link
Member

miekg commented Jan 4, 2017

Yes, we need this. The reason this was not done is that it needs a language that allows you to specify various aspects of networks and/or TSIG keys. It also needs to be similar enough to the (non-specified) language of the rewrite middleware (and maybe other middlewares).

@johnbelamaric do you have an idea for a language here?

@johnbelamaric
Copy link
Member

Ok, this may be overkill, but as I see it this ties into the policy work we (Infoblox) are doing. This is just another example of policy. We want to be able to create policies that control who can request what. Those policies need to be external from the code for sure, but also possibly from the Corefile itself, allowing them to change at runtime without restarting CoreDNS.

Currently the policy work is internal, but we will either be open sourcing it or joining https://openpolicyagent.org and bringing it there. Planning to make that decision ASAP.

Some example policy questions could be:

  • Who can access a given server (i.e., ACLs)?
  • Who can transfer what zones?
  • Can a client from Kubernetes namespace X lookup a record in namespace Y (i.e., multi-tenancy)?
  • Similarly, if a client in K8s namespace X does a wildcard query (*.*.svc.cluster.local), what records should be returned? Only X or others too?
  • Can a query with some specific EDNS0 option lookup some specific record?
  • Should a given query be resolved normally or should the resolved IP be changed (i.e., redirection)?
  • Which of several possible IPs should be returned for a given query?

This provides the functionality of RPZ plus way, way more. I currently have an integration here: https://github.com/johnbelamaric/coredns/tree/policy-middleware that integrates with this internal (to-be-open-sourced) prototype policy engine we have. It lets you externalize the policies into YAML files. Right now it is a middleware that allows you to enable checks on a per query basis. In theory it could support all use cases above, though for ACLs we may want something more specialized.

This current implementation is a completely external system accessed via gRPC. But we could also embed some of this to allow putting policy definitions into the Corefile and applying them locally without making external calls.

Now, that said...we could also consider having a simple policy language internally that can be used across different middleware, while still offering something like this for the more complex use cases.

@miekg
Copy link
Member

miekg commented Jan 4, 2017

Thanks for that update @johnbelamaric. OPA looks nice, but the language (from what I saw) is quite verbose (and powerful). Slight hesitation from my side to include such a framework by default (as opposed to a middleware) as the acl language of CoreDNS.

I'll double check what nsd, bind and powerdns use.

@johnbelamaric
Copy link
Member

Yes, it's probably best as a middleware. We may want a simple built-in policy middleware that just does basic stuff; then these other things would be used for complex stuff.

@miekg
Copy link
Member

miekg commented Jan 4, 2017

Yes! (Only now I see the light). The lightweight policy stuff also needs to be a middleware, which bascially means the transfer to * and friends need to be removed from here and live in their own contained middleware. Ohhh. Nice! :-)

@miekg
Copy link
Member

miekg commented Jan 5, 2017

Well, we still need specify where and how (tsig or not) to transfer to and from. But the acl business can be moved elsewhere.

@miekg
Copy link
Member

miekg commented Jan 5, 2017

NSD:

key:
   name: "sec_key"
   algorithm: hmac-md5
   secret: "6KM6qiKfwfEpamEq72HQdA=="

zone:
    name: home.lan
    zonefile: home.lan.forward
  # notify: 10.0.0.222@53 sec_key
  # provide-xfr: 10.0.0.222 sec_key

(Glanced at some bind stuff as well). The minimum need here is the possibility to specify a key and use that on tranfer-to|from. We don't need any CIDR notation because that's ACL stuff that will live in the acl middleware, only bare IPs with (optional) port numbers are needed (I think).

The main thing to think about is, is it OK to repeat the key's definition or do we need/want it to be named.

Corefile:

miek.nl {
    transfer-to 10.10.1.2 hmac-md5:<base64>
    transfer-to 10.10.1.1 hmac-sha1:<base64>
}

Or

miek.nl {
    key x1 hmac-md5 base64
    key x2 hmac-sha1 base64
    transfer-to 10.10.1.2 x1
    transfer-to 10.10.1.1 x2
}
`transfer-from` *is* btw an ACL thing in my opinion.

@miekg
Copy link
Member

miekg commented Feb 10, 2017

See #57, #178, and #179 for stuff that might benefit from this magical DSL as well. I'll try to write down a proposal.

@miekg
Copy link
Member

miekg commented Mar 15, 2017

I like this: https://caddyserver.com/docs/ipfilter

@stp-ip
Copy link
Member

stp-ip commented Aug 31, 2017

Any update on ACL like stuff for zone transfers?

@miekg
Copy link
Member

miekg commented Aug 31, 2017

No, other than the framework @johnbelamaric mentioned, we didn't make any progress on a small, easy to use DSL for these usecases.

@Centzilius
Copy link

Well at least it is no longer required to put * in the list since 9fb266a which is kind of enough for me but might not be enough for others

@stp-ip
Copy link
Member

stp-ip commented Aug 31, 2017

But only for secondary. It's still necessary fir "file" and "auto" middlewares afaik.

@Centzilius
Copy link

It does work for "file" due to the change in middleware/file/zone.go 9fb266a#diff-e15ee1accd50ebebcb6df120c7a07b0b

@stp-ip
Copy link
Member

stp-ip commented Aug 31, 2017

Seems like this could be done for auto too right?

@miekg
Copy link
Member

miekg commented Aug 31, 2017 via email

@stale
Copy link

stale bot commented Oct 30, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot closed this as completed Nov 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants