[refactor] clear PKCS12 password arrays after use

kares · claude · kares · commit 476d375e92a8 · 2026-03-22T19:29:41.000+01:00
Convert Ruby password strings to char[] via ByteList + Charset.decode,
bypassing the immutable Java String that .to_java.to_char_array creates.

Clear the char[] in ensure blocks after KeyStore operations, matching
C OpenSSL's expectation that the caller manages password lifetime.

Co-Authored-By: Claude Opus 4.6 &lt;noreply@anthropic.com&gt;
diff --git a/lib/openssl/pkcs12.rb b/lib/openssl/pkcs12.rb
@@ -28,50 +28,55 @@ def initialize(str = nil, password = '')
         @der = str
       end
 
-      store = SecurityHelper.getKeyStore("PKCS12")
-      store.load(java.io.ByteArrayInputStream.new(@der.to_java_bytes), password.to_java.to_char_array)
+      pass_chars = PEMUtils.toPasswordChars(password)
+      begin
+        store = SecurityHelper.getKeyStore("PKCS12")
+        store.load(java.io.ByteArrayInputStream.new(@der.to_java_bytes), pass_chars)
 
-      aliases = store.aliases
-      aliases.each do |alias_name|
-        if store.is_key_entry(alias_name)
-          if java_certificate = store.get_certificate(alias_name)
-            der = String.from_java_bytes(java_certificate.get_encoded)
-            @certificate = OpenSSL::X509::Certificate.new(der)
-          end
+        aliases = store.aliases
+        aliases.each do |alias_name|
+          if store.is_key_entry(alias_name)
+            if java_certificate = store.get_certificate(alias_name)
+              der = String.from_java_bytes(java_certificate.get_encoded)
+              @certificate = OpenSSL::X509::Certificate.new(der)
+            end
 
-          java_key = store.get_key(alias_name, password.to_java.to_char_array)
-          if java_key
-            der = String.from_java_bytes(java_key.get_encoded)
-            algorithm = java_key.get_algorithm
-            if algorithm == "RSA"
-              @key = OpenSSL::PKey::RSA.new(der)
-            elsif algorithm == "DSA"
-              @key = OpenSSL::PKey::DSA.new(der)
-            elsif algorithm == "DH"
-              @key = OpenSSL::PKey::DH.new(der)
-            elsif algorithm == "EC"
-              @key = OpenSSL::PKey::EC.new(der)
-            else
-              raise PKCS12Error, "Unknown key algorithm #{algorithm}"
+            java_key = store.get_key(alias_name, pass_chars)
+            if java_key
+              der = String.from_java_bytes(java_key.get_encoded)
+              algorithm = java_key.get_algorithm
+              if algorithm == "RSA"
+                @key = OpenSSL::PKey::RSA.new(der)
+              elsif algorithm == "DSA"
+                @key = OpenSSL::PKey::DSA.new(der)
+              elsif algorithm == "DH"
+                @key = OpenSSL::PKey::DH.new(der)
+              elsif algorithm == "EC"
+                @key = OpenSSL::PKey::EC.new(der)
+              else
+                raise PKCS12Error, "Unknown key algorithm #{algorithm}"
+              end
             end
-          end
 
-          @ca_certs = Array.new
-          java_ca_certs = store.get_certificate_chain(alias_name)
-          if java_ca_certs
-            java_ca_certs.each do |java_ca_cert|
-                der = String.from_java_bytes(java_ca_cert.get_encoded)
-                ruby_cert = OpenSSL::X509::Certificate.new(der)
-                if (ruby_cert.to_pem != @certificate.to_pem)
-                  @ca_certs << ruby_cert
-                end
+            @ca_certs = Array.new
+            java_ca_certs = store.get_certificate_chain(alias_name)
+            if java_ca_certs
+              java_ca_certs.each do |java_ca_cert|
+                  der = String.from_java_bytes(java_ca_cert.get_encoded)
+                  ruby_cert = OpenSSL::X509::Certificate.new(der)
+                  if (ruby_cert.to_pem != @certificate.to_pem)
+                    @ca_certs << ruby_cert
+                  end
+              end
             end
+            break
           end
-          break
         end
+      rescue java.lang.Exception => e
+        raise PKCS12Error, e.inspect
+      ensure
+        PEMUtils.clearChars(pass_chars)
       end
-    rescue java.lang.Exception => e
-      raise PKCS12Error, e.inspect
     end
 
     def generate(pass, alias_name, key, cert, ca = nil)
