ext_authz should not change the default admittance behavior

[yanavlasov]

PR #34951 made a change where only two gRPC errors by default are treated as "access denied" errors. The reset are treated as infrastructure errors, subject to the fail-open configuration option.

This can result in a behavior change when upgrading the version of Envoy that contains this change. For example if ext_authz server is returning the CANCELLED error (i.e. due to exceeding a rate limit), then before Envoy would deny the request, and after upgrade it would allow it if it is configured to fail open.

This would lead to the bypass of the authorization policy.

Operators may miss this change, if their qualification tests are returning PERMISSION_DENIED status and do not test for conditions where other status codes may be returned.

In general the existing behavior of the security enforcement point can not be altered, as it can lead to unintentional changes to the policy application.

[yanavlasov]

Adding @konstantin-baidin-y42 as PR author

[konstantin-baidin-y42]

I think there is a confusion. The old behavior was to treat all gRPC errors as "access denied", so fail-open setting was broken because codes like "unavailable" and internal error were considered as "access denied".

The key change of my PR is here https://github.com/envoyproxy/envoy/pull/34951/files#diff-c21d3f52d2ab9b9b72049be64abc353ac630adfd5729d376d7f32e6ba5897e76L117

You can see, that the old behaviour was to consider any non-OK response as "access denied". So my PR does the opposite of what you say.

[konstantin-baidin-y42]

the old behavior where only PERMISSION_DENIED and UNAUTHENTICATED are considered as access denied responses

This is literally the new behaviour, not the old one.

[yanavlasov]

@konstantin-baidin-y42 you are correct. I have misdiagnosed our internal problem. However the Issue still stands. I have updated the description to correctly describe the problem.

[yanavlasov]

I have been too hasty in filing this Issue as it is a security problem and it is older than 7 days allowed by the policy. We are treating it as a 0-day on main branch, with rollback and announcement.