Skip to content

feat(sdk): use opaque keys for sym and asym#836

Open
elizabethhealy wants to merge 70 commits intofeat/pluggable-crypto-servicefrom
dspx-1969-opaque-keys
Open

feat(sdk): use opaque keys for sym and asym#836
elizabethhealy wants to merge 70 commits intofeat/pluggable-crypto-servicefrom
dspx-1969-opaque-keys

Conversation

@elizabethhealy
Copy link
Member

@elizabethhealy elizabethhealy commented Feb 13, 2026

TLDR

  • Opaque key types (KeyPair, PublicKey, PrivateKey, SymmetricKey)
  • new export functions in the crypto service since the key are now opaque (need a way to get them out)
  • move key splitting (splitSymmetricKey) and merging (mergeSymmetricKeys) operations within the crypto service (private symmetric keys shouldnt have to leave the hsm for this)
  • certain optional functions including private key exporting are added for testing and non strict cases
  • adds export /cryptoutils which has some helper functions and types that can be reused in other crypto service implementations to avoid code duplication

Summary of Changes

This pull request introduces a fundamental refactoring of the SDK's cryptographic key management, transitioning from direct manipulation of PEM-encoded strings and raw byte arrays to a more secure and abstract opaque key object model. This change significantly improves the security posture by encapsulating key material, enabling more robust cryptographic service implementations, and streamlining key handling across the library's authentication, DPoP, and TDF components. It also lays the groundwork for future enhancements like hardware security module (HSM) integration.

Highlights

  • Opaque Key Management: The SDK now utilizes opaque key types (KeyPair, PublicKey, PrivateKey, SymmetricKey) for all cryptographic operations, abstracting key material from direct string or byte array representations. This enhances security by encapsulating key details and enables more flexible cryptographic service implementations, such as hardware security module (HSM) integration.
  • Enhanced CryptoService Interface: The CryptoService interface has been significantly expanded to include new methods for importing and exporting opaque keys to/from PEM and JWK formats. It also introduces dedicated methods for symmetric key splitting (splitSymmetricKey) and merging (mergeSymmetricKeys), and updates existing methods to accept and return the new opaque key types.
  • Elliptic Curve Identifier Correction: The P512 elliptic curve identifier has been corrected to P521 across the codebase, ensuring alignment with standard cryptographic specifications and Web Crypto API usage for improved compatibility and correctness.
  • Centralized Key Handling Logic: Key parsing, import, and export logic is now centralized within the CryptoService implementation, reducing scattered PEM string handling and promoting consistent, secure key management throughout the library's authentication, DPoP, and TDF components.
  • Improved Type Safety and Runtime Checks: New type definitions and runtime checks have been introduced in assertion and JWT handling to ensure that the correct opaque key types are used for specific cryptographic operations (e.g., preventing signing with a public key or verifying with a private key), leading to more robust and error-resistant code.

elizabethhealy and others added 30 commits January 20, 2026 16:26
Signed-off-by: Elizabeth Healy <ehealy@virtru.com>
Signed-off-by: Elizabeth Healy <ehealy@virtru.com>
Signed-off-by: Elizabeth Healy <ehealy@virtru.com>
Signed-off-by: Elizabeth Healy <ehealy@virtru.com>
Signed-off-by: Elizabeth Healy <ehealy@virtru.com>
Signed-off-by: Elizabeth Healy <ehealy@virtru.com>
Signed-off-by: Elizabeth Healy <ehealy@virtru.com>
@elizabethhealy elizabethhealy force-pushed the dspx-1969-opaque-keys branch from 8b2121f to 1bbc54e Compare March 2, 2026 19:20
@github-actions
Copy link

github-actions bot commented Mar 2, 2026

X-Test Failure Report

opentdf-sdk-lib
opentdf-ctl

@github-actions
Copy link

github-actions bot commented Mar 2, 2026

@elizabethhealy
Copy link
Member Author

/gemini review

@elizabethhealy
Copy link
Member Author

/gemini summary

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces opaque key types for both symmetric and asymmetric cryptography throughout the SDK, significantly improving security and code clarity by abstracting raw key material. However, a high-severity vulnerability was identified in the assertion verification logic, where untrusted keys from JWT headers are used without validation. Additionally, a medium-severity information leak was found in the CLI tool's logging. For further improvements, the use of SHA-1 in RSA-OAEP should be upgraded to SHA-256 for better security, and a missing method in the CryptoService interface for generating EC signing keys needs to be implemented.

