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.

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

extauthz can't send header values that contains characters of the extended ASCII codes #22996

Open
MiguelAng86 opened this issue Sep 6, 2022 · 12 comments

Comments

@MiguelAng86
Copy link

Description:
Hi, when I send a request with postman against envoy that contains a header value with any character of the extended ASCII codes, envoy shows this log:
[libprotobuf ERROR external/com_google_protobuf/src/google/protobuf/wire_format_lite.cc:581] String field 'envoy.service.auth.v3.AttributeContext.HttpRequest.HeadersEntry.value' contains invalid UTF-8 data when serializing a protocol buffer. Use the 'bytes' type if you intend to send raw bytes.

I'm using envoy version 1.21.4.
The request contains this value:
example-header: España

Thanks in advance.

@MiguelAng86 MiguelAng86 added bug triage Issue requires triage labels Sep 6, 2022
@MiguelAng86 MiguelAng86 changed the title extauthz can't send headers characters of the extended ASCII codes extauthz can't send header values that contains characters of the extended ASCII codes Sep 6, 2022
@wbpcode wbpcode added area/ext_authz and removed triage Issue requires triage labels Sep 9, 2022
@wbpcode
Copy link
Member

wbpcode commented Sep 9, 2022

Because the type of header value is string and it's different with bytes in proto.

See similar problem. #13131

@MiguelAng86
Copy link
Author

Hi @wbpcode,

Yes, I already saw the case with the body and how the pack_as_bytes property was added, but this does not work with the headers. A similar setup would be needed when protobuf is going to parse the headers.

this would be possible?

Thanks in advance.

@RiverPhillips
Copy link
Contributor

I'd like to try and add similar functionality to pack_as_bytes to the headers.

/assign @RiverPhillips

@repokitteh-read-only
Copy link

RiverPhillips is not allowed to assign users.

🐱

Caused by: a #22996 (comment) was created by @RiverPhillips.

see: more, trace.

@dio
Copy link
Member

dio commented Sep 17, 2022

Putting here for discussion: seems like Envoy currently "accepts" non-ASCII header values, which probably shouldn't (?). Reference: https://stackoverflow.com/a/48138818 (Non-ASCII characters are deprecated. You can send them, but there's no guarantee that the recipient will do what you expect it to).

Note: a HeaderValue is a string:

string value = 2 [
.

@RiverPhillips
Copy link
Contributor

Yeah, it seems like the sender would need to encode their headers to send non-ASCII characters as defined in rfc8187

@klarose
Copy link
Contributor

klarose commented Sep 28, 2022

I have also run into this issue a few times. That said, where it differs for me is in that it seems to work fine with any utf-8 characters. e.g. I sent the cookie character: 🍪 using its utf-8 encoding: \xF0\x9F\x8D\xAA

This aligns with my reading of the protobuf code, and with the error message. I'm not sure why the ñ is somehow breaking it. Is it perhaps not encoded as utf-8?

Either way, I think that regardless of the validity of the header, the portion of the envoy pipeline responsible for trigger the ext-authz service should be somewhat lenient so that the service itself can decide what to do with the header. This is particularly important for clients/servers which may be substandard in their standards-adherence, but for which a reverse proxy is the only reasonable method to make them work over the Internet.

@github-actions
Copy link

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 "no stalebot" or other activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale stalebot believes this issue/PR has not been touched recently label Oct 28, 2022
@dio
Copy link
Member

dio commented Oct 28, 2022

No stale.

@mattklein123 mattklein123 added help wanted Needs help! and removed stale stalebot believes this issue/PR has not been touched recently labels Oct 28, 2022
@dio
Copy link
Member

dio commented Nov 2, 2022

@MiguelAng86 could you share your config and also the HTTP client you used? I tested this quickly (sorry was not able to report this earlier). I can inspect the check request payload (to a gRPC auth server) has the following entry (no protobuf "encoding" failure):

      headers {
        key: "example-header"
        value: "Espa\303\261a"
      }

This is done by running (envoy v1.21.4):

envoy -c examples/ext_authz/config/grpc-service/v3.yaml -l trace

with a slight change to examples/ext_authz/config/grpc-service/v3.yaml:

diff --git a/examples/ext_authz/config/grpc-service/v3.yaml b/examples/ext_authz/config/grpc-service/v3.yaml
index f9e2bea51c..81b5b43cfd 100644
--- a/examples/ext_authz/config/grpc-service/v3.yaml
+++ b/examples/ext_authz/config/grpc-service/v3.yaml
@@ -62,5 +62,5 @@ static_resources:
         - endpoint:
             address:
               socket_address:
-                address: ext_authz-grpc-service
+                address: localhost

Then sending the request (with that example-header header with España as the value):

 curl -H "example-header: España" localhost:8000 --verbose

The full relevant log:

2022-11-02 09:04:53.493][52516][debug][http] [source/common/http/filter_manager.cc:840] [C0][S14304257025704593311] request end stream
[2022-11-02 09:04:53.495][52516][trace][filter] [source/extensions/filters/http/ext_authz/ext_authz.cc:60] [C0][S14304257025704593311] ext_authz filter calling authorization server
[2022-11-02 09:04:53.496][52516][trace][ext_authz] [source/extensions/filters/common/ext_authz/ext_authz_grpc_impl.cc:42] Sending CheckRequest: attributes {
  source {
    address {
      socket_address {
        address: "127.0.0.1"
        port_value: 50060
      }
    }
  }
  destination {
    address {
      socket_address {
        address: "127.0.0.1"
        port_value: 8000
      }
    }
  }
  request {
    time {
      seconds: 1667379893
      nanos: 492947825
    }
    http {
      id: "14304257025704593311"
      method: "GET"
      headers {
        key: ":authority"
        value: "localhost:8000"
      }
      headers {
        key: ":method"
        value: "GET"
      }
      headers {
        key: ":path"
        value: "/"
      }
      headers {
        key: ":scheme"
        value: "http"
      }
      headers {
        key: "accept"
        value: "*/*"
      }
      headers {
        key: "example-header"
        value: "Espa\303\261a"
      }
      headers {
        key: "user-agent"
        value: "curl/7.68.0"
      }
      headers {
        key: "x-forwarded-proto"
        value: "http"
      }
      headers {
        key: "x-request-id"
        value: "b1f93009-21d0-4835-834c-29c6d6e06f0a"
      }
      path: "/"
      host: "localhost:8000"
      scheme: "http"
      protocol: "HTTP/1.1"
    }
  }
  metadata_context {
  }
}

[2022-11-02 09:04:53.496][52516][debug][router] [source/common/router/router.cc:486] [C0][S4894333874324175583] cluster 'ext_authz-grpc-service' match for URL '/envoy.service.auth.v3.Authorization/Check'

@klarose
Copy link
Contributor

klarose commented Nov 3, 2022

curl -H "example-header: España" localhost:8000 --verbose

I can reproduce it using something like this:

curl https://website-running-envoy  -H "example-header: $(echo -e '\xe4')"

I think the issue here is that it's an invalid utf-8 encoding. I don't know why OP's seemingly good header doesn't work, but I do see cases with garbage values occasionally causing this problem in production. This seems to happen for us when some misbehaving serverside code puts a bad cookie on a domain. Users who hit that case are stuck. Obviously we should fix said misbehaving code, but that cookie being problematic should really only affect the subsystems depending on it, not our core authz subsystem.

@dio
Copy link
Member

dio commented Nov 8, 2022

Thanks, @klarose. Will check this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants