ext_authz filter: failure_mode_allow not working

[michalholecek]

Hi,
modifying this option does not change behaviour of the filter. If ext_authz service is not accessible auth filter rejects the request and envoy returns 503 Service Unavailable, even if failure_mode_allow is set to true.
Filter conf:

            http_filters:
              - name: envoy.ext_authz
                config:
                  http_service:
                      server_uri:
                        uri: 0.0.0.0:8080
                        cluster: ext-authz
                        timeout: 0.2s
                        failure_mode_allow: true

Envoy log:

[2018-08-13 13:54:17.638][26531][debug][http] source/common/http/async_client_impl.cc:93] async http request response headers (end_stream=false):
':status', '503'
'content-length', '57'
'content-type', 'text/plain'

[2018-08-13 13:54:17.638][26531][debug][filter] source/extensions/filters/http/ext_authz/ext_authz.cc:104] [C0][S4185710137229910656] Ext_authz rejected the request
[2018-08-13 13:54:17.638][26531][debug][lua] source/extensions/filters/common/lua/lua.cc:35] coroutine finished
[2018-08-13 13:54:17.638][26531][debug][http] source/common/http/conn_manager_impl.cc:1045] [C0][S4185710137229910656] encoding headers via codec (end_stream=false):
':status', '503'
'content-length', '57'
'content-type', 'text/plain'
'date', 'Mon, 13 Aug 2018 11:54:17 GMT'
'server', 'envoy'

[2018-08-13T11:54:17.637Z] "GET / HTTP/1.1" 503 UAEX 0 57 1 - "-" "curl/7.52.1" "8843225c-0175-4979-b42d-d1f08edab450" "0.0.0.0:777" "-"

[dio]

@michalholecek could you please test if the following config works?

http_filters:
  - name: envoy.ext_authz
    config:
      http_service:
        server_uri:
          uri: 0.0.0.0:8080
          cluster: ext-authz
          timeout: 0.2s
      failure_mode_allow: true

It seems the failure_mode_allow should be in the same level with http_service, i.e. config.failure_mode_allow.

Ref: https://www.envoyproxy.io/docs/envoy/latest/api-v2/config/filter/http/ext_authz/v2alpha/ext_authz.proto#envoy-api-msg-config-filter-http-ext-authz-v2alpha-extauthz

[michalholecek]

Tried it, does not work either.

[dio]

OK, I think we probably can add a special handler for 5xx in here:

envoy/source/extensions/filters/common/ext_authz/ext_authz_http_impl.cc

Lines 70 to 78 in 1381673

	if (StringUtil::atoul(response->headers().Status()->value().c_str(), status_code)) {
	if (status_code == enumToInt(Http::Code::OK)) {
	authz_response->status = CheckStatus::OK;
	authz_response->status_code = Http::Code::OK;
	} else {
	authz_response->status = CheckStatus::Denied;
	authz_response->body = response->bodyAsString();
	authz_response->status_code = static_cast<Http::Code>(status_code);
	}

and tell the check request to return response->status as CheckStatus::Error instead of CheckStatus::Denied.

Hence, we honor the failure_mode_allow in the following line:

envoy/source/extensions/filters/http/ext_authz/ext_authz.cc

Line 103 in 1381673

(response->status == CheckStatus::Error && !config_->failureModeAllow())) {

@gsagula thoughts?

[gsagula]

@michalholecek Thanks for reporting it.

and tell the check request to return response->status as CheckStatus::Error instead of CheckStatus::Denied

@dio If I remember correctly this logic has been implemented before my modifications, but I think the original intention here was to return denied whenever the authorization server gives a code that is not OK unless failure_mode_allow is true.

Please, let me take a look at this issue. I will send a PR with the fix.

[dio]

Thanks, @gsagula!

but I think the original intention here was to return denied whenever the authorization server gives a code that is not OK unless failure_mode_allow is true.

Seems good, but IMO semantically, given this condition (the server is off), it is better to say that the auth server is in error state (since we know it can't be reached). But, I'm not sure what is the implication to the downstream (i.e. whether we should convey that info or not, if yes how).

[stale]

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or other activity occurs. Thank you for your contributions.

[stale]

This issue has been automatically closed because it has not had activity in the last 37 days. If this issue is still valid, please ping a maintainer and ask them to label it as "help wanted". Thank you for your contributions.