Comment on lines 212 to +216
/**
* Generate an RSA key pair for signing/verification.
* @returns PEM-encoded key pair
* @returns Opaque key pair
*/
generateSigningKeyPair: () => Promise<PemKeyPair>;

/**
* Compute HMAC-SHA256 of content using key.
* @param key - Hex-encoded key bytes (e.g., "0a1b2c...")
* @param content - Hex-encoded content bytes to authenticate
* @returns Hex-encoded HMAC result
*
* Note: Callers should treat inputs and outputs as hex-encoded byte strings.
* Implementations may normalize case, but callers MUST NOT rely on a specific case.
*/
hmac: (key: string, content: string) => Promise<string>;
generateSigningKeyPair: () => Promise<KeyPair>;

Choose a reason for hiding this comment

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

medium

The generateSigningKeyPair method currently only supports generating RSA key pairs, while generateECKeyPair is specifically for ECDH. This leaves a gap for generating EC key pairs for signing (e.g., for ES256/ES384/ES512 assertions).

Consider adding a generateECSigningKeyPair method to the CryptoService interface to provide a complete set of key generation capabilities. This would be consistent with the support for EC-based signing algorithms in other parts of the service.

Copy link
Member Author

Choose a reason for hiding this comment

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

we dont currently have need to generate ec signging keys within the code, any ec signing and verification handling is from external keys coming from assertions or jwts.

should the need arise we can add this logic

@gemini-code-assist
Copy link

Summary of Changes

This pull request introduces a fundamental refactoring of the SDK's cryptographic key management, transitioning from direct manipulation of PEM-encoded strings and raw byte arrays to a more secure and abstract opaque key object model. This change significantly improves the security posture by encapsulating key material, enabling more robust cryptographic service implementations, and streamlining key handling across the library's authentication, DPoP, and TDF components. It also lays the groundwork for future enhancements like hardware security module (HSM) integration.

