Skip to content

Commit 63b49af

Browse files
mkleenegemini-code-assist[bot]claudecoderabbitai[bot]
authored
feat(sdk)!: remove bouncycastle and ayza libraries (#367)
This pull makes this library provider-agnostic. Users can control how cryptography is provided by configuring their `java.security` configuration, as we do in the FIPS-profile tests. We add two new maven profiles `fips` and `non-fips`. These profiles are used solely for testing; the library itself is agnostic to which providers are used for cryptography. In the `non-fips` profile we use the normal providers configured on the JVM. The BouncyCastle provider is included just for some test verification. In the `fips` profile we include the BouncyCastle FIPS providers we need at runtime to verify that our library works with a FIPS-compliant setup. ## Ayza removal I believe it's necessary to remove ayza since it loads a BouncyCastleProvider explicitly. Since the FIPS provider occupies the same namespace loading both providers can't work. ## BouncyCastle moved to test-only Removing BouncyCastle as a compile dependency isn't strictly necessary but it results in much less rework and eliminates a transitive dependency for consumers. ## Remaining FIPS work We still need to: * verify that the JWK libraries we use are FIPS compliant with the correct providers to claim that this library can be made FIPS compliant * verify that we generate random numbers in a FIPS-compliant way. Because BouncyCastle does not provide a FIPS-compliant source of entropy we need to include the SUN provider to get at `/dev/rand`. This is OK, since * we _should_ pick up this provider and use it for all cryptographic operations (but we may want to provide ways to guarantee that a particular provider is used) * the FIPS boundary for random numbers starts at the provider. We don't have tools to verify this now, though The breaking API changes are as follows: | Area | Change | Related to | |---|---|---| | `SDKBuilder.sslFactory(SSLFactory)` | **Removed.** Replaced by `sslFactory(SSLSocketFactory)` and `sslFactory(SSLSocketFactory, X509TrustManager)` | Ayza removal | | `SDKBuilder.sslFactoryFromTrustManager(X509TrustManager)` | **New** method | Ayza removal | | `ECKeyPair(ECCurve, ECAlgorithm)` constructor | **Removed.** Replaced by `ECKeyPair(ECCurve)` | BouncyCastle removal | | `ECKeyPair.publicKeyFromPem` | Removed method from public interface since it should not have been exposed and since the types were changing anyway | BouncyCastle removal | | `ECKeyPair.privateKeyFromPem`, `getPEMPublicKeyFromX509Cert`, `publicKeyFromECPoint`, static `compressECPublickey(String)` | **Removed** from public API (moved to test-only `PemTestUtils`) | BouncyCastle removal | | Dependencies | Drops `io.github.hakky54:ayza*` and the runtime BC dep; adds `bc-fips`, `bcpkix-fips`, `bctls-fips` (via new `fips`/`non-fips` Maven profiles) | Both | <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * FIPS-profile cryptography support and improved TLS trust-material builder (directory/keystore/trust-manager sources). * **Improvements** * Migrated crypto to standard Java security APIs for broader compatibility. * Added AES-GCM key-generation helper and more robust TLS/trust-manager integration across the SDK. * **Tests** * Updated and added tests and test utilities for PEM/EC key handling and TLS/trust behaviors. * **Chores** * Updated build/test config and pinned Bouncy Castle FIPS dependencies. <!-- review_stack_entry_start --> [![Review Change Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/opentdf/java-sdk/pull/367?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
1 parent 9991b07 commit 63b49af

18 files changed

Lines changed: 984 additions & 369 deletions

File tree

.github/workflows/checks.yaml

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -85,13 +85,23 @@ jobs:
8585
path: ~/.sonar/cache
8686
key: ${{ runner.os }}-sonar
8787
restore-keys: ${{ runner.os }}-sonar
88-
- name: Maven Test Coverage
88+
- name: Generate sources
8989
env:
90-
SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
9190
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
9291
BUF_INPUT_HTTPS_USERNAME: opentdf-bot
9392
BUF_INPUT_HTTPS_PASSWORD: ${{ secrets.PERSONAL_ACCESS_TOKEN_OPENTDF }}
94-
run: mvn --batch-mode clean verify org.sonarsource.scanner.maven:sonar-maven-plugin:sonar -Dsonar.projectKey=opentdf_java-sdk -P coverage
93+
run: mvn clean --batch-mode clean generate-sources
94+
- name: Tests and enforcer (fips)
95+
run: mvn --batch-mode test enforcer:enforce -P 'fips,!non-fips' -Dmaven.antrun.skip
96+
- name: Tests with coverage and javadoc (non-fips)
97+
env:
98+
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
99+
SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
100+
run: |
101+
mvn --batch-mode verify \
102+
org.sonarsource.scanner.maven:sonar-maven-plugin:sonar \
103+
-Dmaven.antrun.skip -Dsonar.projectKey=opentdf_java-sdk \
104+
-P 'coverage,non-fips,!fips'
95105
96106
platform-integration:
97107
runs-on: ubuntu-22.04

adr/0001/remove-bc-and-ayza.md

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
# Removing compile dependencies on BouncyCastle and Ayza libraries
2+
3+
## Decision
4+
Remove compile dependencies on BouncyCastle and Ayza libraries.
5+
6+
## Context
7+
### Ayza
8+
FIPS compliance is the main force driving the decision to remove Ayza.
9+
Ayza loads a non-FIPS BouncyCastle provider, which means that the inclusion of
10+
the BC FIPS provider will result in errors when classes from the wrong JAR are loaded.
11+
12+
### BouncyCastle
13+
Removing BouncyCastle is driven by the fact that BouncyCastle implements similar
14+
but often not identical classes in the FIPS and non-FIPS jars; removing any
15+
explicit dependency on BouncyCastle classes means that we are not exposed to
16+
changes that may take place as the FIPS and non-FIPS interfaces change.
17+
18+
## Consequences
19+
### Positive
20+
* better modularity around our usage of cryptographic libraries
21+
* makes it easier to implement FIPS compliance
22+
* consumers of the SDK have fewer dependencies to manage
23+
* easier testing and maintenance because we are not exposed to changes in the BouncyCastle API
24+
### Negative
25+
* possible increased burden to implement operations in terms of the JCA as opposed to using BouncyCastle-specific APIs

cmdline/src/main/java/io/opentdf/platform/Command.java

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import com.google.gson.GsonBuilder;
1111
import com.google.gson.reflect.TypeToken;
1212

13+
import java.security.cert.X509Certificate;
1314
import java.text.ParseException;
1415
import com.google.gson.JsonSyntaxException;
1516
import io.opentdf.platform.sdk.AssertionConfig;
@@ -18,11 +19,11 @@
1819
import io.opentdf.platform.sdk.KeyType;
1920
import io.opentdf.platform.sdk.SDK;
2021
import io.opentdf.platform.sdk.SDKBuilder;
21-
import nl.altindag.ssl.SSLFactory;
2222
import picocli.CommandLine;
2323
import picocli.CommandLine.HelpCommand;
2424
import picocli.CommandLine.Option;
2525

26+
import javax.net.ssl.X509TrustManager;
2627
import java.io.BufferedInputStream;
2728
import java.io.BufferedOutputStream;
2829
import java.io.File;
@@ -262,10 +263,7 @@ void encrypt(
262263
private SDK buildSDK() {
263264
SDKBuilder builder = new SDKBuilder();
264265
if (insecure) {
265-
SSLFactory sslFactory = SSLFactory.builder()
266-
.withUnsafeTrustMaterial() // Trust all certificates
267-
.build();
268-
builder.sslFactory(sslFactory);
266+
builder.insecureSslFactory();
269267
}
270268

271269
return builder.platformEndpoint(platformEndpoint)

pom.xml

Lines changed: 23 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,9 @@
1717
<grpc.version>1.75.0</grpc.version>
1818
<protobuf.version>4.29.2</protobuf.version>
1919
<bouncycastle.version>1.82</bouncycastle.version>
20-
<ayza.version>10.0.0</ayza.version>
20+
<bc-fips.version>2.1.2</bc-fips.version>
21+
<bcpkix-fips.version>2.1.11</bcpkix-fips.version>
22+
<bctls-fips.version>2.1.23</bctls-fips.version>
2123
<bytebuddy.version>1.18.3</bytebuddy.version>
2224
<!-- JaCoCo Properties -->
2325
<jacoco.version>0.8.13</jacoco.version>
@@ -78,39 +80,6 @@
7880
<version>3.4</version>
7981
<scope>provided</scope>
8082
</dependency>
81-
<dependency>
82-
<groupId>io.github.hakky54</groupId>
83-
<artifactId>ayza-for-pem</artifactId>
84-
<version>${ayza.version}</version>
85-
<exclusions>
86-
<exclusion>
87-
<groupId>org.slf4j</groupId>
88-
<artifactId>slf4j-api</artifactId>
89-
</exclusion>
90-
</exclusions>
91-
</dependency>
92-
<dependency>
93-
<groupId>io.github.hakky54</groupId>
94-
<artifactId>ayza</artifactId>
95-
<version>${ayza.version}</version>
96-
<exclusions>
97-
<exclusion>
98-
<groupId>org.slf4j</groupId>
99-
<artifactId>slf4j-api</artifactId>
100-
</exclusion>
101-
</exclusions>
102-
</dependency>
103-
<dependency>
104-
<groupId>io.github.hakky54</groupId>
105-
<artifactId>ayza-for-netty</artifactId>
106-
<version>${ayza.version}</version>
107-
<exclusions>
108-
<exclusion>
109-
<groupId>org.slf4j</groupId>
110-
<artifactId>slf4j-api</artifactId>
111-
</exclusion>
112-
</exclusions>
113-
</dependency>
11483
<dependency>
11584
<groupId>io.grpc</groupId>
11685
<artifactId>grpc-netty-shaded</artifactId>
@@ -157,6 +126,26 @@
157126
<artifactId>bcprov-jdk18on</artifactId>
158127
<version>${bouncycastle.version}</version>
159128
</dependency>
129+
<dependency>
130+
<groupId>org.bouncycastle</groupId>
131+
<artifactId>bctls-jdk18on</artifactId>
132+
<version>${bouncycastle.version}</version>
133+
</dependency>
134+
<dependency>
135+
<groupId>org.bouncycastle</groupId>
136+
<artifactId>bc-fips</artifactId>
137+
<version>${bc-fips.version}</version>
138+
</dependency>
139+
<dependency>
140+
<groupId>org.bouncycastle</groupId>
141+
<artifactId>bcpkix-fips</artifactId>
142+
<version>${bcpkix-fips.version}</version>
143+
</dependency>
144+
<dependency>
145+
<groupId>org.bouncycastle</groupId>
146+
<artifactId>bctls-fips</artifactId>
147+
<version>${bctls-fips.version}</version>
148+
</dependency>
160149
<!--
161150
Pin Byte Buddy for test-time Mockito instrumentation on newer JVMs (e.g. Java 21).
162151
This does NOT add a runtime dependency; it only manages the version used by modules.

sdk/pom.xml

Lines changed: 52 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@
1717
<connect.version>0.7.2</connect.version>
1818
<okhttp.version>4.12.0</okhttp.version>
1919
<platform.branch>protocol/go/v0.16.0</platform.branch>
20+
<!-- in the non-FIPS case we don't need to pass anything to the jvm -->
21+
<java.security.properties.test></java.security.properties.test>
22+
<!-- Default empty argLine; overridden by jacoco:prepare-agent when the `coverage` profile is active -->
23+
<argLine></argLine>
2024
</properties>
2125
<dependencies>
2226
<!-- Logging Dependencies -->
@@ -31,18 +35,6 @@
3135
<artifactId>oauth2-oidc-sdk</artifactId>
3236
<version>11.10.1</version>
3337
</dependency>
34-
<dependency>
35-
<groupId>io.github.hakky54</groupId>
36-
<artifactId>ayza-for-pem</artifactId>
37-
</dependency>
38-
<dependency>
39-
<groupId>io.github.hakky54</groupId>
40-
<artifactId>ayza</artifactId>
41-
</dependency>
42-
<dependency>
43-
<groupId>io.github.hakky54</groupId>
44-
<artifactId>ayza-for-netty</artifactId>
45-
</dependency>
4638
<!-- Serialization and Deserialization Dependencies -->
4739
<dependency>
4840
<groupId>com.google.code.gson</groupId>
@@ -160,15 +152,7 @@
160152
<version>6.0.53</version>
161153
<scope>provided</scope>
162154
</dependency>
163-
<!-- Crypto Dependencies -->
164-
<dependency>
165-
<groupId>org.bouncycastle</groupId>
166-
<artifactId>bcpkix-jdk18on</artifactId>
167-
</dependency>
168-
<dependency>
169-
<groupId>org.bouncycastle</groupId>
170-
<artifactId>bcprov-jdk18on</artifactId>
171-
</dependency>
155+
<!-- Crypto Dependencies are pulled in via the `non-fips` (default) or `fips` profile -->
172156
<!-- Testing Dependencies -->
173157
<dependency>
174158
<groupId>org.junit.jupiter</groupId>
@@ -483,11 +467,56 @@
483467
</execution>
484468
</executions>
485469
</plugin>
470+
<plugin>
471+
<groupId>org.apache.maven.plugins</groupId>
472+
<artifactId>maven-surefire-plugin</artifactId>
473+
<configuration>
474+
<argLine>@{argLine} ${java.security.properties.test}</argLine>
475+
</configuration>
476+
</plugin>
486477
</plugins>
487478
</build>
488-
<!--profile
489-
to execute fuzz test -->
490479
<profiles>
480+
<profile>
481+
<id>non-fips</id>
482+
<activation>
483+
<activeByDefault>true</activeByDefault>
484+
</activation>
485+
<dependencies>
486+
<dependency>
487+
<groupId>org.bouncycastle</groupId>
488+
<artifactId>bcpkix-jdk18on</artifactId>
489+
<scope>test</scope>
490+
</dependency>
491+
</dependencies>
492+
</profile>
493+
<profile>
494+
<id>fips</id>
495+
<activation>
496+
<activeByDefault>false</activeByDefault>
497+
</activation>
498+
<properties>
499+
<java.security.properties.test>-Djava.security.properties=${project.basedir}/src/test/resources/java.security.fips.test</java.security.properties.test>
500+
</properties>
501+
<dependencies>
502+
<dependency>
503+
<groupId>org.bouncycastle</groupId>
504+
<artifactId>bc-fips</artifactId>
505+
<scope>runtime</scope>
506+
</dependency>
507+
<dependency>
508+
<groupId>org.bouncycastle</groupId>
509+
<artifactId>bctls-fips</artifactId>
510+
<scope>runtime</scope>
511+
</dependency>
512+
<dependency>
513+
<groupId>org.bouncycastle</groupId>
514+
<artifactId>bcpkix-fips</artifactId>
515+
<scope>test</scope>
516+
</dependency>
517+
</dependencies>
518+
</profile>
519+
<!-- profile to execute fuzz test -->
491520
<profile>
492521
<id>fuzz</id>
493522
<activation>

sdk/src/main/java/io/opentdf/platform/sdk/AesGcm.java

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import javax.crypto.BadPaddingException;
44
import javax.crypto.Cipher;
55
import javax.crypto.IllegalBlockSizeException;
6+
import javax.crypto.KeyGenerator;
67
import javax.crypto.NoSuchPaddingException;
78
import javax.crypto.SecretKey;
89
import javax.crypto.spec.GCMParameterSpec;
@@ -20,10 +21,27 @@
2021
public class AesGcm {
2122
public static final int GCM_NONCE_LENGTH = 12; // in bytes
2223
public static final int GCM_TAG_LENGTH = 16; // in bytes
24+
public static final int GCM_KEY_SIZE_BITS = 256;
25+
private static final String KEY_ALGORITHM = "AES";
2326
private static final String CIPHER_TRANSFORM = "AES/GCM/NoPadding";
2427

2528
private final SecretKey key;
2629

30+
/**
31+
* <p>Generate a fresh 256-bit AES key using the JCA {@link KeyGenerator}.</p>
32+
*
33+
* @return the encoded key bytes
34+
*/
35+
static byte[] generateKey() {
36+
try {
37+
KeyGenerator keyGenerator = KeyGenerator.getInstance(KEY_ALGORITHM);
38+
keyGenerator.init(GCM_KEY_SIZE_BITS);
39+
return keyGenerator.generateKey().getEncoded();
40+
} catch (NoSuchAlgorithmException e) {
41+
throw new SDKException("error generating AES key", e);
42+
}
43+
}
44+
2745

2846
/**
2947
* <p>Return symmetric key</p>

0 commit comments

Comments
 (0)