Fix test_pkcs12.rb in FIPS.

[junaruga]

This PR is working in progress.

I see the test failures on LibreSSL and AWS-LC cases.

---

• Use the AES-256-CBC using PBKDF2 which is FIPS-approved, instead of the PBE-SHA1-3DES using PKCS12KDF which is not FIPS-approved. See also the man page openssl-pkcs12(1).
• OpenSSL::PKCS12.create calling the PKCS12_create has the argument mac_iter which uses a MAC key using PKCS12KDF which is not FIPS-approved. In the FIPS case, set the mac_iter = -1 to omit the MAC key. See also the man page PKCS12_create(3).
• As the test data OpenSSL::PKCS12.new calling PKCS12_parse verifies the MAC using PKCS12KDF which is not FIPS-approved, I created the test data without MAC by the openssl pkcs12 -nomac.

[rhenium]

I feel these existing tests should simply be skipped. My impression is that PKCS#12 is currently effectively unsupported in FIPS environments as I doubt password-based PKCS#12 is actually used without a MAC.

I think PKCS12_create() allows omitting the MAC mainly so that users can set a MAC with non-default parameters later, for example as shown in #772.

On a side note, RFC 9579 / RFC 9879 introduced an alternative MAC algorithm based on PBMAC1 from PKCS#5v2, which I think is FIPS-compatible. However, this is still pretty new and we don't have a wrapper for it yet: https://docs.openssl.org/master/man3/PKCS12_gen_mac/

[junaruga]

I feel these existing tests should simply be skipped. My impression is that PKCS#12 is currently effectively unsupported in FIPS environments as I doubt password-based PKCS#12 is actually used without a MAC.

Okay. That makes sense. I think we want to test popular use cases.

I rebased the PR without MAC less key.

I am still using the AES-256-CBC as much as possible. Because I feel the AES-256-CBC is more modern and more typical. The AES-256-CBC is also used as openssl pkcs12's default algorithm according to the man page openssl-pkcs12(1).

However, LibreSSL's OpenSSL::PKCS12.create didn't accept the pass: nil in the case of the AES-256-CBC in the test_create_no_pass. The folllowing LibreSSL man page PKCS12_newpass shows a hint not to accept the NULL.

https://man.openbsd.org/PKCS12_newpass.3

If the PKCS#12 structure does not have a password, use the empty string "" for oldpass. Passing NULL for oldpass results in a PKCS12_newpass() failure.

And AWS-LC's OpenSSL::PKCS12.create raised "UNKNOWN_ALGORITHM" in AWS-LC with AES-256-CBC. So, I added some conditional logic.

If you don't like this conditional logic, I can change this PR's source code back to use the PBE-SHA1-3DES as value of the DEFAULT_PBE_PKEYS and DEFAULT_PBE_CERTS used in the master branch.

What do you think?

[rhenium]

To be precise, PBE-SHA1-3DES uses PBKDF1 in PKCS#5 rather than PKCS12KDF. PKCS12KDF is only used for the outer MAC. I'd drop the FIPS reference because it's not relevant.

I noticed you updated the base64-encoded examples to use PBES2, and these tests are passing even with AWS-LC. Did we run into an incompatibility in PKCS12_create()? If so, it might be better to just skip these tests.

[junaruga]

To be precise, PBE-SHA1-3DES uses PBKDF1 in PKCS#5 rather than PKCS12KDF. PKCS12KDF is only used for the outer MAC. I'd drop the FIPS reference because it's not relevant.

I would agree on removing the conditional parts, keeping the original parts. Because the entire content of this file is not relevant to FIPS. However, I still don't understand your opinion PBE-SHA1-3DES uses PBKDF1 in PKCS#5 rather than PKCS12KDF. Because when I added the following test test_foo, and ran it in FIPS, I got the following error message "error:0308010C:digital envelope routines:inner_evp_generic_fetch:unsupported (Global default library context, Algorithm (PKCS12KDF : 0), Properties ())" which indicated the PKCS12KDF was unsupported in the case. As I did set the mac_iter as -1 not to use the MAC key, I don't think this error comes from the MAC key. I think it comes from the PBE-SHA1-3DES itself. What do you think?