Highlights

  • Opaque Key Management: The SDK now utilizes opaque key types (KeyPair, PublicKey, PrivateKey, SymmetricKey) for all cryptographic operations, abstracting key material from direct string or byte array representations. This enhances security by encapsulating key details and enables more flexible cryptographic service implementations, such as hardware security module (HSM) integration.
  • Enhanced CryptoService Interface: The CryptoService interface has been significantly expanded to include new methods for importing and exporting opaque keys to/from PEM and JWK formats. It also introduces dedicated methods for symmetric key splitting (splitSymmetricKey) and merging (mergeSymmetricKeys), and updates existing methods to accept and return the new opaque key types.
  • Elliptic Curve Identifier Correction: The P512 elliptic curve identifier has been corrected to P521 across the codebase, ensuring alignment with standard cryptographic specifications and Web Crypto API usage for improved compatibility and correctness.
  • Centralized Key Handling Logic: Key parsing, import, and export logic is now centralized within the CryptoService implementation, reducing scattered PEM string handling and promoting consistent, secure key management throughout the library's authentication, DPoP, and TDF components.
  • Improved Type Safety and Runtime Checks: New type definitions and runtime checks have been introduced in assertion and JWT handling to ensure that the correct opaque key types are used for specific cryptographic operations (e.g., preventing signing with a public key or verifying with a private key), leading to more robust and error-resistant code.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog
  • cli/src/cli.ts
    • Updated key pair type imports and removed PEM format validation.
  • lib/package-lock.json
    • Resolved a merge conflict and standardized the jose dependency version.
  • lib/src/auth/auth.ts
    • Updated key type imports and modified reqSignature and updateClientPublicKey to use opaque key types.
  • lib/src/auth/dpop.ts
    • Replaced PEM key types with opaque key types, removed PEM string validation, and updated DPoP JWT generation to use opaque keys.
  • lib/src/auth/oidc-clientcredentials-provider.ts
    • Updated updateClientPublicKey to accept opaque KeyPair.
  • lib/src/auth/oidc-externaljwt-provider.ts
    • Updated updateClientPublicKey to accept opaque KeyPair.
  • lib/src/auth/oidc-refreshtoken-provider.ts
    • Updated updateClientPublicKey to accept opaque KeyPair.
  • lib/src/auth/oidc.ts
    • Replaced PemKeyPair with KeyPair and updated public key export for headers to use cryptoService.exportPublicKeyPem.
  • lib/src/crypto/enums.ts
    • Corrected the P512 elliptic curve identifier to P521.
  • lib/src/crypto/pemPublicToCrypto.ts
    • Corrected P_512 to P_521 and updated related curve name mapping functions.
  • lib/src/opentdf.ts
    • Updated dpopKeys type to KeyPair and adjusted comments for opaque key generation.
  • lib/tdf3/index.ts
    • Exported new opaque key types and updated WebCryptoService export.
  • lib/tdf3/src/assertions.ts
    • Updated AssertionKey to support opaque key types, added runtime type checks for signing and verification, and updated key import logic.
  • lib/tdf3/src/ciphers/aes-gcm-cipher.ts
    • Modified encrypt and decrypt methods to accept opaque SymmetricKey objects.
  • lib/tdf3/src/ciphers/symmetric-cipher-base.ts
    • Updated generateKey to return SymmetricKey and modified abstract encrypt and decrypt methods to use SymmetricKey.
  • lib/tdf3/src/client/builders.ts
    • Updated DecryptKeyMiddleware to use opaque SymmetricKey.
  • lib/tdf3/src/client/index.ts
    • Replaced PemKeyPair with KeyPair for DPoP keys and session key creation, and updated key parsing functions to use parsePublicKeyPem.
  • lib/tdf3/src/crypto/declarations.ts
    • Introduced new opaque key types (KeyAlgorithm, KeyOptions, PublicKey, PrivateKey, KeyPair, SymmetricKey) and updated the CryptoService interface to use them.
  • lib/tdf3/src/crypto/index.ts
    • Implemented internal wrapping/unwrapping for opaque keys, updated all cryptographic functions to use opaque types, and added new key import/export methods.
    • Modified parsePublicKeyPem to include minimum key size validation for RSA keys.
  • lib/tdf3/src/crypto/jwt.ts
    • Updated signJwt and verifyJwt to accept opaque key types and added type-checking logic for symmetric vs. asymmetric algorithms.
  • lib/tdf3/src/models/encryption-information.ts
    • Removed keySplit import, updated KeyInfo to use SymmetricKey, and modified key generation, encryption, decryption, and key access object creation to use opaque symmetric keys.
  • lib/tdf3/src/models/key-access.ts
    • Updated key type imports, changed ECWrapped.ephemeralKeyPair to KeyPair, and modified write methods to use opaque key types and new CryptoService functions.
  • lib/tdf3/src/tdf.ts
    • Removed keyMerge import, updated configuration types to use KeyPair, and modified signature generation, encryption, and decryption flows to use opaque symmetric and asymmetric keys.
  • lib/tests/mocha/unit/assertions.spec.ts
    • Updated tests to generate and use opaque ECDSA keys for signing and verification, and removed a PEM conversion helper.
  • lib/tests/mocha/unit/crypto-di.spec.ts
    • Updated the mock CryptoService interface to conform to the new opaque key types and methods.
  • lib/tests/mocha/unit/crypto/crypto-service.spec.ts
    • Updated HMAC, digest, key generation, ECDH, and signing/verification tests to use opaque key types and new CryptoService methods.
  • lib/tests/mocha/unit/jwt.spec.ts
    • Updated JWT signing and verification tests to use opaque key objects and adjusted error message expectations.
  • lib/tests/mocha/unit/key-access.test.ts
    • Updated mock CryptoService to return opaque key types and modified test data to use opaque symmetric keys.
  • lib/tests/mocha/unit/keysplits.spec.ts
    • Replaced generateKey with randomBytes for raw key generation in tests.
  • lib/tests/server.ts
    • Modified encryptWithPublicKey call to correctly import the client public key as an opaque PublicKey.
  • web-app/src/session.ts
    • Updated imports, changed signingKey to an opaque KeyPair, and modified key generation and handling to use WebCryptoService for importing/exporting opaque keys.
