grpc / grpc-java / #20364
89%

DEFAULT BRANCH: master
Ran
Jobs 1
Files 686
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.125% (-0.007%) from 89.132%
#20364

push

github

web-flow 
core: reject a JWK entry missing x5c when loading a SPIFFE trust bundle (#12911)

`SpiffeUtil.extractCert` silently truncates a SPIFFE trust bundle when a
JWK `keys[]` entry is
missing `x5c`:

```java
for (Map<String, ?> keyNode : keysNode) {
  checkJwkEntry(keyNode, trustDomainName);
  List<String> rawCerts = JsonUtil.getListOfStrings(keyNode, "x5c");
  if (rawCerts == null) {
    break;          // abandons the loop, dropping every remaining cert
  }
  if (rawCerts.size() != 1) {
    throw new IllegalArgumentException(...);   // sibling paths throw
  }
  ...
}
```

`checkJwkEntry` has already guaranteed the entry is a declared
`x509-svid` key (`use == "x509-svid"`,
`kty` in `{RSA, EC}`), so an entry with no `x5c` is malformed. But
instead of failing, the `break`
abandons the loop and returns only the certificates collected **before**
the bad entry — so a
trust domain whose `keys` array has a missing-`x5c` entry ahead of valid
ones loads a **silently
truncated** trust store, and peers whose chain roots in a dropped CA
fail verification (or a
partially loaded store is accepted with no error).

This also contradicts the method's own contract
(`loadTrustBundleFromFile` javadoc: *"If any
element of the JSON content is invalid or unsupported, an
`IllegalArgumentException` is thrown and
the entire Bundle is considered invalid"*). Every other malformed
condition in `extractCert`
(`use`, `kty`, `kid`, `x5c.size() != 1`, unparseable cert) throws.

**Fix:** throw `IllegalArgumentException` for a missing `x5c`,
consistent with the sibling error
paths and the documented contract, instead of silently dropping
certificates.

**Tests:** added `spiffebundle_missing_x5c.json` and an assertion in
`SpiffeUtilTest.loadTrustBundleFromFileFailureTest`. It fails against
the current code (the old
`break` returns a truncated bundle without throwing) and passes with the
fix. `:grpc-core`
checkstyle/animalsniffer clean.

38061 of 42705 relevant lines covered (89.13%)

0.89 hits per line

Coverage Regressions

Lines Coverage ∆ File
5
83.02
 -9.43% ../core/src/main/java/io/grpc/internal/Http2Ping.java
3
93.22
 -1.69% ../core/src/main/java/io/grpc/internal/AbstractClientStream.java
3
76.72
 -1.72% ../servlet/src/main/java/io/grpc/servlet/ServletServerStream.java
1
90.97
 -0.35% ../core/src/main/java/io/grpc/internal/DelayedClientCall.java
1
96.55
 -0.57% ../xds/src/main/java/io/grpc/xds/client/LoadReportClient.java
Jobs
ID Job ID Ran Files Coverage
1 #20364.1 686
89.13
Source Files on build #20364