Add classes encapsulating rules for DHCPv6 client traffic (in/out)

[nbarrientos]

No description provided.

[nbarrientos]

Maybe it makes sense to provide as well nftables::dhcpv6 or similar to make it easier to activate the rules. It's something generic and wide-spread enough to justify it I'd say. Your call.

[duritong]

Maybe it makes sense to provide as well nftables::dhcpv6 or similar to make it easier to activate the rules. It's something generic and wide-spread enough to justify it I'd say. Your call.

So is this something that you want rather to use together? I mean if you want to request dhcpv6 the out are enough, not?

[nbarrientos]

Thanks for the review :)

So is this something that you want rather to use together?

Not sure I understand your point but I think it's perhaps I didn't express correctly what I was proposing to do. I was asking if as part of this patch you'd like me to add a parameter to the class nftables called in_dhcpv6 (or maybe in_out_dhcpv6) to be consumed in inet_filter.pp to include nftables::rules::dhcpv6 and nftables::rules::out::dhcpv6 (similar to what you're doing for ssh, for example). In our particular case we're not going to use it because the caller of the nftables module configures an all out accept policy and then we're including nftables::rules::dhcpv6 directly to accept the traffic coming from the DHCP server. We're happy to get the patch accepted as it is now :)

[nbarrientos]

I mean if you want to request dhcpv6 the out are enough, not?

Ah, got you now. Yes, it's the client initiating in all cases so the packets back from the server will be accepted as things stand now because they're part of an established "connection" (quoted because it's UDP). It's corrected, the patch only contains a rule for the output chain.

So yes, the only pending issue is to know if you'd like to have a parameter out_dhcpv6 in the nftables class.

[nbarrientos]

Uhm, not convinced. The initial packet from the client is sent to the multicast addr ff02::1:2 but the response has the link scope addr of the server as source so connection tracking might not apply and a explicit INPUT rule for NEW traffic like the one initially proposed might be in order.

[nbarrientos]

I've reintroduced the input rule class and a parameter dhcpv6 in nftables (to be discussed)

[duritong]

Our idea with the parameters for the nftables class is, that you should be able to get a as locked down as possible host by including the nftables class. Now, what you need always from our view, is putgoing dns resolution, yum repos (thus https) and ntp, as well as incoming ssh. This is what we defined as our standard baseline.
We understand that there are situations, where you want to have any outgoing connection allowed, which is why there is a global option.
Now with regard to the dhcpv6 option, one could argue that in certain environments it is necessary, on the other hand in most server environments, you likely have some kind of static assignment. Thus the dhcpv6 rule would definitely be too much to enable (I assume) by default. And also we would not want to have for every rule a parameter in the main class. So if you do not have strong arguments to have dhcpv6 somehow enabled when just including the nftables class (and setting a hiera value), I don't think it is really necessary to do so.

[nbarrientos]

Makes total sense. I've removed the commit adding the parameter. Next time you're in Genf beers are on me :)

[nbarrientos]

Happy to do s/dhcpv6/dhcpv6c to make clear that the rules are for the DHCPv6 client.

[duritong]

Happy to do s/dhcpv6/dhcpv6c to make clear that the rules are for the DHCPv6 client.

Question: I understand you (and dhcpv6) correctly, that you will always need both (in&out) so that a host is able to do dhcpv6, right? So I would propose to merge them together to one class (e.g. nftables::rules::dhcvp6_client) so one need only to include one class to enable dhcpv6 client. Or what do you think?

Sorry for the back and forth and thank you a lot for the contribution! The module is quite fresh and thus not everything is yet settled how it should be.

[nbarrientos]

So I would propose to merge them together to one class (e.g. nftables::rules::dhcvp6_client) so one need only to include one class to enable dhcpv6 client. Or what do you think?

Yes, however one might be setting nftables::out_all and therefore only needing the input rule so perhaps having them separate is a good idea.

Sorry for the back and forth and thank you a lot for the contribution! The module is quite fresh and thus not everything is yet settled how it should be.

No worries at all, patches should be discussed and agreed 👍

[duritong]

So I would propose to merge them together to one class (e.g. nftables::rules::dhcvp6_client) so one need only to include one class to enable dhcpv6 client. Or what do you think?

Yes, however one might be setting nftables::out_all and therefore only needing the input rule so perhaps having them separate is a good idea.

Good point, let's keep the classes separate, but then let's add one that combines the two, then we have all use-cases simplified.

[nbarrientos]

let's add one that combines the two, then we have all use-cases simplified.

I've renamed nftables::rules::dhcvp6 to nftables::rules::dhcvp6_client and nftables::rules::out::dhcvp6 to nftables::rules::out::dhcvp6_client.

Since rules in manifests/rules seem to be all input rules whereas those in manifests/rules/out are output ones, where do you suggest to put the class combining both and how to name it?

[duritong]

Since rules in manifests/rules seem to be all input rules whereas those in manifests/rules/out are output ones, where do you suggest to put the class combining both and how to name it?

Yeah, that was also not clear to me. But maybe it would also be ok, to have a class param for that low-level option? I mean Boolean $nftables::dhcpv6_client = false (which is what you proposed earlier, but as I said, let's figure out how to best structure the module :-/ ) and then we just include both and don't have to think about the class name. But maybe there will be classes like that? E.g. for services. So maybe it could also go into a class: nftables::services::dhcpv6_client and this would be the place where we put such classes that combine in & out rules?

[nbarrientos]

So maybe it could also go into a class: nftables::services::dhcpv6_client and this would be the place where we put such classes that combine in & out rules?

Makes sense. It's implemented.

[duritong]

Thank you!