Activity
  • On 2026-02-13, gemini-code-assist[bot] provided an initial summary of changes and identified several high and medium priority review comments related to potential data leakage, generic function naming, fragile key detection, and lack of minimum key size enforcement.
  • On 2026-02-13, github-actions[bot] provided standard git commands for signoff or revert.
  • On 2026-02-26, github-actions[bot] reported successful X-Test results across all Go, Java, and JS versions.
  • On 2026-02-26, elizabethhealy requested a /gemini review.
  • On 2026-02-26, gemini-code-assist[bot] added review comments regarding runtime checks for PrivateKey usage in verification and clarifying the generateECKeyPair function's purpose.
  • On 2026-02-26, github-actions[bot] reported X-Test failures for opentdf-sdk-lib and opentdf-ctl.
  • On 2026-02-26, github-actions[bot] reported successful X-Test results across all Go, Java, and JS versions.
  • On 2026-02-26, github-actions[bot] reported X-Test failures for opentdf-sdk-lib and opentdf-ctl.
  • On 2026-02-26, github-actions[bot] reported X-Test failures across all Go, Java, and JS versions.
  • On 2026-02-26, github-actions[bot] reported successful X-Test results across all Go, Java, and JS versions.
  • On 2026-02-26, github-actions[bot] reported X-Test failures for opentdf-sdk-lib, opentdf-ctl, and JS versions.
  • On 2026-02-26, github-actions[bot] reported successful X-Test results across all Go, Java, and JS versions.
  • On 2026-03-02, elizabethhealy commented on a previous review thread, noting that the private key detection logic had been moved.
  • On 2026-03-02, elizabethhealy requested a /gemini review.
  • On 2026-03-02, gemini-code-assist[bot] added new review comments concerning the lack of minimum RSA key size enforcement, insecure storage of DPoP private keys in sessionStorage, a duplicate derToPem helper, and the absence of an EC signing key generation method.
  • On 2026-03-02, github-actions[bot] provided standard git commands for signoff or revert.
  • On 2026-03-02, github-actions[bot] reported X-Test failures for opentdf-sdk-lib, opentdf-ctl, and go-v0.9.0.
  • On 2026-03-02, github-actions[bot] reported X-Test failures for opentdf-sdk-lib and opentdf-ctl.
  • On 2026-03-02, github-actions[bot] reported X-Test failures for opentdf-sdk-lib, opentdf-ctl, and go-v0.9.0.
  • On 2026-03-02, elizabethhealy requested a /gemini review.
  • On 2026-03-02, elizabethhealy requested a /gemini summary.
  • On 2026-03-02, gemini-code-assist[bot] provided a review comment suggesting the addition of a generateECSigningKeyPair method to the CryptoService interface.

@elizabethhealy
Copy link
Member Author

/gemini summary

@elizabethhealy
Copy link
Member Author

/gemini review

@gemini-code-assist
Copy link

Summary of Changes

This pull request fundamentally refactors the SDK's cryptographic key management by transitioning from direct manipulation of PEM-encoded strings and raw byte arrays to a more secure and abstract opaque key object model. This change significantly improves the security posture by encapsulating key material, enabling more robust cryptographic service implementations, and streamlining key handling across the library's authentication, DPoP, and TDF components. It also lays the groundwork for future enhancements like hardware security module (HSM) integration, ensuring a more secure and extensible cryptographic foundation.

