Skip to content

Commit 0d0fb71

Browse files
committed
added use of nio.Files to minimise S/MIME copy exposure, relates to github #2326
1 parent 4830a77 commit 0d0fb71

10 files changed

Lines changed: 71 additions & 5 deletions

File tree

CONTRIBUTORS.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -591,7 +591,7 @@
591591
<li>subbudvk &lt;https://github.com/subbudvk&gt; - initial author on S2K parser hardening work for OpenPGP API.</li>
592592
<li>mkarasik &lt;https://github.com/mkarasik&gt; - initial work on EST server-side key generation (RFC 7030 4.4).</li>
593593
<li>Bernd Pr&uuml;nster (A-SIT Plus) &lt;bernd.pruenster&#064;a-sit.at&gt; - reported lenient ASN.1 UTCTime/GeneralizedTime parsing accepting structurally malformed content, with fuzzing-derived test cases.</li>
594-
<li>rootvector2 &lt;https://github.com/rootvector2&gt; - constant time comparison of membership and confirmation tags in the MLS API. Reported unbounded array allocation in the BKS/UBER keystore load path (OutOfMemoryError DoS from a crafted keystore) and introduction of a capped decompression limit in PGPCompressedData.getDataStream(), both with POC.</li>
594+
<li>rootvector2 &lt;https://github.com/rootvector2&gt; - constant time comparison of membership and confirmation tags in the MLS API. Reported unbounded array allocation in the BKS/UBER keystore load path (OutOfMemoryError DoS from a crafted keystore) and introduction of a capped decompression limit in PGPCompressedData.getDataStream(), both with POC. Original submission creating S/MIME backing temp files with owner-only permissions (PR #2326).</li>
595595
<li>suraj0208 &lt;https://github.com/suraj0208&gt; - initial work on auto-detecting private key reader (JcaPrivateKeyReader).</li>
596596
</ul>
597597
</body>

ant/jdk13.xml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -293,6 +293,7 @@
293293
</copy>
294294
<copy todir="${src.dir}" overwrite="true">
295295
<fileset dir="core/src/main/jdk1.4" includes="**/*.java" />
296+
<fileset dir="mail/src/main/jdk1.4" includes="**/*.java" />
296297
<fileset dir="prov/src/main/jdk1.4" includes="**/*.java" >
297298
<exclude name="**/LDAP*.java" />
298299
<exclude name="**/X509LDAP*.java" />

ant/jdk14.xml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,7 @@
232232
</copy>
233233
<copy todir="${src.dir}" overwrite="true">
234234
<fileset dir="core/src/main/jdk1.4" includes="**/*.java"/>
235+
<fileset dir="mail/src/main/jdk1.4" includes="**/*.java"/>
235236
<fileset dir="prov/src/main/jdk1.4" includes="**/*.java"/>
236237
<fileset dir="pkix/src/main/jdk1.4" includes="**/*.java"/>
237238
<fileset dir="pg/src/main/jdk1.4" includes="**/*.java"/>

ant/jdk15+.xml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@
7373
</copy>
7474
<copy todir="${src.dir}" overwrite="true">
7575
<fileset dir="core/src/main/jdk1.5" includes="**/*.java" />
76+
<fileset dir="mail/src/main/jdk1.5" includes="**/*.java" />
7677
<fileset dir="prov/src/main/jdk1.5" includes="**/*.java" />
7778
<fileset dir="prov/src/test/jdk1.5" includes="**/*.java" />
7879
<fileset dir="pkix/src/main/jdk1.5" includes="**/*.java" />

