Add input filter rule for wireguard #6250
Conversation
Without this rule, in an environment with a default drop, Wireguard traffic will not be accepted. This change adds a static input rule in the same fashion as done for other encapsulation modes (ipip, vxlan) to ensure these packets are accepted. No drop rule is needed however as wireguard does its own validation of incoming messages.
marvin-tigera
added
docs-pr-required
|
|
|
Interesting. I don't believe the equivalent rules for VXLAN / IPIP were added to deal with a default DROP environment, but rather to tighten the security posture by only allowing encap packets from known Calico hosts. That said, I see that they would address that scenario as well. I think this is probably OK to do for Wireguard as well, as the encap'd packets will still be run through Calico policy afterwards and as you say Wireguard will do verification as well based on its configuration. /sem-approve |
|
Oh, and @muff1nman you'll need to sign the contributor agreement to make the CLA bot happy. |
|
Hi @muff1nman, First of all, thanks for your contribution! The wireguard port is added to the failsafes (when enabled) in calico/felix/dataplane/driver.go Line 186 in 13bbda0 I'm just trying to understand, what was the issue you were seeing with default DROP? I'm thinking the fix may be something else (if wireguard is not working properly with default DROP)... |
By also adding an output filter rule for wireguard, this allows wireguard to be consistent with other tunneling rules and removes the need for failsafes entirely. Accordingly failsafe rules for wireguard are removed.
|
@coutinhop Based on our out-of-band conversation I've removed the failsafes logic entirely for wireguard and added any missing rules to the static rules (namely OUTPUT). Feel free to reiterate/summarize our conversation here especially if you feel the latest change doesn't meet your expectations. I'll plan to squash. |
|
@muff1nman thanks for these changes! That was exactly what I had in mind. @caseydavenport @fasaxc could you take a look and see if you can tell anything that could go wrong by switching from the failsafes to these static rules? Thanks! /sem-approve |
|
/sem-approve |
Description
Without this rule, in an environment with a default drop, Wireguard
traffic will not be accepted. This change adds a static input rule in the
same fashion as done for other encapsulation modes (ipip, vxlan) to
ensure these packets are accepted. No drop rule is needed however as
wireguard does its own validation of incoming messages.
Todos
Release Note
Reminder for the reviewer
Make sure that this PR has the correct labels and milestone set.
Every PR needs one
docs-*label.docs-pr-required: This change requires a change to the documentation that has not been completed yet.docs-completed: This change has all necessary documentation completed.docs-not-required: This change has no user-facing impact and requires no docs.Every PR needs one
release-note-*label.release-note-required: This PR has user-facing changes. Most PRs should have this label.release-note-not-required: This PR has no user-facing changes.Other optional labels:
cherry-pick-candidate: This PR should be cherry-picked to an earlier release. For bug fixes only.needs-operator-pr: This PR is related to install and requires a corresponding change to the operator.