Highlights

  • Opaque Key Management: The SDK now exclusively uses opaque key types (KeyPair, PublicKey, PrivateKey, SymmetricKey) for all cryptographic operations, abstracting key material from direct string or byte array representations to enhance security and enable flexible cryptographic service implementations like HSM integration.
  • Enhanced CryptoService Interface: The CryptoService interface has been significantly expanded to include new methods for importing and exporting opaque keys to/from PEM and JWK formats, as well as dedicated methods for symmetric key splitting and merging. Existing methods have been updated to accept and return these new opaque key types.
  • Elliptic Curve Identifier Correction: The P512 elliptic curve identifier has been corrected to P521 across the codebase, ensuring alignment with standard cryptographic specifications and Web Crypto API usage for improved compatibility and correctness.
  • Centralized Key Handling Logic: Key parsing, import, and export logic is now centralized within the CryptoService implementation, reducing scattered PEM string handling and promoting consistent, secure key management throughout the library's authentication, DPoP, and TDF components.
  • Improved Type Safety and Runtime Checks: New type definitions and runtime checks have been introduced in assertion and JWT handling to ensure that the correct opaque key types are used for specific cryptographic operations (e.g., preventing signing with a public key or verifying with a private key), leading to more robust and error-resistant code.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog
  • cli/src/cli.ts
    • Updated key pair type imports from PemKeyPair to KeyPair.
    • Removed PEM format validation function isPemFormatted.
  • lib/package-lock.json
    • Resolved a merge conflict related to the 'jose' dependency version.
  • lib/package.json
    • Added a new export mapping for './cryptoutils' to expose crypto-related helper functions.
  • lib/src/auth/auth.ts
    • Updated imports to use KeyPair and PrivateKey opaque types.
    • Modified reqSignature to accept an opaque PrivateKey instead of a PEM string.
    • Updated updateClientPublicKey to accept an opaque KeyPair instead of PemKeyPair.
  • lib/src/auth/dpop.ts
    • Updated imports to use KeyPair and PrivateKey opaque types.
    • Removed the isPemString utility function.
    • Modified jwt function to accept an opaque PrivateKey for signing.
    • Updated DPoP function to accept an opaque KeyPair and use exportPublicKeyJwk for the header.
  • lib/src/auth/oidc-clientcredentials-provider.ts
    • Updated updateClientPublicKey method to accept an opaque KeyPair.
  • lib/src/auth/oidc-externaljwt-provider.ts
    • Updated updateClientPublicKey method to accept an opaque KeyPair.
  • lib/src/auth/oidc-refreshtoken-provider.ts
    • Updated updateClientPublicKey method to accept an opaque KeyPair.
  • lib/src/auth/oidc.ts
    • Updated imports to use KeyPair opaque type.
    • Changed the signingKey property type from PemKeyPair to KeyPair.
    • Modified X-VirtruPubKey header generation to export the opaque public key to PEM format using cryptoService.exportPublicKeyPem.
  • lib/src/crypto/enums.ts
    • Corrected the P512 elliptic curve identifier to P521 in the NamedCurve enum.
  • lib/src/crypto/index.ts
    • Exported additional crypto utilities and PEM formatting functions from pemPublicToCrypto.js and crypto-utils.js.
  • lib/src/crypto/pemPublicToCrypto.ts
    • Corrected P_512 to P_521 in CurveName type definition.
    • Updated guessCurveName and toJwsAlg functions to use P_521 instead of P_512.
  • lib/src/opentdf.ts
    • Updated dpopKeys type from Promise to Promise.
    • Adjusted comments to reflect the use of opaque KeyPair for DPoP keys and HSM-backed crypto implementations.
  • lib/tdf3/index.ts
    • Exported new opaque key types: KeyPair, KeyOptions, KeyAlgorithm, PrivateKey, PublicKey, and SymmetricKey.
    • Updated the export of WebCryptoService to use DefaultCryptoService.
  • lib/tdf3/src/assertions.ts
    • Updated imports to include PrivateKey, PublicKey, and SymmetricKey opaque types.
    • Added runtime type checks in the sign function to prevent signing with a PublicKey and to import PEM strings/Uint8Arrays into opaque keys.
    • Added runtime type checks in the verify function to prevent verifying with a PrivateKey.
    • Updated AssertionKey type to support opaque PrivateKey, PublicKey, and SymmetricKey.
  • lib/tdf3/src/ciphers/aes-gcm-cipher.ts
    • Updated encrypt and decrypt methods to accept an opaque SymmetricKey instead of Binary for the encryption key.
  • lib/tdf3/src/ciphers/symmetric-cipher-base.ts
    • Updated generateKey to return an opaque SymmetricKey instead of a string.
    • Modified abstract encrypt and decrypt methods to use SymmetricKey.
  • lib/tdf3/src/client/builders.ts
    • Updated DecryptKeyMiddleware type to use an opaque SymmetricKey.
  • lib/tdf3/src/client/index.ts
    • Updated imports to use KeyPair and SymmetricKey opaque types.
    • Changed dpopKeys type from Promise to Promise.
    • Updated algorithmFromPEM and resolveKasInfo to use cryptoService.parsePublicKeyPem.
    • Modified createSessionKeys to return an opaque KeyPair.
    • Removed comments related to base64 encoding PEM strings for public keys.
    • Updated decrypt method's keyMiddleware to accept an opaque SymmetricKey.
  • lib/tdf3/src/crypto/declarations.ts
    • Introduced new opaque types: KeyAlgorithm, KeyOptions, PublicKey, PrivateKey, KeyPair, and SymmetricKey.
    • Updated the CryptoService interface to use these new opaque types for all key-related operations.
    • Added new methods to CryptoService for importing/exporting opaque keys (importPublicKey, importPrivateKey, exportPublicKeyPem, exportPrivateKeyPem, exportPublicKeyJwk, importSymmetricKey).
    • Added splitSymmetricKey and mergeSymmetricKeys methods to CryptoService.
  • lib/tdf3/src/crypto/index.ts
    • Implemented internal wrapping/unwrapping logic for opaque PublicKey, PrivateKey, and SymmetricKey types.
    • Updated generateKey to return an opaque SymmetricKey.
    • Updated generateKeyPair and generateSigningKeyPair to return opaque KeyPair objects.
    • Modified encryptWithPublicKey and decryptWithPrivateKey to accept/return opaque key types.
    • Updated encrypt and decrypt to accept opaque SymmetricKey for encryption/decryption.
    • Replaced string-based hmac with a new hmac function that accepts an opaque SymmetricKey.
    • Updated sign and verify to accept opaque PrivateKey and PublicKey respectively.
    • Removed generateInitializationVector, sha256, and the old string-based hmac, signSymmetric, verifySymmetric functions.
    • Added implementations for importPublicKey, importPrivateKey, exportPublicKeyPem, exportPrivateKeyPem, exportPublicKeyJwk, importSymmetricKey, splitSymmetricKey, and mergeSymmetricKeys.
    • Added minimum RSA key size validation in parsePublicKeyPem.
  • lib/tdf3/src/crypto/jwt.ts
    • Updated imports to include PrivateKey, PublicKey, and SymmetricKey opaque types.
    • Modified signJwt and verifyJwt to accept opaque key types.
    • Added type-checking logic in signJwt and verifyJwt to ensure correct key types are used for symmetric vs. asymmetric algorithms.
  • lib/tdf3/src/models/encryption-information.ts
    • Removed keySplit import from '../utils/index.js'.
    • Updated KeyInfo type to use unwrappedKey: SymmetricKey instead of unwrappedKeyBinary: Binary.
    • Modified generateKey, encrypt, decrypt, and getKeyAccessObjects methods to work with opaque SymmetricKey and cryptoService.splitSymmetricKey.
  • lib/tdf3/src/models/key-access.ts
    • Updated imports to include KeyPair and SymmetricKey opaque types.
    • Changed ECWrapped's ephemeralKeyPair to Promise.
    • Modified ECWrapped.write to accept an opaque SymmetricKey for dek and use cryptoService.importPublicKey and cryptoService.exportPublicKeyPem.
    • Modified Wrapped.write to accept an opaque SymmetricKey for keyBuffer and use cryptoService.importPublicKey.
  • lib/tdf3/src/tdf.ts
    • Updated imports to include KeyPair and SymmetricKey opaque types.
    • Removed keyMerge import from '../utils/index.js'.
    • Updated EncryptConfiguration and DecryptConfiguration to use KeyPair for dpopKeys.
    • Modified getSignature and getSignatureVersion422 to accept an opaque SymmetricKey for unwrappedKey.
    • Updated various encryption and decryption flows to use opaque SymmetricKey for DEKs and KeyPair for ephemeral keys.
  • lib/tests/mocha/unit/assertions.spec.ts
    • Updated ES256 test to generate ECDSA keys directly for signing and verification using Web Crypto API and wrap them as opaque keys.
    • Removed the pemToArrayBuffer helper function.
  • lib/tests/mocha/unit/crypto-di.spec.ts
    • Updated the mock CryptoService interface to align with the new opaque key types and methods, ensuring all new methods are present and throw NOT_IMPLEMENTED errors.
  • lib/tests/mocha/unit/crypto/crypto-service.spec.ts
    • Updated hmac tests to use importSymmetricKey for key creation and verify hex encoded output.
    • Modified digest tests to use digest function directly and verify hex encoded output.
    • Modified generateKey tests to assert on SymmetricKey properties (_brand, length).
    • Updated generateKeyPair to test with 2048 bits.
    • Modified generateECKeyPair tests to assert on opaque PublicKey and PrivateKey properties.
    • Updated deriveKeyFromECDH tests to verify SymmetricKey return type and properties.
    • Modified signSymmetric and verifySymmetric tests to use hmac and verifyHmac with opaque SymmetricKey.
    • Updated importPublicKeyPem tests to use parsePublicKeyPem and exportPublicKeyPem for key generation.
    • Modified sign and verify tests for ES256, ES384, and ES512 to explicitly generate ECDSA keys for signing and wrap them as opaque keys.
  • lib/tests/mocha/unit/jwt.spec.ts
    • Updated privateKeyPem and publicKeyPem variables to privateKey and publicKey (opaque types).
    • Adjusted signJwt and verifyJwt calls to pass opaque key objects.
    • Updated error messages in tests to reflect new opaque key type requirements for HS256 and RS256.
  • lib/tests/mocha/unit/key-access.test.ts
    • Updated mock CryptoService to return opaque KeyPair and SymmetricKey for relevant methods.
    • Modified dek variable to dekBytes and used importSymmetricKey to create an opaque dek for testing.
  • lib/tests/mocha/unit/keysplits.spec.ts
    • Replaced generateKey with randomBytes for raw key generation in tests.
  • lib/tests/server.ts
    • Updated encryptWithPublicKey call to import the client public key as an opaque PublicKey using DefaultCryptoService.importPublicKey.
  • lib/tests/web/auth/auth.test.ts
    • Updated generateTestSigningKey to return an opaque KeyPair.
  • web-app/src/session.ts
    • Updated imports to include KeyPair and WebCryptoService.
    • Changed signingKey to a private property of type KeyPair.
    • Modified key generation and handling to use WebCryptoService.importPublicKey and WebCryptoService.importPrivateKey for opaque key import.
    • Updated DPoP header generation to use WebCryptoService.exportPublicKeyPem and WebCryptoService.exportPrivateKeyPem to convert opaque keys to PEM for external libraries.
