Skip to content

Commit 4702868

Browse files
Fix test_create_with_mac_iter accidently setting keytype not maciter
This test was accidentally passing the value 2048 into the keytype parameter of PKCS12_create, not the mac_iter parameter (because it had one too many `nil`s in the call). This value is invalid, and will make OpenSSL perform an out-of-bounds read which is caught when compiling with ASAN. This commit fixes the tests, and also adds some validation to PKCS12.create to make sure any keytype passed is actually valid. Since there only two valid keytype constants, and the whole feature is an export-grade crypto era thing only ever supported by old MSIE, it seems far more likely that code in the whild is using keytype similarly by mistake rather than as intended. So this validation might catch that.
1 parent b1bfe8a commit 4702868

File tree

2 files changed

+38
-1
lines changed

2 files changed

+38
-1
lines changed

ext/openssl/ossl_pkcs12.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,10 @@ ossl_pkcs12_s_create(int argc, VALUE *argv, VALUE self)
134134
if (!NIL_P(keytype))
135135
ktype = NUM2INT(keytype);
136136

137+
if (ktype != 0 && ktype != KEY_SIG && ktype != KEY_EX) {
138+
ossl_raise(rb_eArgError, "Unknown key usage type %"PRIsVALUE, INT2NUM(ktype));
139+
}
140+
137141
obj = NewPKCS12(cPKCS12);
138142
x509s = NIL_P(ca) ? NULL : ossl_x509_ary2sk(ca);
139143
p12 = PKCS12_create(passphrase, friendlyname, key, x509, x509s,
@@ -272,4 +276,8 @@ Init_ossl_pkcs12(void)
272276
rb_attr(cPKCS12, rb_intern("ca_certs"), 1, 0, Qfalse);
273277
rb_define_method(cPKCS12, "initialize", ossl_pkcs12_initialize, -1);
274278
rb_define_method(cPKCS12, "to_der", ossl_pkcs12_to_der, 0);
279+
280+
/* MSIE specific PKCS12 key usage extensions */
281+
rb_define_const(cPKCS12, "KEY_EX", INT2NUM(KEY_EX));
282+
rb_define_const(cPKCS12, "KEY_SIG", INT2NUM(KEY_SIG));
275283
}

test/openssl/test_pkcs12.rb

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,6 @@ def test_create_with_mac_itr
159159
DEFAULT_PBE_PKEYS,
160160
DEFAULT_PBE_CERTS,
161161
nil,
162-
nil,
163162
2048
164163
)
165164

@@ -178,6 +177,36 @@ def test_create_with_mac_itr
178177
end
179178
end
180179

180+
def test_create_with_keytype
181+
OpenSSL::PKCS12.create(
182+
"omg",
183+
"hello",
184+
@mykey,
185+
@mycert,
186+
[],
187+
DEFAULT_PBE_PKEYS,
188+
DEFAULT_PBE_CERTS,
189+
nil,
190+
nil,
191+
OpenSSL::PKCS12::KEY_SIG
192+
)
193+
194+
assert_raise(ArgumentError) do
195+
OpenSSL::PKCS12.create(
196+
"omg",
197+
"hello",
198+
@mykey,
199+
@mycert,
200+
[],
201+
DEFAULT_PBE_PKEYS,
202+
DEFAULT_PBE_CERTS,
203+
nil,
204+
nil,
205+
2048
206+
)
207+
end
208+
end
209+
181210
def test_new_with_no_keys
182211
# generated with:
183212
# openssl pkcs12 -certpbe PBE-SHA1-3DES -in <@mycert> -nokeys -export

0 commit comments

Comments
 (0)