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.

Sign up for GitHub

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

Jump to bottom

endpoint: Do not override deny entries with proxy redirects #26344

Merged
merged 2 commits into from
Jul 6, 2023
Merged

endpoint: Do not override deny entries with proxy redirects #26344

merged 2 commits into from
Jul 6, 2023

Commits on Jul 6, 2023

  1. policy: Export DenyPreferredInsertWithChanges, make revertible 

    Export DenyPreferredInsertWithChanges and make it revertible by taking a
map of old values as a new optional argument.

Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
    @jrajahalme
    jrajahalme committed Jul 6, 2023
    Configuration menu
    Copy the full SHA
    a46fd6b View commit details
    Browse the repository at this point in the history

  2. endpoint: Do not override deny entries with proxy redirects 

    Use DenyPreferredInsert instead of directly manipulating policy map state
to make sure deny entries are not overridden by new proxy redirect
entries.

Prior to this fix it was possible for a proxy redirect to be pushed onto
the policy map when it should have been overridden by a deny at least in
these cases:

- L3-only deny with L3/L4 redirect: No redirect should be added as the L3
  is denied

- L3-only deny with L4-only redirect: L4-only redirect should be added
  and an L3/L4 deny should also be added, but the L3/L4 deny is only added
  by deny preferred insert, and is missed when the map is manipulated
  directly. A new test case verifies this.

It is clear that in the latter case the addition of the redirect can not
be completely blocked, so we can't fix this by making AllowsL4 more
restrictive. But also in the former case it is possible that the deny
rule only covers a subset of security identities, while the redirect rule
covers some of the same security identities, but also some more that
should not be blocked. Hence the correct fix here is to leave AllowsL4 to
be L3-independent, and cover these cases with deny preferred insert
instead of adding redirect entries to the map directly.

This commit also contains a related change that allows a redirect entry
to be updated, maybe with a changed proxy port. I've not seen evidence
that this is currently fixing a bug, but it feels like a real
possibility.

Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
    @jrajahalme
    jrajahalme committed Jul 6, 2023
    Configuration menu
    Copy the full SHA
    37a5c68 View commit details
    Browse the repository at this point in the history