Activity
  • On 2026-02-13, gemini-code-assist[bot] provided an initial summary of changes and identified several high and medium priority review comments related to potential data leakage, generic function naming, fragile key detection, and lack of minimum key size enforcement.
  • On 2026-02-13, github-actions[bot] provided standard git commands for signoff or revert.
  • On 2026-02-26, github-actions[bot] reported successful X-Test results across all Go, Java, and JS versions.
  • On 2026-02-26, elizabethhealy requested a /gemini review.
  • On 2026-02-26, gemini-code-assist[bot] added review comments regarding runtime checks for PrivateKey usage in verification and clarifying the generateECKeyPair function's purpose.
  • On 2026-02-26, github-actions[bot] reported X-Test failures for opentdf-sdk-lib and opentdf-ctl.
  • On 2026-02-26, github-actions[bot] reported successful X-Test results across all Go, Java, and JS versions.
  • On 2026-02-26, github-actions[bot] reported X-Test failures for opentdf-sdk-lib and opentdf-ctl.
  • On 2026-02-26, github-actions[bot] reported X-Test failures across all Go, Java, and JS versions.
  • On 2026-02-26, github-actions[bot] reported successful X-Test results across all Go, Java, and JS versions.
  • On 2026-02-26, github-actions[bot] reported X-Test failures for opentdf-sdk-lib, opentdf-ctl, and JS versions.
  • On 2026-02-26, github-actions[bot] reported successful X-Test results across all Go, Java, and JS versions.
  • On 2026-03-02, elizabethhealy commented on a previous review thread, noting that the private key detection logic had been moved.
  • On 2026-03-02, elizabethhealy requested a /gemini review.
  • On 2026-03-02, gemini-code-assist[bot] added new review comments concerning the lack of minimum RSA key size enforcement, insecure storage of DPoP private keys in sessionStorage, a duplicate derToPem helper, and the absence of an EC signing key generation method.
  • On 2026-03-02, github-actions[bot] provided standard git commands for signoff or revert.
  • On 2026-03-02, github-actions[bot] reported X-Test failures for opentdf-sdk-lib, opentdf-ctl, and go-v0.9.0.
  • On 2026-03-02, github-actions[bot] reported X-Test failures for opentdf-sdk-lib and opentdf-ctl.
  • On 2026-03-02, github-actions[bot] reported X-Test failures for opentdf-sdk-lib, opentdf-ctl, and go-v0.9.0.
  • On 2026-03-02, elizabethhealy requested a /gemini review.
  • On 2026-03-02, elizabethhealy requested a /gemini summary.
  • On 2026-03-02, gemini-code-assist[bot] provided a review comment suggesting the addition of a generateECSigningKeyPair method to the CryptoService interface.
  • On 2026-03-02, elizabethhealy responded to the suggestion, stating that the functionality is not currently needed.
  • On 2026-03-02, elizabethhealy requested a /gemini summary and /gemini review again.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This is a major and important refactoring to introduce opaque keys for cryptographic operations, enhancing security and abstraction. The changes are extensive, touching many parts of the SDK, from the core crypto service to authentication providers and the CLI. The new CryptoService interface is well-defined, and the implementation correctly uses opaque types to encapsulate key material. The correction of the elliptic curve identifier from P-512 to P-521 is also a welcome fix. Overall, this is a high-quality contribution that significantly improves the SDK's architecture. My review includes one minor suggestion for improving type consistency in the web-app session handling.

