OpenSC / libp11

PKCS#11 wrapper library
GNU Lesser General Public License v2.1
309 stars 187 forks source link

RSA OAEP is broken #485

Closed lsh123 closed 1 year ago

lsh123 commented 1 year ago

Looks like it was implemented

https://github.com/OpenSC/libp11/commit/0e84a1ed601a5fd952d007328ea12a5925f423d7

and then removed in a completely unrelated change here:

https://github.com/OpenSC/libp11/commit/d216164760dde3ab7cbe809973c7ef4303b07616

and is still that way on master:

https://github.com/OpenSC/libp11/blob/master/src/p11_rsa.c#L57

jtalir commented 1 year ago

Any chance to look at this?

Here are the steps how reproduce an error on Fedora 37:

[root@test ~]# wget https://github.com/lsh123/xmlsec/files/10999653/test-rsa-oaep.zip
--2023-03-17 09:10:37--  https://github.com/lsh123/xmlsec/files/10999653/test-rsa-oaep.zip
Resolving github.com (github.com)... 140.82.121.4
Connecting to github.com (github.com)|140.82.121.4|:443... connected.
HTTP request sent, awaiting response... 302 Found
Location: https://objects.githubusercontent.com/github-production-repository-file-5c1aeb/50599822/10999653?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=AKIAIWNJYAX4CSVEH53A%2F20230317%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20230317T091038Z&X-Amz-Expires=300&X-Amz-Signature=70330e76e7c72902ca1e98739e5d200dd0dcb062842d28aaf35a00536601e5ea&X-Amz-SignedHeaders=host&actor_id=0&key_id=0&repo_id=50599822&response-content-disposition=attachment%3Bfilename%3Dtest-rsa-oaep.zip&response-content-type=application%2Fzip [following]
--2023-03-17 09:10:38--  https://objects.githubusercontent.com/github-production-repository-file-5c1aeb/50599822/10999653?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=AKIAIWNJYAX4CSVEH53A%2F20230317%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20230317T091038Z&X-Amz-Expires=300&X-Amz-Signature=70330e76e7c72902ca1e98739e5d200dd0dcb062842d28aaf35a00536601e5ea&X-Amz-SignedHeaders=host&actor_id=0&key_id=0&repo_id=50599822&response-content-disposition=attachment%3Bfilename%3Dtest-rsa-oaep.zip&response-content-type=application%2Fzip
Resolving objects.githubusercontent.com (objects.githubusercontent.com)... 185.199.111.133, 185.199.108.133, 185.199.109.133, ...
Connecting to objects.githubusercontent.com (objects.githubusercontent.com)|185.199.111.133|:443... connected.
HTTP request sent, awaiting response... 200 OK
Length: 1710 (1.7K) [application/zip]
Saving to: ‘test-rsa-oaep.zip’

test-rsa-oaep.zip               100%[=======================================================>]   1.67K  --.-KB/s    in 0s      

2023-03-17 09:10:39 (8.20 MB/s) - ‘test-rsa-oaep.zip’ saved [1710/1710]

[root@test ~]# unzip test-rsa-oaep.zip 
Archive:  test-rsa-oaep.zip
   creating: test-rsa-oaep/
  inflating: test-rsa-oaep/run.sh    
  inflating: test-rsa-oaep/source.xml  
  inflating: test-rsa-oaep/template.xml  
 extracting: test-rsa-oaep/softhsm.conf  
[root@test ~]# cd test-rsa-oaep
[root@test test-rsa-oaep]# ./run.sh 
Last metadata expiration check: 0:01:48 ago on Fri 17 Mar 2023 09:09:01 AM UTC.
Dependencies resolved.
================================================================================================================================
 Package                              Architecture             Version                          Repository                 Size
================================================================================================================================
Installing:
 opensc                               x86_64                   0.23.0-2.fc37                    updates                   1.3 M
 openssl                              x86_64                   1:3.0.8-1.fc37                   updates                   1.2 M
 openssl-pkcs11                       x86_64                   0.4.12-2.fc37                    fedora                     74 k
 softhsm                              x86_64                   2.6.1-5.fc37.5                   fedora                    444 k
 xmlsec1                              x86_64                   1.2.34-4.fc37                    updates                   196 k
 xmlsec1-openssl                      x86_64                   1.2.34-4.fc37                    updates                    96 k
