Add support for FIPS edition of BouncyCastle

[buleuszmatak]

Problem

I want to use NATS Auth Callout in an application that has to use a FIPS-certified cryptographic module, particularly the FIPS-certified edition of BouncyCastle in approved-only mode.
To use the FIPS-certified edition, I need to first ensure that the normal edition is not on the classpath, i.e. exclude it from any transitive dependencies including from nkeys-java and jnats.
After doing that and adding the FIPS edition, the nkeys-java library fails at runtime, because it is using BouncyCastle APIs specific to the normal edition.

Example

https://github.com/nats-io/java-nats-examples/pull/18/files

It uses version of nkeys-java from this PR.
Build this PR locally to see the working version or revert io.nats:nkeys-java to 2.1.1 to get the error mentioned above.

Proposed solution

Use APIs that are common across the normal and the FIPS-certified editions and add a system property io.nats.nkey.security.provider allowing users to switch to a different java.security.Provider.

• No changes to system properties will maintain current behavior and attempt to use the normal edition of BouncyCastle.
• -Dio.nats.nkey.security.provider will cause nkeys-java to use the provider configured in the JVM.
• io.nats.nkey.security.provider=BC or io.nats.nkey.security.provider=BCFIPS will cause nkeys-java to use the configured provider.

Java 15 added support for Ed25519, but I found no way to derive public key from private key, that would also work with BCFIPS in approved-only mode, assuming no security provider specific code.
That's why the only providers that can be configured are BC and BCFIPS.

Perhaps if/when NATS Java libraries raise the requirement to Java 15+, the property could be dropped and the default provider could be switched to the one configured in the JVM, which would allow to drop the BouncyCastle dependency. That would of course require the solution to the public key derivation. On Java 15+ itself the derivation can be done by using the FixedSecureRandom pattern, but that throws on the FIPS-edition in approved-only mode.

[scottf]

So your changes here are probably fine, but I think that the changes should be part of the java client as well. The idea of the nkeys library was to have it ready for the eventuality that I can break up the jnats client into pieces, which is only going to happen with a major version. So for now we need to keep the libraries in sync.

[scottf]

Suggested change

	}
	private static @Nullable Provider getSecurityProvider() {
	String property = System.getProperty("io.nats.nkey.security.provider");
	if (property == null || property.isEmpty()) {
	// Instantiating the BouncyCastle provider to maintain backwards compatibility
	return DefaultSecurityProviderFactory.getProvider();
	}
	if (property.isEmpty()) {
	// Use whatever is configured as the default in the JVM
	return null;
	}
	return Security.getProvider(property);
	}

[buleuszmatak]

Flattened the ifs (no else statements), but kept the conditions.
null - system property not specified - create BouncyCastle provider for backwards compatibility
empty - property with empty value - use whatever provider is configured in the JVM
other - property with non-empty value - use the provider with the specified name

[scottf]

Can we make io.nats.nkey.security.provider a public constant, maybe in NKeyConstants?

[buleuszmatak]

Done. Added SECURITY_PROVIDER_PROPERTY to NKeyConstants

[scottf]

1. Easy, put the license in each file.

2. I apologize for my ignorance about this library and I can't just approve stuff until I understand it better. So please help educate me with a little more explanation of the KeyCodec, specifically an explanation of what the key prefixes are. Please add some references to public documents that match the values that you chose.

3. Could you please figure out how to make verified commits. Start here: https://docs.github.com/en/authentication/managing-commit-signature-verification
   Verified commits are a CNCF policy. Often it's waived for initial commits, but I would like to finish this and the other PR verified.

Thanks for bearing with me on this process.

[buleuszmatak]

So your changes here are probably fine, but I think that the changes should be part of the java client as well. The idea of the nkeys library was to have it ready for the eventuality that I can break up the jnats client into pieces, which is only going to happen with a major version. So for now we need to keep the libraries in sync.

Will create a PR in jnats once everything here is ready to be merged.

This PR adds a system property to switch between security providers. Its current name is io.nats.nkey.security.provider to match package name in this repo.
Should the property be named io.nats.client.security.provider in jnats? Or should it have the same name for forward compatibility? Perhaps a different name?

[buleuszmatak]