Note: Security Review is unavailable for this PR.

// TODO: This validation function might be too restrictive and also allow for some weird edge cases.
// Like a PEM that begins with BEGIN PUBLIC KEY but ends with END PRIVATE KEY
// Use the isPemKeyPair from the lib/src/crypto/crypto-utils.ts file for a more robust solution if this becomes an issue.
function isPemFormatted(key: string): boolean {
Copy link
Member Author

Choose a reason for hiding this comment

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

removing now unnecessary pem comversion code

export enum NamedCurve {
P256 = 'P-256',
P384 = 'P-384',
P512 = 'P-512',
Copy link
Member Author

Choose a reason for hiding this comment

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

this was fixed on main as well i believe

removePemFormatting,
isPemKeyPair,
isCryptoKeyPair,
} from '../../tdf3/src/crypto/crypto-utils.js';
Copy link
Member Author

Choose a reason for hiding this comment

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

exporting some helpful crypto types and helpers for downstream use to avoid code duplication

encryptWithPublicKey: (payload: Binary, publicKey: string) => Promise<Binary>;

/** Get length random bytes as a hex-encoded string. */
generateInitializationVector: (length?: number) => Promise<string>;
Copy link
Member Author

@elizabethhealy elizabethhealy Mar 2, 2026

Choose a reason for hiding this comment

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

removed in favor of randomBytes, seemed like unnecessary duplication

* Note: Callers should treat inputs and outputs as hex-encoded byte strings.
* Implementations may normalize case, but callers MUST NOT rely on a specific case.
*/
hmac: (key: string, content: string) => Promise<string>;
Copy link
Member Author

Choose a reason for hiding this comment

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

this remains, it just does not handle the hex encoding anymore, the core hmac functionality replaces the hmac+signSymmetric functions to avoid unnecessary duplication

*
* Note: Multi-KAS may not be available in all secure environments (single KAS only)
*/
splitSymmetricKey: (key: SymmetricKey, numShares: number) => Promise<SymmetricKey[]>;
Copy link
Member Author

Choose a reason for hiding this comment

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

needed to move these into the crypto service as well since technically keys shouldnt leave and hsm for these ops

} else if (keySize === 4096) {
algorithm = 'rsa:4096';
} else {
throw new ConfigurationError(
Copy link
Member Author

Choose a reason for hiding this comment

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

being more specific on supported key sizes

@github-actions
Copy link

github-actions bot commented Mar 2, 2026

* @returns Validated PEM and detected algorithm
* @throws ConfigurationError if key format invalid or algorithm not supported
*/
importPublicKeyPem: (pem: string) => Promise<PublicKeyInfo>;
Copy link
Member Author

Choose a reason for hiding this comment

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

rename for clarity, not really an import anymore, just getting info about the public key

}

if (typeof key.key === 'string') {
if (!cryptoService.importPrivateKey) {
Copy link
Member Author

@elizabethhealy elizabethhealy Mar 2, 2026

Choose a reason for hiding this comment

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

to maintain compatibility with some of the existing flow -- this allows importing private keys only if the crypto service supports it, if a users crypto service impl does not then they will have to provide opaque keys generated by their crypto service

mimeType = 'unknown',
windowSize = DEFAULT_SEGMENT_SIZE,
keyMiddleware = defaultKeyMiddleware,
keyMiddleware: keyMiddlewareOpt,
Copy link
Member Author

Choose a reason for hiding this comment

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

needed to support the ability to pass the cryoto service into the key middle ware on construction

"require": "./dist/cjs/src/crypto/index.js",
"import": "./dist/web/src/crypto/index.js"
}
},
Copy link
Member Author

Choose a reason for hiding this comment

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

exposing for use as helpers downstream

@elizabethhealy
Copy link
Member Author

xtest failure is due to some added tests, they are failing on main as well so im ignoring for now and filing as separate bug

@elizabethhealy elizabethhealy marked this pull request as ready for review March 2, 2026 21:26
@elizabethhealy elizabethhealy requested review from a team as code owners March 2, 2026 21:26
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.

1 participant