Blake pointed out that as we add authn/authz functionality within Istio via authservice we need to be mindful of how unexpected failures and in general resource ordering could potentially leave applications exposed.
A straightforward way we might be able to somewhat alleviate this would be applying authorization policies (keycloak + authservice functionality) before the Istio resource generation. This would likely be an improvement for first reconciliation but we'd have to probably be more creative for cases where the VirtualService already existed.
"There is a potential security issue where this can fail open:
Create a Package without groups, and wait for UDS operator to create the VirtualService
Update the Package to have authservice (and optionally groups).
If the operator has an error during processing of either of these SSO functions, these functions will throw an error and halt the reconcile... but the old VirtualService will stay around. This means that it fails open.
(similarly you could go from Package+authservice → Package+authservice+groups and get an error in group application and get similar bad fail-open)
I'm unsure what the best solution is. Should we maybe catch errors in the authservice code and tear down the virtual services? That would mean the operator fails closed? Perhaps move istioResources() below this code and pass it a bool destroy option that triggers teardown (this allows better code reuse)?"
Blake pointed out that as we add authn/authz functionality within Istio via authservice we need to be mindful of how unexpected failures and in general resource ordering could potentially leave applications exposed.
A straightforward way we might be able to somewhat alleviate this would be applying authorization policies (keycloak + authservice functionality) before the Istio resource generation. This would likely be an improvement for first reconciliation but we'd have to probably be more creative for cases where the
VirtualServicealready existed."There is a potential security issue where this can fail open:
(similarly you could go from Package+authservice → Package+authservice+groups and get an error in group application and get similar bad fail-open)
I'm unsure what the best solution is. Should we maybe catch errors in the authservice code and tear down the virtual services? That would mean the operator fails closed? Perhaps move
istioResources()below this code and pass it a booldestroyoption that triggers teardown (this allows better code reuse)?"_Originally posted by @bburky in https://github.com/defenseunicorns/uds-core/pull/201#discussion_r1657844035_