@@ -80,15 +85,18 @@ def generate(pass, alias_name, key, cert, ca = nil)
       certificates = cert.to_pem
       ca.each { |ca_cert| certificates << ca_cert.to_pem } if ca
 
+      pass_chars = PEMUtils.toPasswordChars(pass.nil? ? "" : pass)
       begin
         der_bytes = PEMUtils.generatePKCS12(
           java.io.StringReader.new(key.to_pem), certificates.to_java_bytes,
-          alias_name, ( pass.nil? ? "" : pass ).to_java.to_char_array
+          alias_name, pass_chars
         )
       rescue java.security.KeyStoreException, java.security.cert.CertificateException => e
         raise PKCS12Error, e.message
       rescue java.security.GeneralSecurityException, java.io.IOException => e
         raise PKCS12Error, e.inspect
+      ensure
+        PEMUtils.clearChars(pass_chars)
       end
 
       @der = String.from_java_bytes(der_bytes)
diff --git a/src/main/java/org/jruby/ext/openssl/PEMUtils.java b/src/main/java/org/jruby/ext/openssl/PEMUtils.java
@@ -27,6 +27,10 @@
 import java.io.IOException;
 import java.io.Reader;
 import java.io.Writer;
+import java.nio.ByteBuffer;
+import java.nio.CharBuffer;
+import java.nio.charset.Charset;
+import java.nio.charset.StandardCharsets;
 import java.security.GeneralSecurityException;
 
 import java.security.Key;
@@ -39,6 +43,7 @@
 import java.security.spec.AlgorithmParameterSpec;
 import java.security.spec.PKCS8EncodedKeySpec;
 import java.security.spec.X509EncodedKeySpec;
+import java.util.Arrays;
 import java.util.Collection;
 import javax.crypto.SecretKey;
 import javax.crypto.spec.IvParameterSpec;
@@ -59,6 +64,10 @@
 import org.bouncycastle.openssl.PEMWriter;
 import org.bouncycastle.operator.OperatorCreationException;
 
+import org.jruby.RubyString;
+import org.jruby.runtime.builtin.IRubyObject;
+import org.jruby.util.ByteList;
+
 import org.jruby.ext.openssl.impl.pem.MiscPEMGeneratorHelper;
 import org.jruby.ext.openssl.util.ByteArrayOutputStream;
 //import org.bouncycastle.util.io.pem.PemReader;
