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

core: Spiffe Utils #11522

Closed
wants to merge 16 commits into from
Closed
39 changes: 31 additions & 8 deletions core/src/main/java/io/grpc/internal/JsonParser.java
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
public static Object parse(String raw) throws IOException {
JsonReader jr = new JsonReader(new StringReader(raw));
try {
return parseRecursive(jr);
return parseRecursive(jr, false);
} finally {
try {
jr.close();
Expand All @@ -56,13 +56,32 @@
}
}

private static Object parseRecursive(JsonReader jr) throws IOException {
/**
* Parses a json string, returning either a {@code Map<String, ?>}, {@code List<?>},
* {@code String}, {@code Double}, {@code Boolean}, or {@code null}. Fails if duplicate names
* found.
*/
public static Object parseNoDuplicates(String raw) throws IOException {
JsonReader jr = new JsonReader(new StringReader(raw));
try {
return parseRecursive(jr, true);
} finally {
try {
jr.close();
} catch (IOException e) {
logger.log(Level.WARNING, "Failed to close", e);

Check warning on line 72 in core/src/main/java/io/grpc/internal/JsonParser.java

View check run for this annotation

Codecov / codecov/patch

core/src/main/java/io/grpc/internal/JsonParser.java#L71-L72

Added lines #L71 - L72 were not covered by tests
}
}
}

private static Object parseRecursive(JsonReader jr, Boolean... failOnDuplicates)
throws IOException {
checkState(jr.hasNext(), "unexpected end of JSON");
switch (jr.peek()) {
case BEGIN_ARRAY:
return parseJsonArray(jr);
return parseJsonArray(jr, failOnDuplicates);
case BEGIN_OBJECT:
return parseJsonObject(jr);
return parseJsonObject(jr, failOnDuplicates);
case STRING:
return jr.nextString();
case NUMBER:
Expand All @@ -76,24 +95,28 @@
}
}

private static Map<String, ?> parseJsonObject(JsonReader jr) throws IOException {
private static Map<String, ?> parseJsonObject(JsonReader jr, Boolean... failOnDuplicates)
throws IOException {
jr.beginObject();
Map<String, Object> obj = new LinkedHashMap<>();
while (jr.hasNext()) {
String name = jr.nextName();
Object value = parseRecursive(jr);
if (failOnDuplicates.length > 0 && failOnDuplicates[0] && obj.containsKey(name)) {
throw new IllegalArgumentException("Duplicate key found: " + name);
}
Object value = parseRecursive(jr, failOnDuplicates);
obj.put(name, value);
}
checkState(jr.peek() == JsonToken.END_OBJECT, "Bad token: " + jr.getPath());
jr.endObject();
return Collections.unmodifiableMap(obj);
}

private static List<?> parseJsonArray(JsonReader jr) throws IOException {
private static List<?> parseJsonArray(JsonReader jr, Boolean... params) throws IOException {
jr.beginArray();
List<Object> array = new ArrayList<>();
while (jr.hasNext()) {
Object value = parseRecursive(jr);
Object value = parseRecursive(jr, params);
array.add(value);
}
checkState(jr.peek() == JsonToken.END_ARRAY, "Bad token: " + jr.getPath());
Expand Down
245 changes: 245 additions & 0 deletions core/src/main/java/io/grpc/internal/SpiffeUtil2.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,245 @@
/*
* Copyright 2024 The gRPC Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package io.grpc.internal;

import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;

import com.google.common.base.Optional;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.security.cert.Certificate;
import java.security.cert.CertificateException;
import java.security.cert.CertificateFactory;
import java.security.cert.CertificateParsingException;
import java.security.cert.X509Certificate;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.logging.Level;
import java.util.logging.Logger;

/**
* Will be merged with SpiffeUtil.
* Provides utilities to load, extract, and parse SPIFFE ID according the SPIFFE ID standard.
*
*/
public final class SpiffeUtil2 {

Check warning on line 51 in core/src/main/java/io/grpc/internal/SpiffeUtil2.java

View check run for this annotation

Codecov / codecov/patch

core/src/main/java/io/grpc/internal/SpiffeUtil2.java#L51

Added line #L51 was not covered by tests

private static final Logger log = Logger.getLogger(SpiffeUtil2.class.getName());

private static final Integer URI_SAN_TYPE = 6;
private static final String USE_PARAMETER_VALUE = "x509-svid";
private static final String KTY_PARAMETER_VALUE = "RSA";

/**
* Returns the SPIFFE ID from the leaf certificate, if present.
*
* @param certChain certificate chain to extract SPIFFE ID from
*/
public static Optional<SpiffeId> extractSpiffeId(X509Certificate[] certChain)
throws CertificateParsingException {
checkArgument(checkNotNull(certChain, "certChain").length > 0, "CertChain can't be empty");
Collection<List<?>> subjectAltNames = certChain[0].getSubjectAlternativeNames();
if (subjectAltNames == null) {
return Optional.absent();
}
String uri = null;
for (List<?> altName : subjectAltNames) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a comment before this for loop to add some explanation, e.g. "Search for the unique URI SAN."

if (altName.size() < 2 ) {
continue;

Check warning on line 74 in core/src/main/java/io/grpc/internal/SpiffeUtil2.java

View check run for this annotation

Codecov / codecov/patch

core/src/main/java/io/grpc/internal/SpiffeUtil2.java#L74

Added line #L74 was not covered by tests
}
if (URI_SAN_TYPE.equals(altName.get(0))) {
if (uri != null) {
throw new IllegalArgumentException("Multiple URI SAN values found in the leaf cert.");
}
uri = (String) altName.get(1);
}
}
if (uri == null) {
return Optional.absent();

Check warning on line 84 in core/src/main/java/io/grpc/internal/SpiffeUtil2.java

View check run for this annotation

Codecov / codecov/patch

core/src/main/java/io/grpc/internal/SpiffeUtil2.java#L84

Added line #L84 was not covered by tests
}
// Real validation will be plugged in via another PR (SpiffeUtil).
String[] parts = uri.substring(9).split("/", 2);
String trustDomain = parts[0];
String path = parts[1];
return Optional.of(new SpiffeId(trustDomain, path));
}

/**
* Loads a SPIFFE trust bundle from a file, parsing it from the JSON format.
* In case of success, returns trust domains, their associated sequence numbers, and X.509
* certificates.
*
* @param trustBundleFile the file path to the JSON file containing the trust bundle
* @see <a href="https://github.com/spiffe/spiffe/blob/main/standards/SPIFFE_Trust_Domain_and_Bundle.md">JSON format</a>
* @see <a href="https://github.com/spiffe/spiffe/blob/main/standards/X509-SVID.md#61-publishing-spiffe-bundle-elements">JWK entry format</a>
*/
public static SpiffeBundle loadTrustBundleFromFile(String trustBundleFile) throws IOException {
Map<String, ?> trustDomainsNode = readTrustDomainsFromFile(trustBundleFile);
Map<String, List<X509Certificate>> trustBundleMap = new HashMap<>();
Map<String, Long> sequenceNumbers = new HashMap<>();
for (String trustDomainName : trustDomainsNode.keySet()) {
Map<String, ?> domainNode = JsonUtil.getObject(trustDomainsNode, trustDomainName);
if (domainNode == null || domainNode.size() == 0) {
trustBundleMap.put(trustDomainName, Collections.emptyList());
continue;
}
Long sequenceNumber = JsonUtil.getNumberAsLong(domainNode, "sequence_number");
sequenceNumbers.put(trustDomainName, sequenceNumber == null ? -1L : sequenceNumber);
List<Map<String, ?>> keysNode = JsonUtil.getListOfObjects(domainNode, "keys");
if (keysNode == null || keysNode.size() == 0) {
trustBundleMap.put(trustDomainName, Collections.emptyList());
continue;

Check warning on line 117 in core/src/main/java/io/grpc/internal/SpiffeUtil2.java

View check run for this annotation

Codecov / codecov/patch

core/src/main/java/io/grpc/internal/SpiffeUtil2.java#L116-L117

Added lines #L116 - L117 were not covered by tests
}
trustBundleMap.put(trustDomainName, extractCert(keysNode, trustDomainName));
}
return new SpiffeBundle(sequenceNumbers, trustBundleMap);
}

private static Map<String, ?> readTrustDomainsFromFile(String filePath) throws IOException {
Path path = Paths.get(checkNotNull(filePath, "trustBundleFile"));
String json = new String(Files.readAllBytes(path), StandardCharsets.UTF_8);
Object jsonObject = JsonParser.parseNoDuplicates(json);
if (!(jsonObject instanceof Map)) {
throw new IllegalArgumentException(
"SPIFFE Trust Bundle should be a JSON object. Found: "
+ (jsonObject == null ? null : jsonObject.getClass()));
}
@SuppressWarnings("unchecked")
Map<String, ?> root = (Map<String, ?>)jsonObject;
Map<String, ?> trustDomainsNode = JsonUtil.getObject(root, "trust_domains");
Copy link
Contributor

@matthewstevenson88 matthewstevenson88 Sep 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Can trust_domains be a constant? Similarly for the other string literals below.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had a similar thought, but other json parsing examples in the repo never use constants for such cases, for example https://github.com/grpc/grpc-java/blob/master/core/src/main/java/io/grpc/internal/ServiceConfigUtil.java

checkArgument(checkNotNull(trustDomainsNode,
"Mandatory trust_domains element is missing").size() > 0,
"Mandatory trust_domains element is missing");
return trustDomainsNode;
}

private static boolean checkJwkEntry(Map<String, ?> jwkNode, String trustDomainName) {
String kty = JsonUtil.getString(jwkNode, "kty");
if (kty == null || !kty.equals(KTY_PARAMETER_VALUE)) {
log.log(Level.SEVERE, String.format("'kty' parameter must be '%s' but '%s' found. "
+ "Skipping certificate loading for trust domain '%s'.", KTY_PARAMETER_VALUE, kty,
trustDomainName));
return false;
}
String kid = JsonUtil.getString(jwkNode, "kid");
if (kid != null && !kid.equals("")) {
log.log(Level.SEVERE, String.format("'kid' parameter must not be set but value '%s' "
+ "found. Skipping certificate loading for trust domain '%s'.", kid,
trustDomainName));
return false;
}
String use = JsonUtil.getString(jwkNode, "use");
if (use == null || !use.equals(USE_PARAMETER_VALUE)) {
log.log(Level.SEVERE, String.format("'use' parameter must be '%s' but '%s' found. "
+ "Skipping certificate loading for trust domain '%s'.", USE_PARAMETER_VALUE, use,
trustDomainName));
return false;
}
return true;
}

private static List<X509Certificate> extractCert(List<Map<String, ?>> keysNode,
String trustDomainName) {
List<X509Certificate> result = new ArrayList<>();
for (Map<String, ?> keyNode : keysNode) {
if (!checkJwkEntry(keyNode, trustDomainName)) {
break;
}
String rawCert = JsonUtil.getString(keyNode, "x5c");
if (rawCert == null) {
break;

Check warning on line 176 in core/src/main/java/io/grpc/internal/SpiffeUtil2.java

View check run for this annotation

Codecov / codecov/patch

core/src/main/java/io/grpc/internal/SpiffeUtil2.java#L176

Added line #L176 was not covered by tests
}
InputStream stream = new ByteArrayInputStream(rawCert.getBytes(StandardCharsets.UTF_8));
try {
Collection<? extends Certificate> certs = CertificateFactory.getInstance("X509")
.generateCertificates(stream);
if (certs.size() != 1) {
log.log(Level.SEVERE, String.format("Exactly 1 certificate is expected, but %s found for "
+ "domain %s.", certs.size(), trustDomainName));
} else {
result.add(certs.toArray(new X509Certificate[0])[0]);
}
} catch (CertificateException e) {
log.log(Level.SEVERE, String.format("Certificate for domain %s can't be parsed.",

Check warning on line 189 in core/src/main/java/io/grpc/internal/SpiffeUtil2.java

View check run for this annotation

Codecov / codecov/patch

core/src/main/java/io/grpc/internal/SpiffeUtil2.java#L188-L189

Added lines #L188 - L189 were not covered by tests
trustDomainName), e);
}
}
return result;
}

// Will be merged with other PR
public static class SpiffeId {

private final String trustDomain;
private final String path;

private SpiffeId(String trustDomain, String path) {
this.trustDomain = trustDomain;
this.path = path;
}

public String getTrustDomain() {
return trustDomain;
}

public String getPath() {
return path;
}
}

/**
* Represents a Trust Bundle inspired by SPIFFE Bundle Format standard. Only trust domain's
* sequence numbers and x509 certificates are supported.
* @see <a href="https://github.com/spiffe/spiffe/blob/main/standards/SPIFFE_Trust_Domain_and_Bundle.md#4-spiffe-bundle-format">Standard</a>
*/
public static final class SpiffeBundle {

private final ImmutableMap<String, Long> sequenceNumbers;

private final ImmutableMap<String, ImmutableList<X509Certificate>> bundleMap;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Should it be ImmutableSet rather than ImmutableList?

Rationale: There is no inherent order to the trusted certs in this list. If we ultimately need to feed this in to an API that only accepts a List, I'm fine keeping it tho.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a really interesting question. JDK API uses arrays, and gRPC Java uses lists -

void updateTrustedRoots(List<X509Certificate> trustedRoots);

I suspect the issue with Sets is that X509Certificate itself is an abstract class, so developers can override equals/hashcode and it might lead to bugs.
So I'd just prefer to leave it a List similar with surrounding classes


public SpiffeBundle(Map<String, Long> sequenceNumbers,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Prefer using a static factory (and a private constructor) instead in case we need to do work in the constructor in the future (e.g. validation of the inputs).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This class is a part of non-public API and the intent is to use it as a simple 'value' class, similar to https://github.com/grpc/grpc-java/blob/master/core/src/main/java/io/grpc/internal/SpiffeUtil.java#L103. If we need to add some validation, I'd prefer to do it in a caller code instead.
But definitely constructor should be private, fixed

Map<String, List<X509Certificate>> trustDomainMap) {
this.sequenceNumbers = ImmutableMap.copyOf(sequenceNumbers);
ImmutableMap.Builder<String, ImmutableList<X509Certificate>> builder = ImmutableMap.builder();
for (Map.Entry<String, List<X509Certificate>> entry : trustDomainMap.entrySet()) {
builder.put(entry.getKey(), ImmutableList.copyOf(entry.getValue()));
}
this.bundleMap = builder.build();
}

public ImmutableMap<String, Long> getSequenceNumbers() {
return sequenceNumbers;
}

public ImmutableMap<String, ImmutableList<X509Certificate>> getBundleMap() {
return bundleMap;
}
}
}
7 changes: 7 additions & 0 deletions core/src/test/java/io/grpc/internal/JsonParserTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -123,4 +123,11 @@ public void objectStringName() throws IOException {

assertEquals(expected, JsonParser.parse("{\"hi\": 2}"));
}

@Test
public void duplicate() throws IOException {
thrown.expect(IllegalArgumentException.class);

JsonParser.parseNoDuplicates("{\"hi\": 2, \"hi\": 3}");
}
}
Loading