Skip to content

FIPS Implementation#26

Open
scottf wants to merge 5 commits intomainfrom
fips-impl
Open

FIPS Implementation#26
scottf wants to merge 5 commits intomainfrom
fips-impl

Conversation

@scottf
Copy link
Collaborator

@scottf scottf commented Feb 12, 2026

No description provided.

@scottf
Copy link
Collaborator Author

scottf commented Feb 12, 2026

@buleuszmatak I'll give you a few days to check this out and comment before I merge it.


@Override
public byte[] getEncoded() {
return privateKey.getSecret();

Choose a reason for hiding this comment

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

https://github.com/nats-io/nkeys.java/pull/18/changes#r2711848345

If this isn't correct, it may be wrong since the original.

Yes

Also, it may not matter for the purpose of the NKEY.

Yes. nkeys.java returns incorrect value, but everywhere it calls this method, it expects the value, so things cancel out for the nkey.java itself.

After all, it's essentially just a NATS thing, not a general security thing.

java.security.KeyPair is a general security thing, and io.nats.nkey.NKey#getKeyPair is public.

I'm sure I don't understand why this is a problem, but this works

   Ed25519PrivateKeyParameters privateKey = new Ed25519PrivateKeyParameters(nkey.getKeyPair().getPrivate().getEncoded());
   Ed25519Signer signer = new Ed25519Signer();
   signer.init(true, privateKey);
   signer.update(input, 0, input.length);
   return signer.generateSignature();
}

org.bouncycastle.crypto.params.Ed25519PrivateKeyParameters#Ed25519PrivateKeyParameters(byte[]) expects raw seed, as opposed to io.nats.nkey.KeyWrapper which claims to return key in the PKCS#8 format.


Possible solutions are:

  • don't return java.security.KeyPair, but some NATS thing
  • restrict access to io.nats.nkey.NKey#getKeyPair
  • prefix returned values to actually be PKCS#8
    • fixing this here, in FIPS provider, would be as simple as returning result of getEncoded instead of getSecret and calling the previous methods where the raw value is needed within the FipsNKeyProvider

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good suggestion, turned this into an issue to address later. #28

scottf and others added 2 commits February 16, 2026 08:23
Co-authored-by: Buleusz Matak <215200949+buleuszmatak@users.noreply.github.com>
Co-authored-by: Buleusz Matak <215200949+buleuszmatak@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants