grpc / grpc-java / #20476
89%

DEFAULT BRANCH: master
Ran
Jobs 1
Files 698
Run time 2min
Badge
Embed ▾
README BADGES
x

If you need to use a raster PNG badge, change the '.svg' to '.png' in the link

Markdown

Textile

RDoc

HTML

Rst
coverage: 89.327% (+0.009%) from 89.318%
#20476

push

github

web-flow 
xds: fail closed when ext_authz response processing throws (#13051)

When the ext_authz filter processes a `CheckResponse`,
`CheckResponseHandler.handleResponse()` only catches the checked
`HeaderMutationDisallowedException`. An unchecked exception raised
anywhere else in that path escaped `AuthzCallbackObserver.onNext()`.

gRPC reacts to an exception from an application callback by cancelling
the stream, which invokes `onError()`. There, `failure_mode_allow` sent
the request on to the backend. The net effect is that an explicit
`PERMISSION_DENIED` from the authorization server could be turned into
an ALLOW.

The most accessible trigger is a header value that gRPC metadata cannot
represent. `HeaderValue.create()` rejects anything outside horizontal
tab, space and printable ASCII, while the value arrives in a proto
`string` field carrying arbitrary UTF-8. An authorization server that
echoes back a non-ASCII character in a header value is enough; a
malicious server is not required.

`failure_mode_allow` is defined in terms of the authorization service
being unreachable or returning an error. It does not cover a failure to
process a response that the service successfully returned. Routing local
processing errors through that policy is the underlying defect, so
`onNext()` now handles its own failures and fails the call with
`INTERNAL` rather than letting them reach `onError()`.

Fixing this at the observer rather than at each individual throw site
means the invariant holds for future call paths too. The `orElseThrow()`
on the DENY branch is a second instance that was already reachable in
principle and is now covered.

This change also makes a mutation targeting a gRPC-owned header key fail
the RPC instead of being silently dropped.

Testing:

- `deny_withMalformedHeader_failOpen_doesNotReachBackend` is the
regression test. Without the fix it fails with the backend having
received the request despite an explicit denial.
- `deny_withMissingStatus_failsCallW... (continued)

39143 of 43820 relevant lines covered (89.33%)

0.89 hits per line

Coverage Regressions

Lines Coverage ∆ File
2
73.27
 -1.98% ../servlet/src/main/java/io/grpc/servlet/AsyncServletOutputStreamWriter.java
1
93.21
 -0.14% ../okhttp/src/main/java/io/grpc/okhttp/OkHttpClientTransport.java
1
93.83
 -0.62% ../opentelemetry/src/main/java/io/grpc/opentelemetry/OpenTelemetryMetricSink.java
1
87.6
 -0.83% ../rls/src/main/java/io/grpc/rls/LinkedHashLruCache.java
Jobs
ID Job ID Ran Files Coverage
1 #20476.1 698
89.33
Source Files on build #20476