1. Easy, put the license in each file.
2. I apologize for my ignorance about this library and I can't just approve stuff until I understand it better. So please help educate me with a little more explanation of the KeyCodec, specifically an explanation of what the key prefixes are. Please add some references to public documents that match the values that you chose.
3. Could you please figure out how to make verified commits. Start here: https://docs.github.com/en/authentication/managing-commit-signature-verification
   Verified commits are a CNCF policy. Often it's waived for initial commits, but I would like to finish this and the other PR verified.

Thanks for bearing with me on this process.

1. Done

Implementation of io.nats.nkey.KeyWrapper and its subclasses before this PR is incorrect. First - PKCS#8 is only for private keys. Second - java.security.Key#getEncoded is expected to return encoded key in the format returned by java.security.Key#getFormat, but the implementation returns keys in raw format.

I've added assertions to io.nats.nkey.NKeyTests#testInterop that fail with the previous implementation. Feel free to adapt this in a separate PR if you want to fast track the bug fix.

nkeys.java/src/test/java/io/nats/nkey/NKeyTests.java

Lines 561 to 570 in 8139a18

	Signature fromSeedJcaSignature = Signature.getInstance("Ed25519", new BouncyCastleProvider());
	fromSeedJcaSignature.initSign(fromSeed.getKeyPair().getPrivate());
	fromSeedJcaSignature.update(data);
	byte[] fromSeedJcaSignatureBytes = fromSeedJcaSignature.sign();
	assertArrayEquals(fromSeedJcaSignatureBytes, sig);
	Signature fromPublicKeyJcaSignature = Signature.getInstance("Ed25519", new BouncyCastleProvider());
	fromPublicKeyJcaSignature.initVerify(fromSeed.getKeyPair().getPublic());
	fromPublicKeyJcaSignature.update(data);
	assertTrue(fromPublicKeyJcaSignature.verify(sig));

X.509 is the standard format of java.security.PublicKey#getFormat. PKCS#8 is the standard format of java.security.PrivateKey#getFormat
Both use DER serialization format (think JSON).
PEM files are header + base64 encoded DER + footer

io.nats.nkey.KeyCodec#pubBytesToKeySpec and io.nats.nkey.KeyCodec#seedBytesToKeySpec essentially perform an operation that, if with JSON, would look like this '{"algorithm":"Ed25519","data":"' + seedBytes/pubBytes + '"}'.
With JSON you would have to ensure that the variables don't contain special characters,
but io.nats.nkey.KeyCodec#PUBLIC_KEY_PREFIX and io.nats.nkey.KeyCodec#PRIVATE_KEY_PREFIX specify remaining length of the encoded payload and all that's left is to append key bytes.

Comments above the constants describe what each byte means

nkeys.java/src/main/java/io/nats/nkey/KeyCodec.java

Lines 22 to 57 in 8139a18

	// This value can be obtained on Java 15+ with
	// KeyPairGenerator.getInstance("Ed25519").generateKeyPair().getPrivate().getEncoded()
	// which returns this + private key bytes.
	// 48 - sequence tag
	// 46 - length
	// 2 - integer tag
	// 1 - length
	// 0 - version - PKCS#8v1
	// 48 - sequence tag
	// 5 - length
	// 6 - OID tag
	// 3 - length
	// 43 - 1st byte of Ed25519 OID - "1.3.101.112"
	// 101 - 2nd byte
	// 112 - 3rd byte
	// 4 - octet string tag
	// 34 - length
	// 4 - octet string tag
	// 32 - length
	private static final byte[] PRIVATE_KEY_PREFIX = new byte[]{48, 46, 2, 1, 0, 48, 5, 6, 3, 43, 101, 112, 4, 34, 4, 32};
	// This value can be obtained on Java 15+ with
	// KeyPairGenerator.getInstance("Ed25519").generateKeyPair().getPublic().getEncoded()
	// which returns this + public key bytes.
	// 48 - sequence tag
	// 42 - length
	// 48 - sequence tag
	// 5 - length
	// 6 - OID tag
	// 3 - length
	// 43 - 1st byte of Ed25519 OID - "1.3.101.112"
	// 101 - 2nd byte
	// 112 - 3rd byte
	// 3 - bit string tag
	// 33 - length
	// 0 - number of unused bits in final byte
	private static final byte[] PUBLIC_KEY_PREFIX = new byte[]{48, 42, 48, 5, 6, 3, 43, 101, 112, 3, 33, 0};

As for references, here is BouncyCastle doing essentially the same thing for public key encoding
https://github.com/bcgit/bc-java/blob/bdc97032f09c95c1d78ad71e19b6cc2ae712fa99/prov/src/main/java/org/bouncycastle/jcajce/provider/asymmetric/edec/KeyFactorySpi.java#L46