Installing dependencies:
 libicu                               x86_64                   71.1-2.fc37                      fedora                     10 M
 libtool-ltdl                         x86_64                   2.4.7-2.fc37                     fedora                     37 k
 libusb1                              x86_64                   1.0.25-9.fc37                    fedora                     74 k
 libxslt                              x86_64                   1.1.37-1.fc37                    fedora                    184 k
 mozjs102                             x86_64                   102.8.0-1.fc37                   updates                   4.5 M
 pcsc-lite                            x86_64                   1.9.9-1.fc37                     fedora                     95 k
 pcsc-lite-ccid                       x86_64                   1.5.0-2.fc37                     fedora                    319 k
 pcsc-lite-libs                       x86_64                   1.9.9-1.fc37                     fedora                     28 k
 polkit                               x86_64                   121-4.fc37                       fedora                    156 k
 polkit-libs                          x86_64                   121-4.fc37                       fedora                     66 k
 polkit-pkla-compat                   x86_64                   0.1-22.fc37                      fedora                     44 k

Transaction Summary
================================================================================================================================
Install  17 Packages

Total download size: 19 M
Installed size: 59 M
Is this ok [y/N]: y
Downloading Packages:
(1/17): libtool-ltdl-2.4.7-2.fc37.x86_64.rpm                                                    373 kB/s |  37 kB     00:00    
(2/17): libusb1-1.0.25-9.fc37.x86_64.rpm                                                        584 kB/s |  74 kB     00:00    
(3/17): libxslt-1.1.37-1.fc37.x86_64.rpm                                                        1.6 MB/s | 184 kB     00:00    
(4/17): openssl-pkcs11-0.4.12-2.fc37.x86_64.rpm                                                 247 kB/s |  74 kB     00:00    
(5/17): pcsc-lite-1.9.9-1.fc37.x86_64.rpm                                                       174 kB/s |  95 kB     00:00    
(6/17): pcsc-lite-ccid-1.5.0-2.fc37.x86_64.rpm                                                  814 kB/s | 319 kB     00:00    
(7/17): pcsc-lite-libs-1.9.9-1.fc37.x86_64.rpm                                                  148 kB/s |  28 kB     00:00    
(8/17): polkit-121-4.fc37.x86_64.rpm                                                            765 kB/s | 156 kB     00:00    
(9/17): polkit-libs-121-4.fc37.x86_64.rpm                                                       537 kB/s |  66 kB     00:00    
(10/17): polkit-pkla-compat-0.1-22.fc37.x86_64.rpm                                              241 kB/s |  44 kB     00:00    
(11/17): softhsm-2.6.1-5.fc37.5.x86_64.rpm                                                      954 kB/s | 444 kB     00:00    
(12/17): libicu-71.1-2.fc37.x86_64.rpm                                                          4.6 MB/s |  10 MB     00:02    
(13/17): opensc-0.23.0-2.fc37.x86_64.rpm                                                        1.3 MB/s | 1.3 MB     00:00    
(14/17): xmlsec1-1.2.34-4.fc37.x86_64.rpm                                                       3.7 MB/s | 196 kB     00:00    
(15/17): xmlsec1-openssl-1.2.34-4.fc37.x86_64.rpm                                               1.8 MB/s |  96 kB     00:00    
(16/17): openssl-3.0.8-1.fc37.x86_64.rpm                                                        2.4 MB/s | 1.2 MB     00:00    
(17/17): mozjs102-102.8.0-1.fc37.x86_64.rpm                                                     2.0 MB/s | 4.5 MB     00:02    
--------------------------------------------------------------------------------------------------------------------------------
Total                                                                                           4.4 MB/s |  19 MB     00:04     
Running transaction check
Transaction check succeeded.
Running transaction test
Transaction test succeeded.
Running transaction
  Preparing        :                                                                                                        1/1 
  Installing       : polkit-libs-121-4.fc37.x86_64                                                                         1/17 
  Installing       : pcsc-lite-libs-1.9.9-1.fc37.x86_64                                                                    2/17 
  Installing       : libxslt-1.1.37-1.fc37.x86_64                                                                          3/17 
  Installing       : libtool-ltdl-2.4.7-2.fc37.x86_64                                                                      4/17 
  Installing       : xmlsec1-1.2.34-4.fc37.x86_64                                                                          5/17 
  Installing       : libusb1-1.0.25-9.fc37.x86_64                                                                          6/17 
  Installing       : libicu-71.1-2.fc37.x86_64                                                                             7/17 
  Installing       : mozjs102-102.8.0-1.fc37.x86_64                                                                        8/17 
  Running scriptlet: polkit-121-4.fc37.x86_64                                                                              9/17 
  Installing       : polkit-121-4.fc37.x86_64                                                                              9/17 
  Running scriptlet: polkit-121-4.fc37.x86_64                                                                              9/17 
  Installing       : polkit-pkla-compat-0.1-22.fc37.x86_64                                                                10/17 
  Installing       : pcsc-lite-ccid-1.5.0-2.fc37.x86_64                                                                   11/17 
  Running scriptlet: pcsc-lite-ccid-1.5.0-2.fc37.x86_64                                                                   11/17 
  Installing       : pcsc-lite-1.9.9-1.fc37.x86_64                                                                        12/17 
  Running scriptlet: pcsc-lite-1.9.9-1.fc37.x86_64                                                                        12/17 