test_pkcs12.rb

...
    def test_foo
      OpenSSL.debug = true

      pkcs12 = OpenSSL::PKCS12.create(
        "omg",
        "hello",
        @mykey,
        @mycert,
        nil,
        "PBE-SHA1-3DES",
        "PBE-SHA1-3DES",
        nil,
        -1,
      )
    end
...

$ echo $OPENSSL_DIR
/home/jaruga/.local/openssl-4.0.0-dev-fips-debug-1cb0d36b39

$ $OPENSSL_DIR/bin/openssl version
OpenSSL 4.0.0-dev  (Library: OpenSSL 4.0.0-dev )

$ OPENSSL_CONF=$OPENSSL_DIR/ssl/openssl_fips.cnf \
  ruby test/openssl/test_pkcs12.rb --name=test_foo
Loaded suite test/openssl/test_pkcs12
Started
test/openssl/test_pkcs12.rb:39: warning: error on stack: error:0308010C:digital envelope routines:inner_evp_generic_fetch:unsupported (Global default library context, Algorithm (PKCS12KDF : 0), Properties (<null>))
test/openssl/test_pkcs12.rb:39: warning: error on stack: error:1180006B:PKCS12 routines:PKCS12_PBE_keyivgen_ex:key gen error
test/openssl/test_pkcs12.rb:39: warning: error on stack: error:11800067:PKCS12 routines:PKCS12_item_i2d_encrypt_ex:encrypt error
test/openssl/test_pkcs12.rb:39: warning: error on stack: error:11800067:PKCS12 routines:PKCS12_pack_p7encdata_ex:encrypt error
E
========================================================================================
Error: test_foo(OpenSSL::TestPKCS12): OpenSSL::PKCS12::PKCS12Error: encrypt error
test/openssl/test_pkcs12.rb:39:in 'OpenSSL::PKCS12.create'
test/openssl/test_pkcs12.rb:39:in 'OpenSSL::TestPKCS12#test_foo'
     36:     def test_foo
     37:       OpenSSL.debug = true
     38:
  => 39:       pkcs12 = OpenSSL::PKCS12.create(
     40:         "omg",
     41:         "hello",
     42:         @mykey,
========================================================================================
Finished in 0.044698029 seconds.
----------------------------------------------------------------------------------------
1 tests, 1 assertions, 0 failures, 1 errors, 0 pendings, 0 omissions, 0 notifications
0% passed
----------------------------------------------------------------------------------------
22.37 tests/s, 22.37 assertions/s

I noticed you updated the base64-encoded examples to use PBES2, and these tests are passing even with AWS-LC. Did we run into an incompatibility in PKCS12_create()? If so, it might be better to just skip these tests.

Yes, good point. The based64-encoded texts are based on the AES-256-CBC in the test_new_with_no_keys and test_new_with_no_certs. However, the tests passes using the PBE-SHA1-3DES in AWS-LC. It seemed weird to me. But I assumed this might be just a specification.

Anyway, I will revert this part to the original logic.

[junaruga]

I debugged the following error with gdb. I see this calls the PKCS12_key_gen_uni_ex, and failing to fetch PKCS12KDF in it. So,

$ OPENSSL_CONF=$OPENSSL_DIR/ssl/openssl_fips.cnf \
  gdb --args ruby test/openssl/test_pkcs12.rb --name=test_foo

(gdb) f
#0  PKCS12_key_gen_uni_ex (pass=0xde6ba0 "", passlen=8, salt=0xde6b60 "#\365]\304\r=\023R", saltlen=8, id=1, iter=2048, n=24, out=0x7fffffff5070 "\220P\377\377\377\177", md_type=0xd8ea40, libctx=0x0, propq=0x0)
    at crypto/pkcs12/p12_key.c:93
93	    if (kdf == NULL)
(gdb) l
88	
89	    if (n <= 0)
90	        return 0;
91	
92	    kdf = EVP_KDF_fetch(libctx, "PKCS12KDF", propq);
93	    if (kdf == NULL)
94	        return 0;
95	    ctx = EVP_KDF_CTX_new(kdf);
96	    EVP_KDF_free(kdf);
97	    if (ctx == NULL)
(gdb) p kdf
$3 = (EVP_KDF *) 0x0

(gdb) bt
#0  PKCS12_key_gen_uni_ex (pass=0xde6ba0 "", passlen=8, salt=0xde6b60 "#\365]\304\r=\023R", saltlen=8, id=1, iter=2048, n=24, out=0x7fffffff5070 "\220P\377\377\377\177", md_type=0xd8ea40, libctx=0x0, propq=0x0)
    at crypto/pkcs12/p12_key.c:93
#1  0x00007fffcd8a6468 in PKCS12_key_gen_utf8_ex (pass=0x7fffce0afe20 "omg", passlen=3, salt=0xde6b60 "#\365]\304\r=\023R", saltlen=8, id=1, iter=2048, n=24, out=0x7fffffff5070 "\220P\377\377\377\177", md_type=0xd8ea40,
    ctx=0x0, propq=0x0) at crypto/pkcs12/p12_key.c:65
