feat(inbound): Log traffic with the 'audit' policy #3068
Conversation
alpeb
force-pushed
the
alpeb/policy-audit-mode
branch
from
5d90db0 to
9e23f08
Compare
Audit mode is triggered by the policy controller, which will create an authorization named "audit" allowing traffic for the given target. When the proxy processes an authorization with such name it will log it at INFO. Also, add "audit" to the possible values for `LINKERD2_PROXY_INBOUND_DEFAULT_POLICY`, whose effect is the same as "all-unauthenticated".
alpeb
force-pushed
the
alpeb/policy-audit-mode
branch
from
9e23f08 to
a3f0e53
Compare
There was a problem hiding this comment.
Left a small question on parsing default policies. I was also wondering if it's worth adding some tests.
Integration tests have a policy controller mock that implements the inbound and outbound interfaces (see https://github.com/linkerd/linkerd2-proxy/blob/main/linkerd/app/integration/src/policy.rs). There are also some unit tests in:
- Inbound TCP stack (https://github.com/linkerd/linkerd2-proxy/blob/main/linkerd/app/inbound/src/accept.rs#L154-L170)
- Inbound policy module (https://github.com/linkerd/linkerd2-proxy/blob/main/linkerd/app/inbound/src/policy.rs#L211).
I wonder if it makes sense to add a test or two to ensure this all behaves as expected.
linkerd/app/src/env.rs
Outdated
| @@ -1008,7 +1008,7 @@ fn parse_default_policy( | |||
| "all-authenticated" => { | |||
| Ok(inbound::policy::defaults::all_authenticated(detect_timeout).into()) | |||
| } | |||
| "all-unauthenticated" => { | |||
| "all-unauthenticated" | "audit" => { | |||
There was a problem hiding this comment.
question: if we treat this as an all-unauthenticated by default, when we create the default policy will it still have the correct metadata?
What would happen if for example we have an audit default policy and we get an inbound connection? Will we still log with the audit labels?
There was a problem hiding this comment.
Great point, I missed that! This is hard to test manually, but your pointers above might help. Moving the PR into draft till I address this.
I've added a new new branch in the matcher parsing LINKERD2_PROXY_INBOUND_DEFAULT_POLICY that creates a similiar ServerPolicy that for all_unauthenticated, but named "audit", which is what causes the special log entry to be produced. I tested this manually using a proxy with a hard-coded `policy::Config::Fixed` policy with an audit default ServerPolicy to avoid overriding LINKERD2_PROXY_INBOUND_DEFAULT_POLICY's policy with something coming from the policy controller. As for tests, I couldn't quite find a place for this that didn't duplicate what we already have for all_unauthenticated. We could add something with some tracing subscriber that would catch the log entry but not sure if it's worth it. Open to more suggestions.
|
I've added a new new branch in the matcher parsing LINKERD2_PROXY_INBOUND_DEFAULT_POLICY that creates a similiar ServerPolicy as the one used for all_unauthenticated, but named "audit", this name being the only thing that causes the special log entry to be produced. I tested this manually using a proxy with a hard-coded As for unit/integration tests, I couldn't quite find a place for this that didn't duplicate what we already have for all_unauthenticated. We could add something with some tracing subscriber that would catch the log entry but not sure if it's worth it. Open to more suggestions. |
Followup to #12845 This expands the policy controller index in the following ways: - Adds the new Audit variant to the DefaultPolicy enum - Expands the function that synthesizes the authorizations for a given default policy (DefaultPolicy::default_authzs) so that it also creates an Unauthenticated client auth and a allow-all NetworkMatch for the new Audit default policy. - Now that a Server can have a default policy different than Deny, when generating InboundServer authorizations (PolicyIndex::client_authzs) make sure to append the default authorizations when DefaultPolicy is Allow or Audit Also, the admission controller ensures the new accessPolicy field contains a valid value. Required test changes are addressed in #12847. Also note you'll need the proxy changes at linkerd/linkerd2-proxy#3068 to make this work. Please check linkerd/website#1805 for how this is supposed to work from the user's perspective.
Followup to #12845 This expands the policy controller index in the following ways: - Adds the new Audit variant to the DefaultPolicy enum - Expands the function that synthesizes the authorizations for a given default policy (DefaultPolicy::default_authzs) so that it also creates an Unauthenticated client auth and a allow-all NetworkMatch for the new Audit default policy. - Now that a Server can have a default policy different than Deny, when generating InboundServer authorizations (PolicyIndex::client_authzs) make sure to append the default authorizations when DefaultPolicy is Allow or Audit Also, the admission controller ensures the new accessPolicy field contains a valid value. Required test changes are addressed in #12847. Also note you'll need the proxy changes at linkerd/linkerd2-proxy#3068 to make this work. Please check linkerd/website#1805 for how this is supposed to work from the user's perspective.
olix0r
changed the title
Followup to #12845 This expands the policy controller index in the following ways: - Adds the new Audit variant to the DefaultPolicy enum - Expands the function that synthesizes the authorizations for a given default policy (DefaultPolicy::default_authzs) so that it also creates an Unauthenticated client auth and a allow-all NetworkMatch for the new Audit default policy. - Now that a Server can have a default policy different than Deny, when generating InboundServer authorizations (PolicyIndex::client_authzs) make sure to append the default authorizations when DefaultPolicy is Allow or Audit Also, the admission controller ensures the new accessPolicy field contains a valid value. Required test changes are addressed in #12847. Also note you'll need the proxy changes at linkerd/linkerd2-proxy#3068 to make this work. Please check linkerd/website#1805 for how this is supposed to work from the user's perspective.
Audit mode is triggered by the policy controller, which will create an authorization named "audit" allowing traffic for the given target. When the proxy processes an authorization with such name it will log it at INFO.
Also, add "audit" to the possible values for
LINKERD2_PROXY_INBOUND_DEFAULT_POLICY, whose effect is the same as "all-unauthenticated".Please check linkerd/website#1805 for how this is supposed to work from the user's perspective.