Created symlink /etc/systemd/system/sockets.target.wants/pcscd.socket → /usr/lib/systemd/system/pcscd.socket.

  Installing       : opensc-0.23.0-2.fc37.x86_64                                                                          13/17 
  Installing       : xmlsec1-openssl-1.2.34-4.fc37.x86_64                                                                 14/17 
  Installing       : openssl-1:3.0.8-1.fc37.x86_64                                                                        15/17 
  Running scriptlet: softhsm-2.6.1-5.fc37.5.x86_64                                                                        16/17 
  Installing       : softhsm-2.6.1-5.fc37.5.x86_64                                                                        16/17 
  Running scriptlet: softhsm-2.6.1-5.fc37.5.x86_64                                                                        16/17 
  Installing       : openssl-pkcs11-0.4.12-2.fc37.x86_64                                                                  17/17 
  Running scriptlet: openssl-pkcs11-0.4.12-2.fc37.x86_64                                                                  17/17 
  Verifying        : libicu-71.1-2.fc37.x86_64                                                                             1/17 
  Verifying        : libtool-ltdl-2.4.7-2.fc37.x86_64                                                                      2/17 
  Verifying        : libusb1-1.0.25-9.fc37.x86_64                                                                          3/17 
  Verifying        : libxslt-1.1.37-1.fc37.x86_64                                                                          4/17 
  Verifying        : openssl-pkcs11-0.4.12-2.fc37.x86_64                                                                   5/17 
  Verifying        : pcsc-lite-1.9.9-1.fc37.x86_64                                                                         6/17 
  Verifying        : pcsc-lite-ccid-1.5.0-2.fc37.x86_64                                                                    7/17 
  Verifying        : pcsc-lite-libs-1.9.9-1.fc37.x86_64                                                                    8/17 
  Verifying        : polkit-121-4.fc37.x86_64                                                                              9/17 
  Verifying        : polkit-libs-121-4.fc37.x86_64                                                                        10/17 
  Verifying        : polkit-pkla-compat-0.1-22.fc37.x86_64                                                                11/17 
  Verifying        : softhsm-2.6.1-5.fc37.5.x86_64                                                                        12/17 
  Verifying        : mozjs102-102.8.0-1.fc37.x86_64                                                                       13/17 
  Verifying        : opensc-0.23.0-2.fc37.x86_64                                                                          14/17 
  Verifying        : openssl-1:3.0.8-1.fc37.x86_64                                                                        15/17 
  Verifying        : xmlsec1-1.2.34-4.fc37.x86_64                                                                         16/17 
  Verifying        : xmlsec1-openssl-1.2.34-4.fc37.x86_64                                                                 17/17 

Installed:
  libicu-71.1-2.fc37.x86_64                  libtool-ltdl-2.4.7-2.fc37.x86_64              libusb1-1.0.25-9.fc37.x86_64         
  libxslt-1.1.37-1.fc37.x86_64               mozjs102-102.8.0-1.fc37.x86_64                opensc-0.23.0-2.fc37.x86_64          
  openssl-1:3.0.8-1.fc37.x86_64              openssl-pkcs11-0.4.12-2.fc37.x86_64           pcsc-lite-1.9.9-1.fc37.x86_64        
  pcsc-lite-ccid-1.5.0-2.fc37.x86_64         pcsc-lite-libs-1.9.9-1.fc37.x86_64            polkit-121-4.fc37.x86_64             
  polkit-libs-121-4.fc37.x86_64              polkit-pkla-compat-0.1-22.fc37.x86_64         softhsm-2.6.1-5.fc37.5.x86_64        
  xmlsec1-1.2.34-4.fc37.x86_64               xmlsec1-openssl-1.2.34-4.fc37.x86_64         