#2  0x00007fffcd8a4529 in PKCS12_PBE_keyivgen_ex (ctx=0xde6a40, pass=0x7fffce0afe20 "omg", passlen=3, param=0xde4c80, cipher=0xde4890, md=0xd8ea40, en_de=1, libctx=0x0, propq=0x0) at crypto/pkcs12/p12_crpt.c:51
#3  0x00007fffcd77646a in EVP_PBE_CipherInit_ex (pbe_obj=0x7fffcdc85b90 <nid_objs+5840>, pass=0x7fffce0afe20 "omg", passlen=3, param=0xde4c80, ctx=0xde6a40, en_de=1, libctx=0x0, propq=0x0) at crypto/evp/evp_pbe.c:158
#4  0x00007fffcd8a5762 in PKCS12_pbe_crypt_ex (algor=0xde4ca0, pass=0x7fffce0afe20 "omg", passlen=-1,
    in=0xde6470 "0\202\005\3000\202\005\274\006\v*\206H\206\367\r\001\f\n\001\003\240\202\005i0\202\005e\006\n*\206H\206\367\r\001\t\026\001\240\202\005U\004\202\005Q0\202\005M0\202\0035\240\003\002\001\002\002\001\0030\r\006\t*\206H\206\367\r\001\001\v\005", inlen=1476, data=0xdd02e8, datalen=0xdd02e0, en_de=1, libctx=0x0, propq=0x0) at crypto/pkcs12/p12_decr.c:37
#5  0x00007fffcd8a5e9d in PKCS12_item_i2d_encrypt_ex (algor=0xde4ca0, it=0x7fffcdc93c00 <local_it>, pass=0x7fffce0afe20 "omg", passlen=-1, obj=0xdcf280, zbuf=1, ctx=0x0, propq=0x0) at crypto/pkcs12/p12_decr.c:202
#6  0x00007fffcd8a3841 in PKCS12_pack_p7encdata_ex (pbe_nid=146, pass=0x7fffce0afe20 "omg", passlen=-1, salt=0x0, saltlen=0, iter=2048, bags=0xdcf280, ctx=0x0, propq=0x0) at crypto/pkcs12/p12_add.c:132
#7  0x00007fffcd8a53ad in PKCS12_add_safe_ex (psafes=0x7fffffff5470, bags=0xdcf280, nid_safe=146, iter=2048, pass=0x7fffce0afe20 "omg", ctx=0x0, propq=0x0) at crypto/pkcs12/p12_crt.c:323
#8  0x00007fffcd8a4beb in PKCS12_create_ex2 (pass=0x7fffce0afe20 "omg", name=0x7fffce0afdf8 "hello", pkey=0xc2f6b0, cert=0xdcddc0, ca=0x0, nid_key=146, nid_cert=146, iter=2048, mac_iter=-1, keytype=0, ctx=0x0, propq=0x0,
    cb=0x0, cbarg=0x0) at crypto/pkcs12/p12_crt.c:111