They don't use that strategy for private key, but the idea is the same.

As long as private/public key is valid, encoded key returned from io.nats.nkey.KeyCodec#pubBytesToKeySpec and io.nats.nkey.KeyCodec#seedBytesToKeySpec should be parsed fine by BouncyCastle (and others).

io.nats.nkey.KeyCodec#publicKeyToPubBytes however is different. I was a bit pragmatic (time-efficient) there. After noticing that Java 15+ and both BouncyCastle editions encode to the same prefix, I have asserted the prefix in the implementation.
It could start throwing in the future if BouncyCastle decided to change how it encodes public keys, and users updated BouncyCastle on their classpath to a newer version that NKey wasn't tested against.

If you want, I can explore implementation of io.nats.nkey.KeyCodec#publicKeyToPubBytes more. Ideally there would be no assumption about the public key prefix, and the implementation would parse it properly to get the pubKey bytes, but then the APIs that could facilitate that are most likely security provider specific.

3. Done - squashed commits before the comments into a signed commit. New commits are also signed.

---

Alternative strategy for security provider swappability

Move all crypto operations to a separate class, and allow users to specify (as a system property) that their implementation should be used instead. That would in some sense be java.security.Provider, but with extra steps, steps that are needed because Java Crypto API has no way to derive public key from private key (other than extending SecureRandom, which BCFIPS rejects) and get pubKey bytes from that public key (especially pre Java 15).

[scottf]

@buleuszmatak Hey - sorry this has got shelved for the holidays. I'm going to move this library to Java 21 or 25. This is preparation to upgrade the existing client to 21/25.
Since as you noted, "Java 15 added support for Ed25519" I would probably switch to using this and get rid of direct inclusion of BouncyCastle.

I can still merge this PR, but wanted to get your opinion on this.

[buleuszmatak]

I would probably switch to using this and get rid of direct inclusion of BouncyCastle.

I think that can be done, and it would make nats.java a much lighter dependency.

However, I don't think there is anything like org.bouncycastle.crypto.params.Ed25519PrivateKeyParameters#generatePublicKey in Java Crypto API, that would allow to cleanly derive public key from a private Ed25519 key. You will probably have to resort to some trickery like extending java.security.SecureRandom to return a fixed value, like BouncyCastle's FixedSecureRandom does, and then

var kpg = KeyPairGenerator.getInstance("Ed25519");
kpg.initialize(256, new FixedSecureRandom(seed));
var keyPair = kpg.generateKeyPair();
var publicKey = keyPair.getPublicKey();

If you care about compatibility with BCFIPS in approved mode then ...

if you write your FixedSecureRandom constructor like this

FixedSecureRandom(byte[] value) {
    super(null, null);
    this.value = value;
}

which means null SecureRandomSpi and null Provider, which yeah, this class would return a fixed value, it wouldn't delegate to anything else, then BCFIPS in approved only mode will reject this SecureRandom in org.bouncycastle.crypto.fips.Utils#validateRandom(java.security.SecureRandom, int, org.bouncycastle.crypto.fips.FipsAlgorithm, java.lang.String).
If you instead use super(), then the FixedSecureRandom will claim to have the default provider. If the default provider is BCFIPS, then BCFIPS won't reject the FixedSecureRandom.
This, I think, is not the indended use of SecureRandom and perhaps such trickery would impact FIPS compliance.
Not an expert in FIPS compliance though. Maybe that's the way to do it. Just sharing insight from working on this PR.

I can still merge this PR

Unless there is interest in this from anyone else but me, I wouldn't. My attempt to add FIPS compatibility is not for a hobby project, and because of the dynamic environment I'm working in, my need for this has been suspended.

Feel free to close.

[scottf]

@buleuszmatak I'm working on the "alternative strategy" idea. Basically pulling out all code that does not require the BC library into one project with an interface/factory/provider idea and then I'll make a second project implements using the standard BC, and make a third project that is specifically for the fips version of the second. This provides a way to ensure only the desired bc library is around and provides a template for anyone wanting something completely on their own.
Then hopefully I can plug this into the current client without a major version, else it will just go into the major update, which is coming, a full api cleanup and separation of functionality.

[scottf]

@buleuszmatak So this work is ready for a FIPS implementation. #18

[scottf]

Closing. Replaced with new individual implementations. FIPS implementation found in PR #26