Complete!
.............+....+.....+......+..........+......+.....+.+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++*...............+.+....................+.......+.....................+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++*.+.........+......+....+..............+...+.......+...........+....+........+...+.+......+..+.+.....+.+.....+.+..+...+...................+.....+.+...+..+.......+..+....+......+........+.+..+..........+.........+......+...+.....+......+.+.........+...+...+............+........+...+.........+.+..+..................+.+........+...+....+......+.........+.....+......+....+..............+......+.+.............................+....+.........+...+..+...+...+.........+.+...........+.+...+.........+.....+...+.............+..+...+.........+...+.........+....+......+...+...+............+..+...+.+.....+.......+.....+...+.......+.........+...+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
...........+...+..........+......+......+..+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++*.+........+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++*...+...+..........+........+...+......+.+...+......+......+........+....+..+...+...+...+.+.....+......+...+....+....................+...+.......+...+..+............+..........+......+..+..........+.................+.........+......+....+...........+...+.+..............+.+..+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
-----
The token has been initialized and is reassigned to slot 1642188238
Using slot 0 with a present token (0x61e1cdce)
Created private key:
Private Key Object; RSA 
  label:      test
  Usage:      decrypt, sign, unwrap
  Access:     sensitive
func=xmlSecOpenSSLRsaOaepProcess:file=kt_rsa.c:line=754:obj=rsa-oaep-mgf1p:subj=RSA_private_decrypt(RSA_PKCS1_OAEP_PADDING):error=4:crypto library function failed:openssl error: 1098908674: libp11: NULL Unsupported padding type
func=xmlSecOpenSSLRsaOaepExecute:file=kt_rsa.c:line=632:obj=rsa-oaep-mgf1p:subj=xmlSecOpenSSLRsaOaepProcess:error=1:xmlsec library function failed: 
func=xmlSecTransformDefaultPushBin:file=transforms.c:line=1927:obj=rsa-oaep-mgf1p:subj=xmlSecTransformExecute:error=1:xmlsec library function failed:final=1
func=xmlSecTransformDefaultPushBin:file=transforms.c:line=1952:obj=rsa-oaep-mgf1p:subj=xmlSecTransformPushBin:error=1:xmlsec library function failed:final=1;outSize=256
func=xmlSecTransformCtxBinaryExecute:file=transforms.c:line=941:obj=unknown:subj=xmlSecTransformPushBin:error=1:xmlsec library function failed:dataSize=349
func=xmlSecEncCtxDecryptToBuffer:file=xmlenc.c:line=614:obj=unknown:subj=xmlSecTransformCtxBinaryExecute:error=1:xmlsec library function failed: 
func=xmlSecKeysMngrGetKey:file=keys.c:line=1253:obj=unknown:subj=xmlSecKeysMngrFindKey:error=1:xmlsec library function failed: 
func=xmlSecEncCtxEncDataNodeRead:file=xmlenc.c:line=779:obj=unknown:subj=unknown:error=45:key is not found:encMethod=aes256-cbc
func=xmlSecEncCtxDecryptToBuffer:file=xmlenc.c:line=596:obj=unknown:subj=xmlSecEncCtxEncDataNodeRead:error=1:xmlsec library function failed: 
func=xmlSecEncCtxDecrypt:file=xmlenc.c:line=524:obj=unknown:subj=xmlSecEncCtxDecryptToBuffer:error=1:xmlsec library function failed: 
Error: failed to decrypt file
Error: failed to decrypt file "encrypted.xml"
jtalir commented 1 year ago

Is it possible to confirm that libp11 doesn't support OAEP padding at the moment?

popovec commented 1 year ago

I compiled libp11 (from git), and tried make check, it seems that RSA-OAEP is tested, and it works.

