From 00eb86949fb4b0a8428abe08f8898a74c23f8c59 Mon Sep 17 00:00:00 2001 From: Max Lambrecht Date: Thu, 11 Jun 2020 17:09:55 -0300 Subject: [PATCH] Addressing PR comments. Adding documentation. Amendments in READMEs and javadoc comments. Some refactors to improve code and clarity. Signed-off-by: Max Lambrecht --- README.md | 2 +- java-spiffe-core/README.md | 43 +++++++++++++------ .../spiffe/bundle/jwtbundle/JwtBundle.java | 9 ++-- .../spiffe/bundle/x509bundle/X509Bundle.java | 9 ++-- .../bundle/x509bundle/X509BundleSet.java | 10 +++-- .../java/spiffe/workloadapi/JwtSource.java | 6 +-- .../java/spiffe/workloadapi/X509Source.java | 6 ++- .../bundle/jwtbundle/JwtBundleSetTest.java | 2 +- .../bundle/jwtbundle/JwtBundleTest.java | 12 +++--- .../bundle/x509bundle/X509BundleSetTest.java | 8 ++-- .../jwtsvid/JwtSvidParseAndValidateTest.java | 6 +-- .../jwtsvid/JwtSvidParseInsecureTest.java | 4 +- 12 files changed, 72 insertions(+), 45 deletions(-) diff --git a/README.md b/README.md index 5fc7fef..b52123b 100644 --- a/README.md +++ b/README.md @@ -4,7 +4,7 @@ The JAVA-SPIFFE library provides functionality to interact with the Workload API to fetch X.509 and JWT SVIDs and Bundles, and a Java Security Provider implementation to be plugged into the Java Security architecture. This is essentially -a X.509-SVID based KeyStore and TrustStore implementation that handles the certificates in memory and receives the updates +an X.509-SVID based KeyStore and TrustStore implementation that handles the certificates in memory and receives the updates asynchronously from the Workload API. The KeyStore handles the Certificate chain and Private Key to prove identity in a TLS connection, and the TrustStore handles the trusted bundles (supporting federated bundles) and performs peer's certificate and SPIFFE ID verification. diff --git a/java-spiffe-core/README.md b/java-spiffe-core/README.md index 724fe83..37694e5 100644 --- a/java-spiffe-core/README.md +++ b/java-spiffe-core/README.md @@ -6,7 +6,7 @@ Core functionality to fetch, process and validate X.509 and JWT SVIDs and Bundle A `spiffe.workloadapi.X509Source` represents a source of X.509 SVIDs and X.509 bundles maintained via the Workload API. -To create a new X509 Source: +To create a new X.509 Source: ``` X509Source x509Source; @@ -20,13 +20,13 @@ To create a new X509 Source: X509Bundle bundle = x509Source.getBundleForTrustDomain(TrustDomain.of("example.org")); ``` -The `newSource()` blocks until the X.509 materials can be retrieved from the Workload API and the X509Source is -initialized with the X.509 SVIDs and Bundles. A `X509 context watcher` is configured on the X509Source to get automatically +The `newSource()`method blocks until the X.509 materials can be retrieved from the Workload API and the `X509Source` is +initialized with the X.509 SVIDs and Bundles. A `X.509 context watcher` is configured on the `X509Source` to automatically get the updates from the Workload API. This watcher performs retries if at any time the connection to the Workload API reports an error. The socket endpoint address is configured through the environment variable `SPIFFE_ENDPOINT_SOCKET`. Another way to -configure it is by providing a `X509SourceOptions` instance to the `newSource` method: +configure it is by providing an `X509SourceOptions` instance to the `newSource` method: ``` X509Source.X509SourceOptions x509SourceOptions = X509Source.X509SourceOptions @@ -42,16 +42,16 @@ It allows to configure another SVID picker. By default, the first SVID is used. ### Configure a timeout for X509Source initialization -The method `X509Source newSource()` blocks waiting until a X509 context is fetched. The X509 context fetch is retried +The method `X509Source newSource()` blocks waiting until a X.509 context is fetched. The X.509 context fetch is retried using an exponential backoff policy with this progression of delays between retries: 1 second, 2 seconds, 4, 8, 16, 32, 60, 60, 60... It retries indefinitely unless a timeout is configured. This timeout can be configured either providing it through the `newSource(Duration timeout)` method or using a System property: -`spiffe.newX509Source.timeout=30` +`spiffe.newX509Source.timeout=PT30S` -The Time Unit is seconds. +The `timout` duration is expressed in `ISO-8601` format. ## JWT Source @@ -73,15 +73,34 @@ To create a new JWT Source: JwtBundle bundle = jwtSource.getBundleForTrustDomain(TrustDomain.of("example.org")); ``` -The `newSource()` blocks until the JWT materials can be retrieved from the Workload API and the JwtSource is -initialized with the JWT Bundles. A `JWT context watcher` is configured on the JwtSource to get automatically +The `newSource()` method blocks until the JWT materials can be retrieved from the Workload API and the `JwtSource` is +initialized with the JWT Bundles. A `JWT context watcher` is configured on the JwtSource to automatically get the updates from the Workload API. This watcher performs retries if at any time the connection to the Workload API reports an error. The socket endpoint address is configured through the environment variable `SPIFFE_ENDPOINT_SOCKET`. -## Netty Event Loop thread number configuration +Another way to configure it is by providing an `JwtSourceOptions` instance to the `newSource` method: -Use the variable `io.netty.eventLoopThreads` to configure the number of threads for the Netty Event Loop Group. +``` + JwtSource.JwtSourceOptions jwtSourceOptions = JwtSource.JwtSourceOptions + .builder() + .spiffeSocketPath("unix:/tmp/agent-other.sock") + .build(); + + JwtSource jwtSource = JwtSource.newSource(jwtSourceOptions); +``` + +### Configure a timeout for JwtSource initialization + +The method `JwtSource newSource()` blocks until the JWT materials are fetched. The fetching process is retried +using an exponential backoff policy with this progression of delays between retries: 1 second, 2 seconds, 4, 8, 16, 32, 60, 60, 60... +It retries indefinitely unless a timeout is configured. + +This timeout can be configured either providing it through the `newSource(Duration timeout)` method or +using a System property: + +`spiffe.newJwtSource.timeout=PT30S` + +The `timout` duration is expressed in `ISO-8601` format. -By default, it is `availableProcessors * 2`. diff --git a/java-spiffe-core/src/main/java/spiffe/bundle/jwtbundle/JwtBundle.java b/java-spiffe-core/src/main/java/spiffe/bundle/jwtbundle/JwtBundle.java index 51fd1cd..27ff634 100644 --- a/java-spiffe-core/src/main/java/spiffe/bundle/jwtbundle/JwtBundle.java +++ b/java-spiffe-core/src/main/java/spiffe/bundle/jwtbundle/JwtBundle.java @@ -21,6 +21,7 @@ import java.nio.file.Path; import java.security.KeyException; import java.security.PublicKey; import java.text.ParseException; +import java.util.Collections; import java.util.HashMap; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; @@ -57,7 +58,7 @@ public class JwtBundle implements BundleSource { } /** - * Loads a bundle from a file on disk. The file must contain a standard RFC 7517 JWKS document. + * Loads a JWT bundle from a file on disk. The file must contain a standard RFC 7517 JWKS document. *