@@ -72,6 +81,58 @@
  */
 public abstract class PEMUtils {
 
+    /**
+     * Convert a Ruby string to a password char[] without creating an intermediate
+     * Java String (which would be immutable and unclearable in memory).
+     * <p>
+     * This matches C OpenSSL's password handling where the password is used
+     * in-place from the caller's buffer ({@code const char *pass}) with no
+     * intermediate immutable copy.
+     * <p>
+     * The caller can clear the returned array after use via {@link #clearChars}.
+     *
+     * @param passwd a Ruby string (or nil)
+     * @return password as char[], never null
+     */
+    public static char[] toPasswordChars(final IRubyObject passwd) {
+        if (passwd == null || passwd.isNil()) return new char[0];
+
+        final RubyString str = passwd.convertToString();
+        final ByteList byteList = str.getByteList();
+
+        return toPasswordChars(
+            byteList.getUnsafeBytes(), byteList.getBegin(), byteList.getRealSize(),
+            byteList.getEncoding().getCharset()
+        );
+    }
+
+    // Package-private for testing
+    static char[] toPasswordChars(final byte[] bytes, final int offset, final int length, Charset charset) {
+        if (length == 0) return new char[0];
+
+        if (charset == null) charset = StandardCharsets.ISO_8859_1;
+
+        final ByteBuffer byteBuf = ByteBuffer.wrap(bytes, offset, length);
+        final CharBuffer charBuf = charset.decode(byteBuf);
+
+        final char[] result = new char[charBuf.remaining()];
+        charBuf.get(result);
+
+        // Clear the decoder's intermediate buffer
+        if (charBuf.hasArray()) {
+            Arrays.fill(charBuf.array(), '\0');
+        }
+
+        return result;
+    }
+
+    /**
+     * Zero-fill a password char array. Safe to call with null.
+     */
+    public static void clearChars(final char[] chars) {
+        if (chars != null) Arrays.fill(chars, '\0');
+    }
+
     /*
     private static boolean bcPEMParser;
     private static Class<?> pemReaderImpl;
diff --git a/src/test/java/org/jruby/ext/openssl/PEMUtilsPasswordTest.java b/src/test/java/org/jruby/ext/openssl/PEMUtilsPasswordTest.java
@@ -0,0 +1,120 @@
+package org.jruby.ext.openssl;
+
+import java.nio.charset.Charset;
+import java.nio.charset.StandardCharsets;
+
+import org.junit.*;
+import static org.junit.Assert.*;
+
+/**
+ * Tests for {@link PEMUtils#toPasswordChars} and {@link PEMUtils#clearChars},
+ * verifying password handling matches C OpenSSL's approach:
+ * - No intermediate immutable Java String created
+ * - Intermediate buffers are cleared after conversion
+ * - Caller can (and must) clear the returned char[]
+ */
+public class PEMUtilsPasswordTest {
+
+    // C OpenSSL: const char *pass with ASCII bytes
+    @Test
+    public void asciiPassword() {
+        byte[] bytes = "secret".getBytes(StandardCharsets.US_ASCII);
+        char[] result = PEMUtils.toPasswordChars(bytes, 0, bytes.length, StandardCharsets.US_ASCII);
+        assertArrayEquals(new char[] { 's', 'e', 'c', 'r', 'e', 't' }, result);
+    }
+
+    // C OpenSSL: empty password (pass = "")
+    @Test
+    public void emptyPassword() {
+        char[] result = PEMUtils.toPasswordChars(new byte[0], 0, 0, StandardCharsets.UTF_8);
+        assertNotNull(result);
+        assertEquals(0, result.length);
+    }
+
+    // C OpenSSL: NULL password maps to empty char[]
+    @Test
+    public void nullRubyObjectReturnsEmpty() {
+        char[] result = PEMUtils.toPasswordChars(null);
+        assertNotNull(result);
+        assertEquals(0, result.length);
+    }
+
+    // UTF-8 multi-byte: "café" should decode correctly
+    @Test
+    public void utf8Password() {
+        byte[] bytes = "caf\u00e9".getBytes(StandardCharsets.UTF_8);
+        // UTF-8 encoding of é is 0xC3 0xA9 (2 bytes), so 5 bytes total
+        assertEquals(5, bytes.length);
+        char[] result = PEMUtils.toPasswordChars(bytes, 0, bytes.length, StandardCharsets.UTF_8);
+        assertArrayEquals(new char[] { 'c', 'a', 'f', '\u00e9' }, result);
+    }
+
+    // JRuby's ASCII-8BIT (binary): getCharset() returns null, should fall back to ISO-8859-1
+    @Test
+    public void nullCharsetFallsBackToLatin1() {
+        byte[] bytes = new byte[] { (byte) 0xC0, (byte) 0xFF, (byte) 0x41 };
+        char[] result = PEMUtils.toPasswordChars(bytes, 0, bytes.length, (Charset) null);
+        assertEquals(3, result.length);
+        assertEquals('\u00C0', result[0]); // 0xC0 -> U+00C0
+        assertEquals('\u00FF', result[1]); // 0xFF -> U+00FF
+        assertEquals('A', result[2]);       // 0x41 -> 'A'
+    }
+
+    // ISO-8859-1 encoding
+    @Test
+    public void latin1Password() {
+        byte[] bytes = new byte[] { 'p', 'a', (byte) 0xDF, (byte) 0xF1 }; // "paßñ"
+        char[] result = PEMUtils.toPasswordChars(bytes, 0, bytes.length, StandardCharsets.ISO_8859_1);
+        assertArrayEquals(new char[] { 'p', 'a', '\u00DF', '\u00F1' }, result);
+    }
+
+    // Offset and length: only slice of buffer is converted
+    @Test
+    public void offsetAndLength() {
+        byte[] bytes = "XXhelloXX".getBytes(StandardCharsets.US_ASCII);
+        char[] result = PEMUtils.toPasswordChars(bytes, 2, 5, StandardCharsets.US_ASCII);
+        assertArrayEquals(new char[] { 'h', 'e', 'l', 'l', 'o' }, result);
+    }
+
+    // clearChars zeroes the array
+    @Test
+    public void clearCharsZeroesArray() {
+        char[] password = "secret".toCharArray();
+        PEMUtils.clearChars(password);
+        for (char c : password) {
+            assertEquals('\0', c);
+        }
+    }
+
+    // clearChars is safe with null
+    @Test
+    public void clearCharsHandlesNull() {
+        PEMUtils.clearChars(null); // should not throw
+    }
+
+    // Returned array is independent — clearing it doesn't affect the source
+    @Test
+    public void returnedArrayIsACopy() {
+        byte[] bytes = "test".getBytes(StandardCharsets.US_ASCII);
+        char[] result = PEMUtils.toPasswordChars(bytes, 0, bytes.length, StandardCharsets.US_ASCII);
+        PEMUtils.clearChars(result);
+        // Source bytes are unchanged
+        assertEquals('t', (char) bytes[0]);
+        assertEquals('e', (char) bytes[1]);
+    }
+
+    // Typical PKCS12 round-trip: password chars can be used with KeyStore
+    @Test
+    public void passwordCharsCompatibleWithKeyStore() throws Exception {
+        byte[] passBytes = "pkcs12pass".getBytes(StandardCharsets.UTF_8);
+        char[] passChars = PEMUtils.toPasswordChars(passBytes, 0, passBytes.length, StandardCharsets.UTF_8);
+        try {
+            // Verify the char[] works with Java KeyStore API
+            java.security.KeyStore ks = java.security.KeyStore.getInstance("PKCS12");
+            ks.load(null, passChars); // initialize empty store with our password
+            // No exception means the password format is valid
+        } finally {
+            PEMUtils.clearChars(passChars);
+        }
+    }
+}
diff --git a/src/test/ruby/pkcs12/test_pkcs12.rb b/src/test/ruby/pkcs12/test_pkcs12.rb
@@ -0,0 +1,71 @@
+# coding: US-ASCII
+require File.expand_path('../test_helper', File.dirname(__FILE__))
+
+class TestPKCS12 < TestCase
+
+  def setup
+    super
+
+    @key = OpenSSL::PKey::RSA.new(2048)
+    @cert = issue_cert
+  end
+
+  def test_create_and_parse_with_password
+    p12 = OpenSSL::PKCS12.create("secret", "myalias", @key, @cert)
+    assert p12.to_der.bytesize > 0
+
+    parsed = OpenSSL::PKCS12.new(p12.to_der, "secret")
+    assert_instance_of OpenSSL::PKey::RSA, parsed.key
+    assert_equal @cert.subject.to_s, parsed.certificate.subject.to_s
+  end
+
+  def test_create_and_parse_with_empty_password
+    p12 = OpenSSL::PKCS12.create("", "myalias", @key, @cert)
+    parsed = OpenSSL::PKCS12.new(p12.to_der, "")
+    assert_instance_of OpenSSL::PKey::RSA, parsed.key
+    assert_equal @cert.subject.to_s, parsed.certificate.subject.to_s
+  end
+
+  def test_create_and_parse_with_nil_password
+    p12 = OpenSSL::PKCS12.create(nil, "myalias", @key, @cert)
+    parsed = OpenSSL::PKCS12.new(p12.to_der)
+    assert_instance_of OpenSSL::PKey::RSA, parsed.key
+    assert_equal @cert.subject.to_s, parsed.certificate.subject.to_s
+  end
+
+  def test_parse_with_wrong_password_raises
+    p12 = OpenSSL::PKCS12.create("right", "myalias", @key, @cert)
+    assert_raise(OpenSSL::PKCS12::PKCS12Error) do
+      OpenSSL::PKCS12.new(p12.to_der, "wrong")
+    end
+  end
+
+  def test_create_and_parse_with_ca_certs
+    ca_key = OpenSSL::PKey::RSA.new(2048)
+    ca_cert = issue_cert(cn: "CA", key: ca_key)
+    leaf_cert = issue_cert(cn: "leaf", issuer: ca_cert, issuer_key: ca_key)
+
+    p12 = OpenSSL::PKCS12.create("pass", "myalias", @key, leaf_cert, [ca_cert])
+    parsed = OpenSSL::PKCS12.new(p12.to_der, "pass")
+    assert_equal leaf_cert.subject.to_s, parsed.certificate.subject.to_s
+    assert_equal 1, parsed.ca_certs.size
+    assert_equal ca_cert.subject.to_s, parsed.ca_certs.first.subject.to_s
+  end
+
+  private
+
+  def issue_cert(cn: "test", key: nil, issuer: nil, issuer_key: nil)
+    key ||= @key
+    cert = OpenSSL::X509::Certificate.new
+    cert.version = 2
+    cert.serial = 1
+    cert.subject = OpenSSL::X509::Name.parse("/CN=#{cn}")
+    cert.issuer = issuer ? issuer.subject : cert.subject
+    cert.not_before = Time.now
+    cert.not_after = Time.now + 3600
+    cert.public_key = key.public_key
+    cert.sign(issuer_key || key, OpenSSL::Digest::SHA256.new)
+    cert
+  end
+
+end