PASS: rsa-testpkcs11.softhsm
PASS: rsa-testfork.softhsm
PASS: rsa-testlistkeys.softhsm
PASS: rsa-testlistkeys_ext.softhsm
PASS: rsa-evp-sign.softhsm
PASS: ec-evp-sign.softhsm
PASS: ec-testfork.softhsm
PASS: fork-change-slot.softhsm
PASS: rsa-pss-sign.softhsm
PASS: rsa-oaep.softhsm
PASS: case-insensitive.softhsm
PASS: rsa-check-privkey.softhsm
PASS: ec-check-privkey.softhsm
PASS: pkcs11-uri-without-token.softhsm
PASS: search-all-matching-tokens.softhsm
PASS: ec-cert-store.softhsm
PASS: ec-copy.softhsm

Look in the tests directory, there is rsa-oaep.softhsm and rsa-oaep.c. I didn't study it thoroughly, but I ran it through pkcs11-spy. It seems that there is a correct call:

47: C_DecryptInit
P:710645; T:0x140229292721984 2023-08-01 18:35:13.309
[in] hSession = 0x1
[in] pMechanism->type = CKM_RSA_PKCS_OAEP            
[in] pMechanism->pParameter->hashAlg = CKM_SHA_1                    
[in] pMechanism->pParameter->mgf = CKG_MGF1_SHA1  
[in] pMechanism->pParameter->source = 1
[in] pMechanism->pParameter->pSourceData[ulSourceDalaLen] NULL [size : 0x0 (0)]
[in] hKey = 0x2
Returned:  0 CKR_OK

I could not reproduce your test using xmlsec1 (on debian 12). Even if I use <ns0:EncryptionMethod Algorithm="http://www.w3.org/2001/04/xmlenc#rsa-1_5"/> I still get other errors...

func=xmlSecKeysMngrGetKey:file=keys.c:line=1256:obj=unknown:subj=xmlSecKeysMngrFindKey:error=1:xmlsec library function failed: 
func=xmlSecEncCtxEncDataNodeRead:file=xmlenc.c:line=788:obj=unknown:subj=unknown:error=45:key is not found:encMethod=rsa-oaep-mgf1p
func=xmlSecEncCtxDecryptToBuffer:file=xmlenc.c:line=610:obj=unknown:subj=xmlSecEncCtxEncDataNodeRead:error=1:xmlsec library function failed: 
func=xmlSecKeysMngrGetKey:file=keys.c:line=1256:obj=unknown:subj=xmlSecKeysMngrFindKey:error=1:xmlsec library function failed: 
func=xmlSecEncCtxEncDataNodeRead:file=xmlenc.c:line=788:obj=unknown:subj=unknown:error=45:key is not found:encMethod=aes256-cbc
func=xmlSecEncCtxDecryptToBuffer:file=xmlenc.c:line=610:obj=unknown:subj=xmlSecEncCtxEncDataNodeRead:error=1:xmlsec library function failed: 
func=xmlSecEncCtxDecrypt:file=xmlenc.c:line=536:obj=unknown:subj=xmlSecEncCtxDecryptToBuffer:error=1:xmlsec library function failed: 

simmilar for non OAEP...

func=xmlSecKeysMngrGetKey:file=keys.c:line=1256:obj=unknown:subj=xmlSecKeysMngrFindKey:error=1:xmlsec library function failed: 
func=xmlSecEncCtxEncDataNodeRead:file=xmlenc.c:line=788:obj=unknown:subj=unknown:error=45:key is not found:encMethod=rsa-1_5
jtalir commented 1 year ago

Hi Peter, thanks for looking into this. To reproduce this issue, you need xmlsec1 either 1.2.33 (when pkcs11 interface was introduced) or 1.2.34. In 1.2.35 there was the migration to openssl3 and openssl engines were disabled by default. On the xmlsec website https://www.aleksey.com/xmlsec/ it is mentioned that this feature may be enabled in compile time via --enable-openssl3-engines for newer version.

popovec commented 1 year ago

Debian has openssl support enabled (xmlsec1 1.2.37). If I compile it without openssl support, I get another error: func=xmlSecOpenSSLAppEngineKeyLoad:file=app.c:line=560:obj=unknown:subj=unknown:error=9:feature is not implemented:details=OpenSSL Engine interface is not enabled

I have fedora37 in LXC, I managed to reproduce your problem ($ /usr/bin/xmlsec1 --version xmlsec1 1.2.34 (openssl)).