docs/releasenotes.html

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,7 @@ <h3>2.1.2 Defects Fixed</h3>
115115
<li>Follow-up to CVE-2026-5588: the legacy id_alg_composite CompositeVerifier (org.bouncycastle.operator.jcajce.JcaContentVerifierProviderBuilder, used by X509CertificateHolder.isSignatureValid, CMS and TSP) iterated the component count from the supplied signature sequence rather than from the key, so although the earlier fix rejected an empty signature sequence, a composite signature truncated to a verifying prefix (e.g. stripped of its post-quantum or its classical component) still verified. The verifier now requires the signature to carry exactly one component for every component key, rejecting both under- and over-length composite signatures. The final-draft (IANA-OID) composite path was unaffected — it parses a fixed-length concatenation and already verifies every component.</li>
116116
<li>The XMSS and XMSS^MT public-key converters in PublicKeyFactory (the raw RFC 9802 / RFC 8391 SubjectPublicKeyInfo form, which prefixes the key material with a 4-octet parameter-set OID) leaked a RuntimeException out of the createKey / BouncyCastleProvider.getPublicKey decode path on a malformed key: a public key shorter than the 4-octet OID prefix threw ArrayIndexOutOfBoundsException, and one carrying a valid parameter-set OID but truncated root/seed material threw IllegalArgumentException ("public key has wrong size"). Both are reachable when decoding an untrusted certificate, PKCS#8 or SubjectPublicKeyInfo through the BC provider. The converters now reject such input with an IOException, matching the declared contract and the strict-parse behaviour of the neighbouring stateful-hash converters.</li>
117117
<li>BouncyCastleProvider.getPublicKey(SubjectPublicKeyInfo) and getPrivateKey(PrivateKeyInfo) — the provider's internal entry points for turning a decoded key into a JCA PublicKey / PrivateKey, reached when parsing an untrusted certificate, PKCS#12 / BCFKS keystore entry or wrapped key — could leak a RuntimeException (typically a NullPointerException from a key-info converter handed a structurally degenerate SubjectPublicKeyInfo / PrivateKeyInfo, for example one with empty or truncated key material) out of their declared IOException contract. Both methods now catch a converter's RuntimeException and re-throw it as an IOException with the original cause attached, so a malformed key surfaces as the checked exception callers already handle rather than an uncaught crash. The per-algorithm converters are unchanged; this is a defence-in-depth guard at the shared decode boundary.</li>
118+
<li>S/MIME streaming operations buffer decrypted, decompressed and signed plaintext to backing temp files. These were created with File.createTempFile, which honours the process umask and so typically leaves the files world-readable (mode 0644) on POSIX systems, allowing other local users on a shared host to read the plaintext before cleanup. The temp files created by SMIMEUtil and SMIMESignedParser are now created through Files.createTempFile, which restricts them to the owner (0600 on POSIX, owner-only ACL on Windows). The java.nio.file call is isolated in a package-private TempFileFactory, so the legacy pre-Java-7 builds - where java.nio.file is unavailable - fall back to the historical File.createTempFile via jdk1.4 / jdk1.5 source overrides (issue #2326).</li>
118119
</ul>
119120
<h3>2.2.3 Additional Features and Functionality</h3>
120121
<ul>

mail/src/main/java/org/bouncycastle/mail/smime/SMIMESignedParser.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ private static File getTmpFile()
7272
{
7373
try
7474
{
75-
return File.createTempFile("bcMail", ".mime");
75+
return TempFileFactory.createTempFile("bcMail", ".mime");
7676
}
7777
catch (IOException e)
7878
{
@@ -278,7 +278,7 @@ public static SMIMESignedParser getSafeInstance(DigestCalculatorProvider digCalc
278278
{
279279
try
280280
{
281-
File tmpFile = File.createTempFile("bcSigned", ".tmp");
281+
File tmpFile = TempFileFactory.createTempFile("bcSigned", ".tmp");
282282
return getSafeInstance(digCalcProvider, message, "7bit", tmpFile);
283283
}
284284
catch (IOException e)

mail/src/main/java/org/bouncycastle/mail/smime/SMIMEUtil.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -560,7 +560,7 @@ static FileBackedMimeBodyPart toWriteOnceBodyPart(
560560
{
561561
try
562562
{
563-
return new WriteOnceFileBackedMimeBodyPart(content.getContentStream(), File.createTempFile("bcMail", ".mime"));
563+
return new WriteOnceFileBackedMimeBodyPart(content.getContentStream(), TempFileFactory.createTempFile("bcMail", ".mime"));
564564
}
565565
catch (IOException e)
566566
{
@@ -581,7 +581,7 @@ public static FileBackedMimeBodyPart toMimeBodyPart(
581581
{
582582
try
583583
{
584-
return toMimeBodyPart(content, File.createTempFile("bcMail", ".mime"));
584+
return toMimeBodyPart(content, TempFileFactory.createTempFile("bcMail", ".mime"));
585585
}
586586
catch (IOException e)
587587
{
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
package org.bouncycastle.mail.smime;
2+
3+
import java.io.File;
4+
import java.io.IOException;
5+
import java.nio.file.Files;
6+
7+
/**
8+
* Creates the backing temp files used for S/MIME streaming content.
9+
* <p>
10+
* This (Java 7+) version goes through {@link Files#createTempFile} so the file is created with
11+
* owner-only permissions (0600 on POSIX, owner-only ACL on Windows). {@link File#createTempFile}
12+
* honours the umask and so typically leaves the file world-readable, which matters here because
13+
* the temp files hold decrypted / decompressed / signed plaintext. The legacy {@code jdk1.4} and
14+
* {@code jdk1.5} source trees override this class with a {@code File.createTempFile} fallback for
15+
* the pre-Java-7 builds, where {@code java.nio.file} is unavailable.
16+
*/
17+
class TempFileFactory
18+
{
19+
static File createTempFile(String prefix, String suffix)
20+
throws IOException
21+
{
22+
return Files.createTempFile(prefix, suffix).toFile();
23+
}
24+
}
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
package org.bouncycastle.mail.smime;
2+
3+
import java.io.File;
4+
import java.io.IOException;
5+
6+
/**
7+
* Pre-Java-7 fallback used by the legacy (jdk1.3/jdk1.4) builds, where {@code java.nio.file} is
8+
* unavailable. {@link File#createTempFile} honours the umask, so the owner-only permissions the
9+
* Java 7+ version obtains from {@code Files.createTempFile} cannot be applied here; behaviour
10+
* matches the historical S/MIME temp-file handling on those JDKs.
11+
*/
12+
class TempFileFactory
13+
{
14+
static File createTempFile(String prefix, String suffix)
15+
throws IOException
16+
{
17+
return File.createTempFile(prefix, suffix);
18+
}
19+
}
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
package org.bouncycastle.mail.smime;
2+
3+
import java.io.File;
4+
import java.io.IOException;
5+
6+
/**
7+
* Pre-Java-7 fallback used by the legacy (jdk1.5/jdk1.6) builds, where {@code java.nio.file} is
8+
* unavailable. {@link File#createTempFile} honours the umask, so the owner-only permissions the
9+
* Java 7+ version obtains from {@code Files.createTempFile} cannot be applied here; behaviour
10+
* matches the historical S/MIME temp-file handling on those JDKs.
11+
*/
12+
class TempFileFactory
13+
{
14+
static File createTempFile(String prefix, String suffix)
15+
throws IOException
16+
{
17+
return File.createTempFile(prefix, suffix);
18+
}
19+
}

0 commit comments

Comments
 (0)