#9  0x00007fffcd8a4f76 in PKCS12_create_ex (pass=0x7fffce0afe20 "omg", name=0x7fffce0afdf8 "hello", pkey=0xc2f6b0, cert=0xdcddc0, ca=0x0, nid_key=146, nid_cert=146, iter=0, mac_iter=-1, keytype=0, ctx=0x0, propq=0x0)
    at crypto/pkcs12/p12_crt.c:176
#10 0x00007fffcd8a4fd0 in PKCS12_create (pass=0x7fffce0afe20 "omg", name=0x7fffce0afdf8 "hello", pkey=0xc2f6b0, cert=0xdcddc0, ca=0x0, nid_key=146, nid_cert=146, iter=0, mac_iter=-1, keytype=0)
    at crypto/pkcs12/p12_crt.c:185
#11 0x00007fffce041459 in ossl_pkcs12_s_create (argc=9, argv=0x7fffe92ff828, self=140736648167080) at ossl_pkcs12.c:150
...

Below is the part.

https://github.com/openssl/openssl/blob/1cb0d36b39f69367d63e940976faaa2c252763b4/crypto/pkcs12/p12_key.c#L92-L94

[rhenium]

You're right. PKCS#12 was more complicated than I had always assumed. https://datatracker.ietf.org/doc/html/rfc7292#appendix-C says:

The PBES1 encryption scheme defined in PKCS [#]5 provides a number of algorithm identifiers for deriving keys and IVs; here, we specify a few more, all of which use the procedure detailed in Appendices B.2 and B.3 to construct keys (and IVs, where needed).
[...]
pbeWithSHAAnd3-KeyTripleDES-CBC OBJECT IDENTIFIER ::= {pkcs-12PbeIds 3}

I also realized that PBKDF1 can't be used for 3DES in the first place due to its larger key size, so PKCS#12 had to use something else.

Thanks for pointing this out.

Anyway, I will revert this part to the original logic.

I'm fine with either keeping AES only or 3DES only. I just don't feel we need to test different things with different OpenSSL variants.

[junaruga]

I reverted this conditional logic.

Note that I wrote about the above error with the OpenSSL::PKCS12.create with key_pbe: PBE-SHA1-3DES, cert_pbe: PBE-SHA1-3DES, mac_iter: -1 into the comment and commit message.

[junaruga]

Now I noticed you added your comment 2 hours ago. Let me check the comment tomorrow.

[rhenium]

However, LibreSSL's OpenSSL::PKCS12.create didn't accept the pass: nil in the case of the AES-256-CBC in the test_create_no_pass. The folllowing LibreSSL man page PKCS12_newpass shows a hint not not to accept the NULL.

https://man.openbsd.org/PKCS12_newpass.3

This is interesting. It appears that the underlying function in OpenSSL, EVP_PBE_CipherInit_ex() accepts NULL to mean an empty password, as an undocumented behavior. It doesn't make sense to me, but I think it might be just for historical reason.

I also think using PBES2/AES is better, but I'd prefer to reduce conditionals because I'm not very familiar with PKCS#12 and want to keep it simple.

[junaruga]

I also think using PBES2/AES is better, but I'd prefer to reduce conditionals because I'm not very familiar with PKCS#12 and want to keep it simple.

Yes your preference makes sense to me. I will revert the conditional logic to the original logic.

[junaruga]

I rebased this PR, fixing mentioned from you. Now there are 2 commits in the PR. The first commit is to the main. The 2nd commit is to update the comments. Could you review the PR again? Thanks.