Skip to content

Commit 2a003fa

Browse files
Arpan0995dghgit
authored andcommitted
pgsc: the YubiKey decryptor factories clear a clone of the card user PIN rather than the array the KeyPassphraseProvider still owns, incorporating github PR #2444.
1 parent d47c0a3 commit 2a003fa

7 files changed

Lines changed: 334 additions & 5 deletions

File tree

‎CONTRIBUTORS.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -542,7 +542,7 @@ We also wish to acknowledge financial and collaborative support from [CISCO](htt
542542
- jmeeder \<https://github.com/jmeeder\> - reporting that RFC 4998 evidence-record generation rejected time stamps from an authority naming the digest with NULL parameters where BC names it with them absent, both of which RFC 5754 requires a receiver to accept (issue #2379).
543543
- rimuln \<https://github.com/rimuln\> - diagnosis and fix for PKCS12 getCertificateAlias returning the alias of an unrelated certificate, tracing it to the alias and certificate enumerations of the keystore's certs table diverging in order once keys() enumerated a copy (issue #2384, PR #2385).
544544
- Yu Bao \<yubao&#064;paypal.com\> - reporting an API gap, on behalf of the PayPal Cyber Security Team, that the high-level OpenPGP message API (OpenPGPMessageProcessor / OpenPGPMessageInputStream) gave a caller no way to bound how far a compressed data packet expands, where the low-level PGPCompressedData it wraps has carried a bounded getDataStream(long) overload all along, and that OpenPGPPolicy exposed no equivalent property to set. Suggesting a protocol whitelist for CRL Distribution Point fetching, which is now the org.bouncycastle.x509.CRLDP_protocols property, and suggesting that an OCSP response was read up to the length the responder declared for itself, now capped by org.bouncycastle.ocsp.max_response_size, and suggesting bounds on the OpenPGP ASCII armor headers, now capped by org.bouncycastle.openpgp.max_armor_header_length and org.bouncycastle.openpgp.max_armor_headers.
545-
- Arpan Sharma \<https://github.com/Arpan0995\> - initial audit of BCPQC provider consistency starting with HQC, which led to the exposure of a number of issues in the JCA provider service interfaces for other BCPQC algorithms. In-depth auditing of PQC signature algorithms in the provider leading to the correction of a number of JCA API compliance issues. Initial implementation of the guard that lets a signature context be set on the composite ML-DSA services before initSign / initVerify, the composite counterpart of the base-engine fix for issue #2396 (issue #2412). Audit of the LMS / HSS stateful private key decoder, establishing that the level count, the one-time index and the persisted tree cache were all accepted without validation (issue #2414); that issue was also the inspiration for the further investigation which found the HSS and XMSS^MT decoders accepting a private key whose declared index disagreed with the traversal state stored beside it, allowing a one-time key to be used twice. Audit of the high-level OpenPGP API (org.bouncycastle.openpgp, org.bouncycastle.openpgp.api) and the bcpg packet layer beneath it, spanning certificate evaluation, message verification, decryption and signature subpacket parsing, which found certificates being used without the certifications RFC 9580 requires of them, malformed but signed content reaching ordinary reader code as unchecked exceptions, a truncated message being read as a clean end of message, and unverified plaintext being released ahead of the integrity check (issues #2417, #2424, #2426). Audit of the recipient side of the CMS RFC 9629 KEMRecipientInfo path, which found three sender-controlled fields leaving methods declared to throw CMSException as unchecked exceptions and the kekLength never being confirmed against the wrap algorithm, and which led to the same translation being added to the neighbouring AuthEnvelopedData constructor and the two streaming parsers (issue #2422). Audit of RSA-PSS parameter handling in the provider, which found Signature.setParameter reporting a rejected trailer field as an unchecked exception and leaving the rejected spec half-applied, so that the parameters reported for a signature were not the ones it had been made with (issue #2421). Audit of the PKCS#12 key store entry-setting boundary, which found setKeyEntry reporting a certificate whose public key the provider cannot resolve as an unchecked exception rather than the KeyStoreException it declares, and which led to the same guard being applied to setCertificateEntry and to the RFC 9579 PBMAC1 store, and to the rejected entry no longer being left behind in the store (issue #2419). Further auditing of the verification and expiration paths of the high-level OpenPGP API, which found a configured algorithm policy not being applied to inline message verification, an expired primary key still offering its subkeys, and a data signature being reported valid past its own expiration time. CMS SignedData and PKIX certification path test coverage for the eighteen Composite ML-DSA parameter sets, which had been reachable through the CMS and cert-path signature algorithm finders with no test exercising either (PR #2437). Identification and fix of the output offset in the AEAD stream cipher data operator that made Grain-128AEAD return corrupted plaintext from a decryption driven in chunks, with the streamed decryption coverage that had been missing (PR #2447). Identification and fix of the RFC 9709 content-encryption algorithm being left wrapped at the CMS recipient key-size, allowed-algorithm and tag-size checks, so that none of them applied to the algorithm the content was encrypted under (PR #2446).
545+
- Arpan Sharma \<https://github.com/Arpan0995\> - initial audit of BCPQC provider consistency starting with HQC, which led to the exposure of a number of issues in the JCA provider service interfaces for other BCPQC algorithms. In-depth auditing of PQC signature algorithms in the provider leading to the correction of a number of JCA API compliance issues. Initial implementation of the guard that lets a signature context be set on the composite ML-DSA services before initSign / initVerify, the composite counterpart of the base-engine fix for issue #2396 (issue #2412). Audit of the LMS / HSS stateful private key decoder, establishing that the level count, the one-time index and the persisted tree cache were all accepted without validation (issue #2414); that issue was also the inspiration for the further investigation which found the HSS and XMSS^MT decoders accepting a private key whose declared index disagreed with the traversal state stored beside it, allowing a one-time key to be used twice. Audit of the high-level OpenPGP API (org.bouncycastle.openpgp, org.bouncycastle.openpgp.api) and the bcpg packet layer beneath it, spanning certificate evaluation, message verification, decryption and signature subpacket parsing, which found certificates being used without the certifications RFC 9580 requires of them, malformed but signed content reaching ordinary reader code as unchecked exceptions, a truncated message being read as a clean end of message, and unverified plaintext being released ahead of the integrity check (issues #2417, #2424, #2426). Audit of the recipient side of the CMS RFC 9629 KEMRecipientInfo path, which found three sender-controlled fields leaving methods declared to throw CMSException as unchecked exceptions and the kekLength never being confirmed against the wrap algorithm, and which led to the same translation being added to the neighbouring AuthEnvelopedData constructor and the two streaming parsers (issue #2422). Audit of RSA-PSS parameter handling in the provider, which found Signature.setParameter reporting a rejected trailer field as an unchecked exception and leaving the rejected spec half-applied, so that the parameters reported for a signature were not the ones it had been made with (issue #2421). Audit of the PKCS#12 key store entry-setting boundary, which found setKeyEntry reporting a certificate whose public key the provider cannot resolve as an unchecked exception rather than the KeyStoreException it declares, and which led to the same guard being applied to setCertificateEntry and to the RFC 9579 PBMAC1 store, and to the rejected entry no longer being left behind in the store (issue #2419). Further auditing of the verification and expiration paths of the high-level OpenPGP API, which found a configured algorithm policy not being applied to inline message verification, an expired primary key still offering its subkeys, and a data signature being reported valid past its own expiration time. CMS SignedData and PKIX certification path test coverage for the eighteen Composite ML-DSA parameter sets, which had been reachable through the CMS and cert-path signature algorithm finders with no test exercising either (PR #2437). Identification and fix of the output offset in the AEAD stream cipher data operator that made Grain-128AEAD return corrupted plaintext from a decryption driven in chunks, with the streamed decryption coverage that had been missing (PR #2447). Identification and fix of the RFC 9709 content-encryption algorithm being left wrapped at the CMS recipient key-size, allowed-algorithm and tag-size checks, so that none of them applied to the algorithm the content was encrypted under (PR #2446). Identification and fix of the YubiKey OpenPGP smart-card decryptor factories zeroizing the user PIN array belonging to the KeyPassphraseProvider that supplied it, which left the next private-key operation presenting an all-zero PIN to the card (PR #2444).
546546
- Flowdalic \<https://github.com/Flowdalic\> - initial implementation of an AnimalSniffer-based Android API-level compatibility check for the Gradle build (PR #336).
547547
- hannesa2 \<https://github.com/hannesa2\> - initial Dependabot configuration for the Gradle and GitHub Actions ecosystems (PR #883).
548548
- vladhuma \<https://github.com/vladhuma\> - initial implementation of server-side OCSP stapling for the BCJSSE provider, on behalf of Thales Group (PR #1740).

‎docs/releasenotes.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ Date: 2026, TBD
3131
- The AEAD stream cipher data operator, which Grain-128AEAD alone uses, wrote its output without first checking that the caller's buffer was long enough, so a short output buffer surfaced as an ArrayIndexOutOfBoundsException from inside the engine rather than as the OutputLengthException the general path reports for every other AEAD engine. Both directions of processBytes(), and processByte(), now check before anything is written or buffered, and only when the call releases output, as the general path does.
3232
- The RFC 9709 content-encryption AlgorithmIdentifier, which carries the real algorithm inside the parameters of an outer id-alg-cek-hkdf-sha256, was unwrapped at only one of the points where a CMS recipient makes a decision about it. Key-size validation was corrected for plain key transport in 1.86, but the same call in the KEK, RSA-KTS and KEM recipients, and in the key-transport recipient's own ORI-KEM branch, still compared the recovered key against the outer identifier, which registers no key size, so setKeySizeValidation(true) silently checked nothing there; the setAllowedContentAlgorithms allow-list and the setMinimumTagSize floor were applied to the outer identifier on every recipient family, including the one already corrected, so neither constrained an RFC 9709 message. The unwrap now happens once for the key-size check and once for the two policy checks, and every recipient polices and validates the content-encryption algorithm the message actually carries. A recipient with no allow-list, no tag floor and no key-size validation configured behaves exactly as before; a caller who listed id-alg-cek-hkdf-sha256 in an allow-list in order to admit RFC 9709 messages must now list the content-encryption algorithms themselves (github PR #2446).
3333
- A CMS message whose EncryptedContentInfo named the RFC 9709 key derivation but carried no readable content-encryption AlgorithmIdentifier in its parameters was reported as a NullPointerException, or as an IllegalArgumentException from the ASN.1 decoder, out of methods declared to throw CMSException, RecipientInformation.getContent() among them. The four places that resolve the wrapper - the CEK derivation, the content cipher selection, the key-size check, and the recipient's allowed-algorithm and tag-size checks - now share one resolver, which reports an absent or unreadable inner algorithm as a CMSException.
34+
- The two YubiKey OpenPGP smart-card decryptor factories zeroized the user PIN array the KeyPassphraseProvider handed them rather than a copy of it, in a finally block after each private-key operation. Both providers BC ships return the application's own array by reference - DefaultKeyPassphraseProvider hands back the char[] it has cached for the key, and the provider inside OpenPGPApi.editKey returns its argument - so the first card operation destroyed the caller's PIN and the next private-key operation presented an all-zero PIN to the card, which the card refuses at the cost of a PIN retry. Each factory now clears a clone of its own, and KeyPassphraseProvider.getKeyPassword records that the array it returns stays owned by the provider (github PR #2444).
3435

3536
### 2.1.3 Additional Features and Functionality
3637

‎pg/src/main/java/org/bouncycastle/openpgp/api/KeyPassphraseProvider.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ public interface KeyPassphraseProvider
1414
* Return the passphrase for the given key.
1515
* This callback is only fired, if the key is locked and a passphrase is required to unlock it.
1616
* Returning null means, that the passphrase is not available.
17+
* The returned array remains owned by the provider; a caller that needs to zeroize the
18+
* passphrase after use must clone it first.
1719
*
1820
* @param key the locked (sub-)key.
1921
* @return passphrase or null

‎pgsc/src/main/java/org/bouncycastle/openpgp/smartcard/yubikey/operator/bc/BcYubikeyPublicKeyDataDecryptorFactory.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -224,13 +224,13 @@ private static boolean isSupportedCurve(String curveName)
224224
}
225225

226226
/**
227-
* Fetch the card's user PIN. The returned array is the caller's to zeroize once the card has
228-
* verified it.
227+
* Fetch the card's user PIN. A defensive copy of the provider's array is returned, so
228+
* the copy is the caller's to zeroize once the card has verified it.
229229
*/
230230
private char[] requireUserPin()
231231
throws KeyPassphraseException
232232
{
233-
char[] pin = userPinProvider.getKeyPassword(getSecretKey());
233+
char[] pin = Arrays.clone(userPinProvider.getKeyPassword(getSecretKey()));
234234
if (pin == null || pin.length == 0)
235235
{
236236
throw new KeyPassphraseException(getSecretKey(), new IllegalStateException("PIN required."));

‎pgsc/src/main/java/org/bouncycastle/openpgp/smartcard/yubikey/operator/jcajce/JceYubikeyPublicKeyDataDecryptorFactoryBuilder.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,11 +173,15 @@ private static boolean isSupportedCurve(String curveName)
173173
|| "curve25519".equals(curveName);
174174
}
175175

176+
/**
177+
* Fetch the card's user PIN. A defensive copy of the provider's array is returned, so
178+
* the copy is the caller's to zeroize once the card has verified it.
179+
*/
176180
private char[] requireUserPin(KeyPassphraseProvider userPinProvider,
177181
OpenPGPKey.OpenPGPSecretKey key)
178182
throws KeyPassphraseException
179183
{
180-
char[] pin = userPinProvider.getKeyPassword(key);
184+
char[] pin = Arrays.clone(userPinProvider.getKeyPassword(key));
181185
if (pin == null || pin.length == 0)
182186
{
183187
throw new KeyPassphraseException(key, new IllegalStateException("PIN required."));

‎pgsc/src/test/java/org/bouncycastle/openpgp/smartcard/simulator/SimulatorTests.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import org.bouncycastle.openpgp.smartcard.test.AnonymousRecipientSmartCardDecryptionTest;
77
import org.bouncycastle.openpgp.smartcard.test.SmartCardMessageDecryptionTest;
88
import org.bouncycastle.openpgp.smartcard.test.SmartCardTestProperties;
9+
import org.bouncycastle.openpgp.smartcard.test.SmartCardUserPinOwnershipTest;
910
import org.bouncycastle.openpgp.smartcard.test.UnrelatedSmartCardMessageDecryptionTest;
1011
import org.bouncycastle.util.test.SimpleTestResult;
1112

@@ -25,6 +26,7 @@ public void testSimulatorSmartCard()
2526
new SmartCardMessageDecryptionTest(m, p),
2627
new AnonymousRecipientSmartCardDecryptionTest(m, p),
2728
new UnrelatedSmartCardMessageDecryptionTest(m, p),
29+
new SmartCardUserPinOwnershipTest(m, p),
2830
new SimulatorSmartCardTest(m, p)
2931
};
3032

0 commit comments

Comments
 (0)