* Key Types supported are EC and RSA. * @@ -77,7 +78,7 @@ public class JwtBundle implements BundleSource { } /** - * Parses a bundle from a byte array. + * Parses a JWT bundle from a byte array. * * @param trustDomain a {@link TrustDomain} * @param bundleBytes an array of bytes representing the JWT bundle. @@ -114,7 +115,7 @@ public class JwtBundle implements BundleSource { * Returns the JWT authorities in the bundle, keyed by key ID. */ public Map getJwtAuthorities() { - return new HashMap<>(jwtAuthorities); + return Collections.unmodifiableMap(jwtAuthorities); } /** @@ -146,7 +147,7 @@ public class JwtBundle implements BundleSource { * @param keyId Key ID to associate to the jwtAuthority * @param jwtAuthority a PublicKey */ - public void addJwtAuthority(@NonNull String keyId, @NonNull PublicKey jwtAuthority) { + public void putJwtAuthority(@NonNull String keyId, @NonNull PublicKey jwtAuthority) { if (StringUtils.isBlank(keyId)) { throw new IllegalArgumentException("KeyId cannot be empty"); } diff --git a/java-spiffe-core/src/main/java/spiffe/bundle/x509bundle/X509Bundle.java b/java-spiffe-core/src/main/java/spiffe/bundle/x509bundle/X509Bundle.java index 843284b..2a85080 100644 --- a/java-spiffe-core/src/main/java/spiffe/bundle/x509bundle/X509Bundle.java +++ b/java-spiffe-core/src/main/java/spiffe/bundle/x509bundle/X509Bundle.java @@ -14,6 +14,7 @@ import java.nio.file.NoSuchFileException; import java.nio.file.Path; import java.security.cert.CertificateException; import java.security.cert.X509Certificate; +import java.util.Collections; import java.util.HashSet; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; @@ -38,7 +39,7 @@ public class X509Bundle implements BundleSource { } /** - * Creates a new JWT bundle for a trust domain with X.509 Authorities. + * Creates a new X.509 bundle for a trust domain with X.509 Authorities. * * @param trustDomain a {@link TrustDomain} to associate to the JwtBundle * @param x509Authorities a Map of X.509 Certificates @@ -50,7 +51,7 @@ public class X509Bundle implements BundleSource { } /** - * Loads a X.509 bundle from a file on disk. + * Loads an X.509 bundle from a file on disk. * * @param trustDomain a {@link TrustDomain} to associate to the bundle * @param bundlePath a path to the file that has the X.509 authorities @@ -73,7 +74,7 @@ public class X509Bundle implements BundleSource { } /** - * Parses a X095 bundle from an array of bytes. + * Parses a X.509 bundle from an array of bytes. * * @param trustDomain a {@link TrustDomain} to associate to the X.509 bundle * @param bundleBytes an array of bytes that represents the X.509 authorities @@ -109,7 +110,7 @@ public class X509Bundle implements BundleSource { * Returns the X.509 x509Authorities in the bundle. */ public Set getX509Authorities() { - return new HashSet<>(x509Authorities); + return Collections.unmodifiableSet(x509Authorities); } /** diff --git a/java-spiffe-core/src/main/java/spiffe/bundle/x509bundle/X509BundleSet.java b/java-spiffe-core/src/main/java/spiffe/bundle/x509bundle/X509BundleSet.java index 791dbcb..9634c8b 100644 --- a/java-spiffe-core/src/main/java/spiffe/bundle/x509bundle/X509BundleSet.java +++ b/java-spiffe-core/src/main/java/spiffe/bundle/x509bundle/X509BundleSet.java @@ -7,6 +7,7 @@ import spiffe.bundle.BundleSource; import spiffe.exception.BundleNotFoundException; import spiffe.spiffeid.TrustDomain; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -39,12 +40,12 @@ public class X509BundleSet implements BundleSource { } /** - * Adds a bundle to this Set, if the trust domain already exists, + * Adds an X.509 bundle to this Set, if the trust domain already exists, * replaces the bundle. * * @param x509Bundle a {@link X509Bundle} */ - public void add(@NonNull X509Bundle x509Bundle){ + public void put(@NonNull X509Bundle x509Bundle){ bundles.put(x509Bundle.getTrustDomain(), x509Bundle); } @@ -64,7 +65,10 @@ public class X509BundleSet implements BundleSource { return bundle; } + /** + * Returns the X.509 bundles of this X.509 Bundle Set. + */ public Map getBundles() { - return new HashMap<>(bundles); + return Collections.unmodifiableMap(bundles); } } diff --git a/java-spiffe-core/src/main/java/spiffe/workloadapi/JwtSource.java b/java-spiffe-core/src/main/java/spiffe/workloadapi/JwtSource.java index 8cfa592..7df270d 100644 --- a/java-spiffe-core/src/main/java/spiffe/workloadapi/JwtSource.java +++ b/java-spiffe-core/src/main/java/spiffe/workloadapi/JwtSource.java @@ -29,7 +29,7 @@ import java.util.logging.Level; import static spiffe.workloadapi.internal.ThreadUtils.await; /** - * A JwtSource represents a source of SPIFFE JWT SVID and JWT bundles + * A JwtSource represents a source of SPIFFE JWT SVIDs and JWT bundles * maintained via the Workload API. */ @Log @@ -38,7 +38,7 @@ public class JwtSource implements JwtSvidSource, BundleSource, Closea private static final Duration DEFAULT_TIMEOUT; static { - DEFAULT_TIMEOUT = Duration.ofSeconds(Long.getLong("spiffe.newJwtSource.timeout", 0)); + DEFAULT_TIMEOUT = Duration.parse(System.getProperty("spiffe.newJwtSource.timeout", "PT0S")); } private JwtBundleSet bundles; @@ -131,7 +131,7 @@ public class JwtSource implements JwtSvidSource, BundleSource, Closea } /** - * Returns the JWT SVID handled by this source. + * Fetches a new JWT SVID from the Workload API for the given subject SPIFFE ID and audiences. * * @return a {@link JwtSvid} * @throws IllegalStateException if the source is closed diff --git a/java-spiffe-core/src/main/java/spiffe/workloadapi/X509Source.java b/java-spiffe-core/src/main/java/spiffe/workloadapi/X509Source.java index 5f2a84a..57f28ca 100644 --- a/java-spiffe-core/src/main/java/spiffe/workloadapi/X509Source.java +++ b/java-spiffe-core/src/main/java/spiffe/workloadapi/X509Source.java @@ -41,12 +41,12 @@ import static spiffe.workloadapi.internal.ThreadUtils.await; * after close has been called. */ @Log -public class X509Source implements X509SvidSource, BundleSource, Closeable { +public class X509Source implements X509SvidSource, BundleSource, Closeable { private static final Duration DEFAULT_TIMEOUT; static { - DEFAULT_TIMEOUT = Duration.ofSeconds(Long.getLong("spiffe.newX509Source.timeout", 0)); + DEFAULT_TIMEOUT = Duration.parse(System.getProperty("spiffe.newX509Source.timeout", "PT0S")); } private X509Svid svid; @@ -125,6 +125,8 @@ public class X509Source implements X509SvidSource, BundleSource, Closeable { * @throws X509SourceException if the source could not be initialized */ public static X509Source newSource(@NonNull X509SourceOptions options, @NonNull Duration timeout) throws SocketEndpointAddressException, X509SourceException { + + System.out.println("TIMEOUT: ***** " + timeout); if (options.workloadApiClient == null) { options.workloadApiClient = createClient(options); } diff --git a/java-spiffe-core/src/test/java/spiffe/bundle/jwtbundle/JwtBundleSetTest.java b/java-spiffe-core/src/test/java/spiffe/bundle/jwtbundle/JwtBundleSetTest.java index 5206a63..ab42274 100644 --- a/java-spiffe-core/src/test/java/spiffe/bundle/jwtbundle/JwtBundleSetTest.java +++ b/java-spiffe-core/src/test/java/spiffe/bundle/jwtbundle/JwtBundleSetTest.java @@ -114,7 +114,7 @@ class JwtBundleSetTest { JwtBundleSet bundleSet = JwtBundleSet.of(bundleList); JwtBundle jwtBundle2 = new JwtBundle(TrustDomain.of("example.org")); - jwtBundle2.addJwtAuthority("key1", new DummyPublicKey()); + jwtBundle2.putJwtAuthority("key1", new DummyPublicKey()); bundleSet.add(jwtBundle2); assertTrue(bundleSet.getBundles().containsValue(jwtBundle2)); diff --git a/java-spiffe-core/src/test/java/spiffe/bundle/jwtbundle/JwtBundleTest.java b/java-spiffe-core/src/test/java/spiffe/bundle/jwtbundle/JwtBundleTest.java index 9342f8b..f7cb2c9 100644 --- a/java-spiffe-core/src/test/java/spiffe/bundle/jwtbundle/JwtBundleTest.java +++ b/java-spiffe-core/src/test/java/spiffe/bundle/jwtbundle/JwtBundleTest.java @@ -287,8 +287,8 @@ class JwtBundleTest { // Test addJWTAuthority DummyPublicKey jwtAuthority1 = new DummyPublicKey(); DummyPublicKey jwtAuthority2 = new DummyPublicKey(); - jwtBundle.addJwtAuthority("key1", jwtAuthority1); - jwtBundle.addJwtAuthority("key2", jwtAuthority2); + jwtBundle.putJwtAuthority("key1", jwtAuthority1); + jwtBundle.putJwtAuthority("key2", jwtAuthority2); assertEquals(2, jwtBundle.getJwtAuthorities().size()); @@ -310,7 +310,7 @@ class JwtBundleTest { assertTrue(jwtBundle.hasJwtAuthority("key2")); // Test update - jwtBundle.addJwtAuthority("key2", jwtAuthority1); + jwtBundle.putJwtAuthority("key2", jwtAuthority1); assertEquals(jwtAuthority1, jwtBundle.getJwtAuthorities().get("key2")); assertEquals(1, jwtBundle.getJwtAuthorities().size()); } @@ -319,7 +319,7 @@ class JwtBundleTest { void testAddJwtAuthority_emtpyKeyId_throwsIllegalArgumentException() { JwtBundle jwtBundle = new JwtBundle(TrustDomain.of("example.org")); try { - jwtBundle.addJwtAuthority("", new DummyPublicKey()); + jwtBundle.putJwtAuthority("", new DummyPublicKey()); } catch (IllegalArgumentException e) { assertEquals("KeyId cannot be empty", e.getMessage()); } @@ -329,7 +329,7 @@ class JwtBundleTest { void testAddJwtAuthority_nullKeyId_throwsNullPointerException() { JwtBundle jwtBundle = new JwtBundle(TrustDomain.of("example.org")); try { - jwtBundle.addJwtAuthority(null, new DummyPublicKey()); + jwtBundle.putJwtAuthority(null, new DummyPublicKey()); } catch (NullPointerException e) { assertEquals("keyId is marked non-null but is null", e.getMessage()); } @@ -339,7 +339,7 @@ class JwtBundleTest { void testAddJwtAuthority_nullJwtAuthority_throwsNullPointerException() { JwtBundle jwtBundle = new JwtBundle(TrustDomain.of("example.org")); try { - jwtBundle.addJwtAuthority("key1", null); + jwtBundle.putJwtAuthority("key1", null); } catch (NullPointerException e) { assertEquals("jwtAuthority is marked non-null but is null", e.getMessage()); } diff --git a/java-spiffe-core/src/test/java/spiffe/bundle/x509bundle/X509BundleSetTest.java b/java-spiffe-core/src/test/java/spiffe/bundle/x509bundle/X509BundleSetTest.java index 3b6e037..599b9b3 100644 --- a/java-spiffe-core/src/test/java/spiffe/bundle/x509bundle/X509BundleSetTest.java +++ b/java-spiffe-core/src/test/java/spiffe/bundle/x509bundle/X509BundleSetTest.java @@ -41,7 +41,7 @@ class X509BundleSetTest { X509BundleSet bundleSet = X509BundleSet.of(bundleList); X509Bundle x509Bundle2 = new X509Bundle(TrustDomain.of("other.org")); - bundleSet.add(x509Bundle2); + bundleSet.put(x509Bundle2); assertTrue(bundleSet.getBundles().containsValue(x509Bundle1)); assertTrue(bundleSet.getBundles().containsValue(x509Bundle2)); @@ -53,7 +53,7 @@ class X509BundleSetTest { List bundleList = Collections.singletonList(x509Bundle1); X509BundleSet bundleSet = X509BundleSet.of(bundleList); - bundleSet.add(x509Bundle1); + bundleSet.put(x509Bundle1); assertTrue(bundleSet.getBundles().containsValue(x509Bundle1)); assertEquals(1, bundleSet.getBundles().size()); @@ -67,7 +67,7 @@ class X509BundleSetTest { X509Bundle x509Bundle2 = new X509Bundle(TrustDomain.of("example.org")); x509Bundle2.addX509Authority(new DummyX509Certificate()); - bundleSet.add(x509Bundle2); + bundleSet.put(x509Bundle2); assertTrue(bundleSet.getBundles().containsValue(x509Bundle2)); assertFalse(bundleSet.getBundles().containsValue(x509Bundle1)); @@ -81,7 +81,7 @@ class X509BundleSetTest { X509BundleSet bundleSet = X509BundleSet.of(bundleList); try { - bundleSet.add(null); + bundleSet.put(null); fail("should have thrown exception"); } catch (NullPointerException e) { assertEquals("x509Bundle is marked non-null but is null", e.getMessage()); diff --git a/java-spiffe-core/src/test/java/spiffe/svid/jwtsvid/JwtSvidParseAndValidateTest.java b/java-spiffe-core/src/test/java/spiffe/svid/jwtsvid/JwtSvidParseAndValidateTest.java index cbc4870..615ebfb 100644 --- a/java-spiffe-core/src/test/java/spiffe/svid/jwtsvid/JwtSvidParseAndValidateTest.java +++ b/java-spiffe-core/src/test/java/spiffe/svid/jwtsvid/JwtSvidParseAndValidateTest.java @@ -106,9 +106,9 @@ class JwtSvidParseAndValidateTest { TrustDomain trustDomain = TrustDomain.of("test.domain"); JwtBundle jwtBundle = new JwtBundle(trustDomain); - jwtBundle.addJwtAuthority("authority1", key1.getPublic()); - jwtBundle.addJwtAuthority("authority2", key2.getPublic()); - jwtBundle.addJwtAuthority("authority3", key3.getPublic()); + jwtBundle.putJwtAuthority("authority1", key1.getPublic()); + jwtBundle.putJwtAuthority("authority2", key2.getPublic()); + jwtBundle.putJwtAuthority("authority3", key3.getPublic()); SpiffeId spiffeId = trustDomain.newSpiffeId("host"); Date expiration = new Date(System.currentTimeMillis() + 3600000); diff --git a/java-spiffe-core/src/test/java/spiffe/svid/jwtsvid/JwtSvidParseInsecureTest.java b/java-spiffe-core/src/test/java/spiffe/svid/jwtsvid/JwtSvidParseInsecureTest.java index 092f6b4..5743276 100644 --- a/java-spiffe-core/src/test/java/spiffe/svid/jwtsvid/JwtSvidParseInsecureTest.java +++ b/java-spiffe-core/src/test/java/spiffe/svid/jwtsvid/JwtSvidParseInsecureTest.java @@ -88,8 +88,8 @@ class JwtSvidParseInsecureTest { TrustDomain trustDomain = TrustDomain.of("test.domain"); JwtBundle jwtBundle = new JwtBundle(trustDomain); - jwtBundle.addJwtAuthority("authority1", key1.getPublic()); - jwtBundle.addJwtAuthority("authority2", key2.getPublic()); + jwtBundle.putJwtAuthority("authority1", key1.getPublic()); + jwtBundle.putJwtAuthority("authority2", key2.getPublic()); SpiffeId spiffeId = trustDomain.newSpiffeId("host"); Date expiration = new Date(System.currentTimeMillis() + 3600000);