I will try to compile xmlsec1 version 1.2.34 on Debian (and latest version of xmlsec1 from git), we will see if it is the same as on Fedora.

popovec commented 1 year ago

I tried to compile xmlsec1 on debian 12 .... I also managed to get to the error on Debian that you have on Fedora:

1.2.34

func=xmlSecOpenSSLRsaOaepProcess:file=kt_rsa.c:line=754:obj=rsa-oaep-mgf1p:subj=RSA_private_decrypt(RSA_PKCS1_OAEP_PADDING):
error=4:crypto library function failed:openssl error: 1098908674: libp11: NULL Unsupported padding type

xmlsec1 reports another error already in version 1.2.35 1.2.35

func=xmlSecKeysMngrGetKey:file=keys.c:line=1256:obj=unknown:subj=xmlSecKeysMngrFindKey:error=1:xmlsec library function failed: 
func=xmlSecEncCtxEncDataNodeRead:file=xmlenc.c:line=778:obj=unknown:subj=unknown:error=45:key is not found:encMethod=rsa-oaep-mgf1p

GIT version, commit 8689a8994bddfc89412c9b2c78e5221b16ad326b we won't even get to decryption here..

xmlsec1 encrypt --pubkey-cert-pem cert.pem --session-key aes-256 --xml-data source.xml --node-xpath '/*[local-name()='\''root'\'']/*[local-name()='\''encrypted'\'']' template.xml
Error: failed to encrypt xml file "source.xml"
Error: failed to encrypt file with template "template.xml"

Always used libp11 and openssl distributed in debian:

openssl 3.0.9-1 libp11-3:amd64 0.4.12-0.1

Since different versions of xmlsec1 show different errors (with the same version of libp11), I assume that the problem will have to be found in xmlsec1.

jtalir commented 1 year ago

As mentioned earlier 1.2.35 disabled openssl engines so, this is why (I guess) it cannot read keys from pkc11 engine. This is certainly in xmlsec1.

But 1.2.34 is throwing an error in libp11 and at the top of this issue, there is the line of code where this exception is probably thrown. There used to be a "case RSA_PKCS1_OAEP_PADDING:" that disappeared and so it defaults to unsupported padding type.

popovec commented 1 year ago

I will try to return to version xmlsec1 1.2.34 and compile different versions of libp11 or with modified libp11.

jtalir commented 1 year ago

Thanks for this. The key question here for the experts is:

popovec commented 1 year ago

I have returned the RSA_PKCS1_OAEP_PADDING mechanism to pkcs11_mechanism() code.

We get a little further through the pkcs11_private_decrypt() function, it crashes on:

rv = CRYPTOKI_call(ctx,
                C_DecryptInit(session, &mechanism, key->object));
jtalir commented 1 year ago

I tested softhsm 2.6.1-2 via Java's sunpkcs11 provider to decrypt RSA OAEP encrypted messages without any problem so it's my assumption that softhsm should not be an issue here.

Apparently rsa-oaep.softhsm is not testing this part of libp11 so maybe the way forward is to modify these tests.

mtrojnar commented 1 year ago

We get a little further through the pkcs11_private_decrypt() function, it crashes on:

@popovec Can you please include a stack backtrace for the crash?

popovec commented 1 year ago

There is no real crash, rv (after C_DecryptInit...) is 7.. I'm looking for where it falls, currently pkcs11-spy says this:

32: C_DecryptInit
P:1193458; T:0x139759826167616 2023-08-02 12:54:09.698
[in] hSession = 0x1
[in] pMechanism->type = CKM_RSA_PKCS_OAEP            
[in] pMechanism->pParameter = NULL
[in] hKey = 0x2
Returned:  7 CKR_ARGUMENTS_BAD

I will try to fix it, there is a missing parameter pMechanism->pParameter->mgf = CKG_MGF1_SHA1

If I find a solution, I will prepare a PR.

popovec commented 1 year ago

So, a short summary, while OAEP is solved in the file src/p11_pkey.c, it is not so in the file src/p11_rsa.c. The test in tests/rsa-oaep* uses part of the code from src/p11_pkey.c.

The above test with xmlsec1 (1.2.34) now works with the following patch:

diff --git a/src/p11_rsa.c b/src/p11_rsa.c
index d01ee35..a6f01bb 100644
--- a/src/p11_rsa.c
+++ b/src/p11_rsa.c
@@ -64,6 +64,9 @@ static int pkcs11_mechanism(CK_MECHANISM *mechanism, const int padding)
        case RSA_NO_PADDING:
                mechanism->mechanism = CKM_RSA_X_509;
                break;
+       case RSA_PKCS1_OAEP_PADDING:
+               mechanism->mechanism = CKM_RSA_PKCS_OAEP;
+               break;
        case RSA_X931_PADDING:
                mechanism->mechanism = CKM_RSA_X9_31;
                break;
@@ -133,10 +136,22 @@ int pkcs11_private_decrypt(int flen, const unsigned char *from, unsigned char *t
        CK_MECHANISM mechanism;
        CK_ULONG size = flen;
        CK_RV rv;
+       CK_RSA_PKCS_OAEP_PARAMS oaep_params;

        if (pkcs11_mechanism(&mechanism, padding) < 0)
                return -1;

+       if (mechanism.mechanism = CKM_RSA_PKCS_OAEP) {
+               mechanism.pParameter = &oaep_params;
+               mechanism.ulParameterLen = sizeof oaep_params;
+               oaep_params.mgf = CKG_MGF1_SHA1;
+               oaep_params.hashAlg = CKM_SHA_1;
+
+//             oaep_params.source = CKZ_DATA_SPECIFIED;
+               oaep_params.source = 0;
+               oaep_params.pSourceData = NULL;
+               oaep_params.ulSourceDataLen = 0;
+       }
        if (pkcs11_get_session(slot, 0, &session))
                return -1;

(Currently, the parameters for OAEP are fixedly coded in the code, these must be taken from the context. I still have to finish this.)

dwmw2 commented 1 year ago

From your run.sh I see:

SOFTHSM2_CONF=./softhsm.conf xmlsec1 decrypt --privkey-openssl-engine:test "pkcs11;pkcs11:token=test;object=test;pin-value=secret1"  encrypted.xml

That's kind of awful. We should be able to just pass a PKCS#11 URI in place of a file name and applications should do the right thing. Is there a bug open already with xmlsec for that?

lsh123 commented 1 year ago

From your run.sh I see:

SOFTHSM2_CONF=./softhsm.conf xmlsec1 decrypt --privkey-openssl-engine:test "pkcs11;pkcs11:token=test;object=test;pin-value=secret1"  encrypted.xml

That's kind of awful. We should be able to just pass a PKCS#11 URI in place of a file name and applications should do the right thing. Is there a bug open already with xmlsec for that?

https://github.com/lsh123/xmlsec/blob/8689a8994bddfc89412c9b2c78e5221b16ad326b/apps/xmlsec.c#L308

dwmw2 commented 1 year ago

Not exactly compliant with the vision of https://www.ietf.org/archive/id/draft-woodhouse-cert-best-practice-01.html that applications should Just Work without making life hard for their users :)

I shouldn't have to tell the application whether I'm giving it a DER or a PEM file. That much is obvious from the content of the file. Likewise whether it's PKCS#1, PKCS#8 or PKCS#12. Users shouldn't have to know or care about that stuff. They have a file with their key in, they just tell the application to use that file. Even if that file is a TPM-wrapped key, ideally that should Just Work too.

And if what they pass is a PKCS#11 URI starting pkcs11: instead of a filename, there's not really much excuse for applications to demand any more magic incantations on the command line. Especially incantations that differ according to which crypto library said application was built against this week.

dwmw2 commented 1 year ago

(Sorry, that was a bit rantier than it probably should have been. Happy to help work on fixing this stuff.... my original question was just whether there's a bug/rfe already open.)

lsh123 commented 1 year ago

We agree to disagree. I don’t like magic — too often it creates more problems than it solves. Specifically for consecutive tool, I prefer to have explicit parameters that do exactly one thing and give a reasonable error when this one thing doesn’t work.

mtrojnar commented 1 year ago

Not exactly compliant with the vision of https://www.ietf.org/archive/id/draft-woodhouse-cert-best-practice-01.html that applications should Just Work without making life hard for their users :)

@dwmw2 Would you commit a PR to ensure compliance?

dwmw2 commented 1 year ago

Would you commit a PR to ensure compliance

For libp11 I think we have everything. We support RFC7512 URIs, we use p11-kit-proxy.so by default, and everything Just Works™.

But applications still need to see that the user gives a "filename" which is actually a pkcs11: URI and do the right thing for whatever crypto library they happen to be linked against. Which in the case of OpenSSL is loading the pkcs11 engine manually.

dwmw2 commented 1 year ago

We agree to disagree. I don’t like magic — too often it creates more problems than it solves.

Respectfully, I think that's a bit of a cop-out. This is hardly magic.

If a user gives the application a PEM file, that file explicitly says something like

-----BEGIN PRIVATE KEY-----
-----BEGIN RSA PRIVATE KEY-----
-----BEGIN EC PRIVATE KEY-----
-----BEGIN TSS2 PRIVATE KEY-----

It's not magic to silently do the right thing with those files instead of requiring a different command line argument for each one.

It's also not that magical to tell that a file is a text file or a binary file. Even grep does that. And if it's a binary file, the ASN.1 forms of those keys are also fairly trivial to distinguish by their structure and the OIDs therein, so that's not really magic either.

I'll grant you that it's really awful that the crypto libraries don't do this for you, but it isn't magic. And it's awful for users to deal with applications which need babysitting in such a way. For users, it is magic.

jtalir commented 1 year ago

(Currently, the parameters for OAEP are fixedly coded in the code, these must be taken from the context. I still have to finish this.)

This would be certainly useful not to have parameter hard coded. I hope that parameter passing will be consistent with the way xmlsec1 is filling these parameters. @lsh123 could probably help here. Are other MGF functions then CKG_MGF1_SHA1 supported for OAEP in xmlsec1 1.2.x versions?

popovec commented 1 year ago

Openssl API for RSA_private_decrypt() allows to use RSA_PKCS1_OAEP_PADDING only with SHA_1 hash and and MGF1_SHA1 mask gen function. It is not possible to use RFC8017 "Label" or PKCS#11 "source data" respectively.

https://www.openssl.org/docs/man3.0/man3/RSA_private_decrypt.html

It is better not to use this interface. Look at the options that uses openssl pkeyutl, focus on using the pkcs11_try_pkey_rsa_decrypt() call from src/p11_pkey.c . In this function, OAEP support is almost complete, there is still missing support for "Label" (pkcs11_params_oaep()).

lsh123 commented 1 year ago

Openssl API for RSA_private_decrypt() allows to use RSA_PKCS1_OAEP_PADDING only with SHA_1 hash and and MGF1_SHA1 mask gen function. It is not possible to use RFC8017 "Label" or PKCS#11 "source data" respectively.

https://www.openssl.org/docs/man3.0/man3/RSA_private_decrypt.html

It is better not to use this interface. Look at the options that uses openssl pkeyutl, focus on using the pkcs11_try_pkey_rsa_decrypt() call from src/p11_pkey.c . In this function, OAEP support is almost complete, there is still missing support for "Label" (pkcs11_params_oaep()).

This is not correct. See the xmlsec code for example that sets/checks label and other parameters:

https://github.com/lsh123/xmlsec/blob/8689a8994bddfc89412c9b2c78e5221b16ad326b/src/openssl/kt_rsa.c#L784

popovec commented 1 year ago

@lsh123 I'm not saying your code is setting anything wrong. But I have a problem with the decryption parameters.

The only information available to me is "int padding". According to the openssl documentation: https://www.openssl.org/docs/man3.0/man3/RSA_private_decrypt.html

the following values are available:

RSA_PKCS1_OAEP_PADDING
RSA_NO_PADDING
RSA_PKCS1_PADDING

When I tried the above test, I found the padding set to the value RSA_PKCS1_OAEP_PADDING. This was not addressed in the libp11 code. The patch mentioned above takes care of this, with the fact that it only has this information available. Maybe OpenSSL somehow allows to get additional parameters for OAEP, but the openssl documentation says:

RSA_PKCS1_OAEP_PADDING
EME-OAEP as defined in PKCS #1 v2.0 with SHA-1, MGF1 and an empty encoding parameter. This mode is recommended for all new applications.

See how OAEP is handled in the test code: https://github.com/OpenSC/libp11/blob/master/tests/rsa-oaep.c

In this code, the OAEP parameters are available (but the "label" parameter is also not applicable).

jtalir commented 1 year ago

In any case, I can confirm that patch works for me. Many thanks @popovec! Any idea when this get's to the master branch?