Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pass RSA-PSS operation to the token *when it supports it* #176

Closed
mouse07410 opened this issue Aug 18, 2017 · 129 comments
Closed

Pass RSA-PSS operation to the token *when it supports it* #176

mouse07410 opened this issue Aug 18, 2017 · 129 comments

Comments

@mouse07410
Copy link
Contributor

This is not a bug report (at least I don't think it's a bug). MacOS Sierra 10.12.6, Xcode-8.3.3.

Current Github version of libp11, p11-kit v0.23.2.

I have a USB PKCS#11 device that currently cannot be served by OpenSC opensc-pkcs11.so.

My system has p11-kit installed. The library that serves this device is /usr/local/lib/yubihsm_pkcs11.so.

Could you please help me configuring p11-kit or libp11 or OpenSSL so that libp11 would use the above library upon request?

Thank you!

@nmav perhaps you could step in too?

@nmav
Copy link
Contributor

nmav commented Aug 19, 2017

Most likely you only need to register yubihsm_pkcs11.so with p11-kit.
See https://p11-glue.freedesktop.org/doc/p11-kit/config-example.html

@mouse07410
Copy link
Contributor Author

Most likely you only need to register yubihsm_pkcs11.so with p11-kit.

@nmav thank you! But it looks like I need to do more. I know that this module should not be initialized "in advance". The README for adding this module to OpenSSL said to add "init = 0". Is there any way to do the same with p11-kit?

$ p11-kit list-modules
yubihsm_pkcs11: Error opening configuration file '(null)'
p11-kit: yhsm2: module failed to initialize, skipping: The operation failed
p11-kit-trust: p11-kit-trust.so
    library-description: PKCS#11 Kit Trust Module
    library-manufacturer: PKCS#11 Kit
    library-version: 0.23
    token: Default Trust
        manufacturer: PKCS#11 Kit
        model: p11-kit-trust
        serial-number: 1
        hardware-version: 0.23
        flags:
               write-protected
               token-initialized
    token: System Trust
        manufacturer: PKCS#11 Kit
        model: p11-kit-trust
        serial-number: 1
        hardware-version: 0.23
        flags:
               write-protected
               token-initialized
pkcs11: /Library/OpenSC/lib/opensc-pkcs11.dylib
    library-description: OpenSC smartcard framework
    library-manufacturer: OpenSC Project
    library-version: 0.17
softhsm: /opt/local/lib/softhsm/libsofthsm2.dylib
    library-description: Implementation of PKCS11
    library-manufacturer: SoftHSM
    library-version: 2.3
    token: Botan PKCS#11 tests
        manufacturer: SoftHSM project
        model: SoftHSM v2
        serial-number: b158018a21bc4979
        hardware-version: 2.3
        firmware-version: 2.3
        flags:
               rng
               login-required
               user-pin-initialized
               restore-key-not-needed
               token-initialized
    token: test
        manufacturer: SoftHSM project
        model: SoftHSM v2
        serial-number: 02b891b82879828e
        hardware-version: 2.3
        firmware-version: 2.3
        flags:
               rng
               login-required
               user-pin-initialized
               restore-key-not-needed
               token-initialized
    token: 
        manufacturer: SoftHSM project
        model: SoftHSM v2
        serial-number: 
        hardware-version: 2.3
        firmware-version: 2.3
        flags:
               rng
               login-required
               restore-key-not-needed
               so-pin-locked
               so-pin-to-be-changed
ykcs11: /usr/local/lib/libykcs11.dylib
    library-description: PKCS#11 PIV Library (SP-800-73)
    library-manufacturer: Yubico (www.yubico.com)
    library-version: 1.44
$ cat ~/.config/pkcs11/modules/yhsm2.module 
module: /usr/local/lib/yubihsm_pkcs11.dylib
$

@mouse07410
Copy link
Contributor Author

mouse07410 commented Aug 20, 2017

Update
Looks like adding critical: no to the yhsm2.module resolved the problem...?

$ p11-kit list-modules
p11-kit-trust: p11-kit-trust.so
    library-description: PKCS#11 Kit Trust Module
    library-manufacturer: PKCS#11 Kit
    library-version: 0.23
    token: Default Trust
        manufacturer: PKCS#11 Kit
        model: p11-kit-trust
        serial-number: 1
        hardware-version: 0.23
        flags:
               write-protected
               token-initialized
    token: System Trust
        manufacturer: PKCS#11 Kit
        model: p11-kit-trust
        serial-number: 1
        hardware-version: 0.23
        flags:
               write-protected
               token-initialized
pkcs11: /Library/OpenSC/lib/opensc-pkcs11.dylib
    library-description: OpenSC smartcard framework
    library-manufacturer: OpenSC Project
    library-version: 0.17
softhsm: /opt/local/lib/softhsm/libsofthsm2.dylib
    library-description: Implementation of PKCS11
    library-manufacturer: SoftHSM
    library-version: 2.3
    token: Botan PKCS#11 tests
        manufacturer: SoftHSM project
        model: SoftHSM v2
        serial-number: b158018a21bc4979
        hardware-version: 2.3
        firmware-version: 2.3
        flags:
               rng
               login-required
               user-pin-initialized
               restore-key-not-needed
               token-initialized
    token: test
        manufacturer: SoftHSM project
        model: SoftHSM v2
        serial-number: 02b891b82879828e
        hardware-version: 2.3
        firmware-version: 2.3
        flags:
               rng
               login-required
               user-pin-initialized
               restore-key-not-needed
               token-initialized
    token: 
        manufacturer: SoftHSM project
        model: SoftHSM v2
        serial-number: 
        hardware-version: 2.3
        firmware-version: 2.3
        flags:
               rng
               login-required
               restore-key-not-needed
               so-pin-locked
               so-pin-to-be-changed
yhsm2: /usr/local/lib/yubihsm_pkcs11.dylib
    library-description: YubiHSM PKCS#11 Library
    library-manufacturer: Yubico (www.yubico.com)
    library-version: 0.9
    token: YubiHSM
        manufacturer: Yubico (www.yubico.com)
        model: YubiHSM
        serial-number: 056xxxxx
        hardware-version: 0.100
        firmware-version: 0.100
        flags:
               rng
               login-required
               user-pin-initialized
               token-initialized
ykcs11: /usr/local/lib/libykcs11.dylib
    library-description: PKCS#11 PIV Library (SP-800-73)
    library-manufacturer: Yubico (www.yubico.com)
    library-version: 1.44
$ 

But when I try to use this yhsm2 engine, I'm getting error from OpenSSL:

$ OPENSSL_CONF=~/src/openssl.cnf openssl engine -t -c pkcs11
(pkcs11) pkcs11 engine
 [RSA]
     [ available ]
$ OPENSSL_CONF=~/src/openssl.cnf openssl engine -t -c yhsm2
bad engine id
140736391603208:error:260B606D:engine routines:DYNAMIC_LOAD:init failed:eng_dyn.c:545:
140736391603208:error:2606A074:engine routines:ENGINE_by_id:no such engine:eng_list.c:390:id=yhsm2
$

I symlinked libpkcs11.dylib to libyhsm2.dylib because I wanted libp11 to take care of this... Here's the tail of my openssl.cnf:

. . . . .
[engine_section]
pkcs11 = pkcs11_section
yhsm2  = yhsm2_section

[pkcs11_section]
engine_id = pkcs11
dynamic_path = /opt/local/lib/engines/libpkcs11.so
MODULE_PATH = /Library/OpenSC/lib/opensc-pkcs11.so
init = 0

[yhsm2_section]
engine_id = yhsm2
dynamic_path = /opt/local/lib/engines/libyhsm2.dylib
MODULE_PATH = /usr/local/lib/yubihsm_pkcs11.dylib
init = 0

Another thing I haven't figured out is how exactly to tie all these together. I.e., how to inform p11-kit and/or libp11 when I want to use pkcs11.module, when ykcs11.module, and when - yhsm2.module. I tried to explicitly define module yhsm2 by symlinking libpkcs11.dylib to libyhsm2.dylib, but it did not quite work.

It looks like I have to stick with engine pkcs11 definition if I want to use libp11 - unless there's some config trick that I'm now aware of. How can I choose and then verify which library (opensc-pkcs11.dylib or yubihsm_pkcs11.dylib) is used for a request, such as $ openssl rand -engine pkcs11 128 -hex?

@nmav
Copy link
Contributor

nmav commented Aug 20, 2017 via email

@dengert
Copy link
Member

dengert commented Aug 20, 2017 via email

@mouse07410
Copy link
Contributor Author

My experience with yubi pkcs11 modules is that they are often incomplete and half baked.

No argument from me here! ;-)

Does p11tool show anything for that module?

Yep:

$ p11tool --list-tokens
Token 0:
	URL: pkcs11:model=p11-kit-trust;manufacturer=PKCS%2311%20Kit;serial=1;token=Default%20Trust
	Label: Default Trust
	Type: Trust module
	Manufacturer: PKCS#11 Kit
	Model: p11-kit-trust
	Serial: 1
	Module: p11-kit-trust.so


Token 1:
	URL: pkcs11:model=p11-kit-trust;manufacturer=PKCS%2311%20Kit;serial=1;token=System%20Trust
	Label: System Trust
	Type: Trust module
	Manufacturer: PKCS#11 Kit
	Model: p11-kit-trust
	Serial: 1
	Module: p11-kit-trust.so


Token 2:
	URL: pkcs11:model=SoftHSM%20v2;manufacturer=SoftHSM%20project;serial=b158018a21bc4979;token=Botan%20PKCS%2311%20tests
	Label: Botan PKCS#11 tests
	Type: Generic token
	Manufacturer: SoftHSM project
	Model: SoftHSM v2
	Serial: b158018a21bc4979
	Module: /opt/local/lib/softhsm/libsofthsm2.dylib


Token 3:
	URL: pkcs11:model=SoftHSM%20v2;manufacturer=SoftHSM%20project;serial=02b891b82879828e;token=test
	Label: test
	Type: Generic token
	Manufacturer: SoftHSM project
	Model: SoftHSM v2
	Serial: 02b891b82879828e
	Module: /opt/local/lib/softhsm/libsofthsm2.dylib


Token 4:
	URL: pkcs11:model=YubiHSM;manufacturer=Yubico%20%28www.yubico.com%29;serial=056xxxxx;token=YubiHSM
	Label: YubiHSM
	Type: Hardware token
	Manufacturer: Yubico (www.yubico.com)
	Model: YubiHSM
	Serial: 056xxxxx
	Module: /usr/local/lib/yubihsm_pkcs11.dylib


$ p11tool --login --list-all "pkcs11:model=YubiHSM;manufacturer=Yubico%20%28www.yubico.com%29;serial=056xxxxx;token=YubiHSM"
Token 'YubiHSM' with URL 'pkcs11:model=YubiHSM;manufacturer=Yubico%20%28www.yubico.com%29;serial=056xxxxx;token=YubiHSM' requires user PIN
Enter PIN: 
Object 0:
	URL: pkcs11:model=YubiHSM;manufacturer=Yubico%20%28www.yubico.com%29;serial=056xxxxx;token=YubiHSM;id=%00%01;object=DEFAULT%20AUTHKEY%20CHANGE%20THIS%20ASAP;type=secret-key
	Type: Secret key
	Label: DEFAULT AUTHKEY CHANGE THIS ASAP
	Flags: CKA_PRIVATE; CKA_EXTRACTABLE; CKA_SENSITIVE; 
	ID: 00:01

Object 1:
	URL: pkcs11:model=YubiHSM;manufacturer=Yubico%20%28www.yubico.com%29;serial=056xxxxx;token=YubiHSM;id=%8a%17;object=label%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00;type=secret-key
	Type: Secret key
	Label: label
	Flags: CKA_PRIVATE; CKA_NEVER_EXTRACTABLE; CKA_SENSITIVE; 
	ID: 8a:17

Object 2:
	URL: pkcs11:model=YubiHSM;manufacturer=Yubico%20%28www.yubico.com%29;serial=056xxxxx;token=YubiHSM;id=%8f%b8;object=label%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00;type=private
	Type: Private key
	Label: label
	Flags: CKA_PRIVATE; CKA_NEVER_EXTRACTABLE; CKA_SENSITIVE; 
	ID: 8f:b8

Object 3:
	URL: pkcs11:model=YubiHSM;manufacturer=Yubico%20%28www.yubico.com%29;serial=056xxxxx;token=YubiHSM;id=%8f%b8;object=label%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00;type=public
	Type: Public key
	Label: label
	Flags: CKA_EXTRACTABLE; 
	ID: 8f:b8

Object 4:
	URL: pkcs11:model=YubiHSM;manufacturer=Yubico%20%28www.yubico.com%29;serial=056xxxxx;token=YubiHSM;id=%8f%b8;object=label%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00;type=cert
	Type: X.509 Certificate
	Label: label
	Flags: CKA_PRIVATE; 
	ID: 8f:b8

Object 5:
	URL: pkcs11:model=YubiHSM;manufacturer=Yubico%20%28www.yubico.com%29;serial=056xxxxx;token=YubiHSM;id=%d7%41;object=label%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00;type=private
	Type: Private key
	Label: label
	Flags: CKA_PRIVATE; CKA_NEVER_EXTRACTABLE; CKA_SENSITIVE; 
	ID: d7:41

Object 6:
	URL: pkcs11:model=YubiHSM;manufacturer=Yubico%20%28www.yubico.com%29;serial=056xxxxx;token=YubiHSM;id=%d7%41;object=label%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00;type=public
	Type: Public key
	Label: label
	Flags: CKA_EXTRACTABLE; 
	ID: d7:41

$ 

https://www.yubico.com/products/yubihsm/ says: ...

Yes, that was their first attempt to sell an HSM device, and indeed it was rather limited (aka unsuitable for my needs - no asymmetric, etc.).

This is their 2nd version, and it's still in Beta - so you can't find it on a product site yet. They are still enhancing and improving it ("it" applies to hardware, software, and documentation).

Can you get OpenSC pkcs11-tool to list the mechanisms supported by the /usr/local/lib/yubihsm_pkcs11.so?

Yep, you bet:

$ pkcs11-tool --module $YUBIHSM_PKCS11_MODULE -M
Using slot 0 with a present token (0x0)
Supported mechanisms:
  RSA-PKCS, keySize={2048,4096}, hw, decrypt, sign
  SHA1-RSA-PKCS, keySize={2048,4096}, hw, decrypt, sign
  SHA256-RSA-PKCS, keySize={2048,4096}, hw, decrypt, sign
  SHA384-RSA-PKCS, keySize={2048,4096}, hw, decrypt, sign
  SHA512-RSA-PKCS, keySize={2048,4096}, hw, decrypt, sign
  RSA-PKCS-PSS, keySize={2048,4096}, hw, sign
  SHA1-RSA-PKCS-PSS, keySize={2048,4096}, hw, sign
  SHA256-RSA-PKCS-PSS, keySize={2048,4096}, hw, sign
  SHA384-RSA-PKCS-PSS, keySize={2048,4096}, hw, sign
  SHA512-RSA-PKCS-PSS, keySize={2048,4096}, hw, sign
  RSA-PKCS-KEY-PAIR-GEN, keySize={2048,4096}, hw, generate_key_pair
  ECDSA-KEY-PAIR-GEN, keySize={256,521}, hw, generate_key_pair, other flags=0x1500000
  SHA-1-HMAC, keySize={1,512}, hw, sign, verify
  SHA256-HMAC, keySize={1,512}, hw, sign, verify
  SHA384-HMAC, keySize={1,1024}, hw, sign, verify
  SHA512-HMAC, keySize={1,1024}, hw, sign, verify
  ECDSA, keySize={256,521}, hw, sign, other flags=0x1500000
  ECDSA-SHA1, keySize={256,521}, hw, sign, other flags=0x1500000
  ECDSA-SHA256, keySize={256,521}, hw, sign, other flags=0x1500000
  ECDSA-SHA348, keySize={256,521}, hw, sign, other flags=0x1500000
  ECDSA-SHA512, keySize={256,521}, hw, sign, other flags=0x1500000
  RSA-PKCS-OAEP, keySize={2048,4096}, hw, decrypt
  mechtype-0x80000004, keySize={128,128}, hw, wrap
  SHA-1, digest
  SHA256, digest
  SHA384, digest
  SHA512, digest
  GENERIC-SECRET-KEY-GEN, keySize={128,128}, hw, generate
$ 

AFAIK libp11 and its engine only support the use of asymmetric keys RSA and EC via the engine. (Sounds like a future project.)

Are you sure? Can it access RNG that the token provides? I.e., if I'm accessing a token programmatically via OpenSSL+libp11, and want to get some randomness from its RNG - I should be able to, right?

I don't see on github.com/yubico a yubihsm so it may not be open source.

No, not yet at least (but that's up to Yubico to decide).

To see if trying it with p11-kit, can you get a PKCS#11 trace. You could use opensc-spy in front of yubihsm_pkcs11.

To do that I would need to set YUBIHSM_PKCS11_MODULE=/Library/OpenSC/lib/opensc-spy.dylib and PKCS11SPY=/usr/local/lib/yubihsm_pkcs11.dylib, and run p11-kit? Right?

@dengert
Copy link
Member

dengert commented Aug 20, 2017

It looks like you have two ways to use the yubihsm with OpenSSL as an engine.
You refer to trying:

[yhsm2_section]
engine_id = yhsm2
dynamic_path = /opt/local/lib/engines/libyhsm2.dylib
MODULE_PATH = /usr/local/lib/yubihsm_pkcs11.dylib
init = 0

Is libyhsm2.dylib an engine module? Does it call PKCS#11?
Does it need to load other libs? What does ldd or MacOS version show what it needs to load.

The other choice is to use libp11's engine, and let it use p11-kit or a PKCS#11 module.
It also looks like yubihsm_pkcs11.dylib does not support any symmetric mechanisms.

But I don't think symmetric keys or random are supported by libp11 either.
Look for ENGINE_SET_ functions or ENGINE_set_RAND and ENGNE_SET_ciphers in libp11 or the yubi libs.

@mouse07410
Copy link
Contributor Author

Is libyhsm2.dylib an engine module? Does it call PKCS#11? Does it need to load other libs?

Wel, that's my ugly hack. I took pkcs11.dylib (the PKCS#11 engine/library from libp11) and copied it to libyhsm2.dylib. One reason why - because ultimately I want the libp11 engine to handle this, as a part of all the PKCS#11 token support (CAC, PIV, Yubikey, and this new Yubico HSM2 device).

What does ldd or MacOS version show what it needs to load.

All the normal things - it's a copy of working libp11... The MacOS version is $ otool -L /path/to/libyhsm2.dylib.

The other choice is to use libp11's engine, and let it use p11-kit or a PKCS#11 module.

Since my current installation has libp11 using p11-kit, I think this is the path I'd like to follow.

It also looks like yubihsm_pkcs11.dylib does not support any symmetric mechanisms.

Could you give an example of a symmetric mechanism, so I know what to look/check for? I.e., how would I tell pkcs11-tool to do something "symmetric", and what would pkcs11-tool -M report for symmetric mechanisms?

Thanks!

@mtrojnar
Copy link
Member

Could you give an example of a symmetric mechanism

libp11 does not support symmetric algorithms.

@mouse07410
Copy link
Contributor Author

libp11 does not support symmetric algorithms.

OK, thanks. I still would like to know what OpenSC would report if the token itself supported symmetric algorithms, and what would those algorithms (typically) be, besides hash (SHA-x) and HMAC.

@dengert
Copy link
Member

dengert commented Aug 22, 2017

@Mouse you asked: "what would pkcs11-tool -M report for symmetric mechanisms:

pkcs11-tool.c maps CKM_* via the static struct mech_info p11_mechanisms or prints the HEX value of the mechanism. So look for CKM_AES_ (The table appears to have all the PKCS#11 symmetric key mechanisms in the table.)_

You said you were working with some beta version of the yubihsm.

https://www.yubico.com/products/yubihsm/
says it supports AES.

So why does it not support AES? (If the module supported it, pkcs11-tool -M should have listed it.)
Or why does the web page say it supports AES?

In addition, If you have any input with Yubico on this have them do something to make the 0x80000004 mechanism Yubico specific, like what Netscape did.

@dengert
Copy link
Member

dengert commented Aug 22, 2017

@Mouse you also asked: " how would I tell pkcs11-tool to do something "symmetric".

Just because pkcs11-tool.c can list all the mechanisms, it does not mean it can use them.

@mouse07410
Copy link
Contributor Author

You said you were working with some beta version of the yubihsm.

Yep. It is pre-production. Exact specs not finalized yet.

https://www.yubico.com/products/yubihsm/ says it supports AES. So why does it not support AES? (If the module supported it, pkcs11-tool -M should have listed it.)

Yeah, but that page is for a different HSM. Maybe this one would be able to do the same, or maybe it would be more in line with the "big guns" HSM that mostly do audited asymmetric things like CA operations... As it is highly unlikely one would use a USB-connected HSM with Yubikey-Nano form-factor to off-load bulk encryption to.

In addition, If you have any input with Yubico on this have them do something to make the 0x80000004 mechanism Yubico specific, like what Netscape did.

I cannot claim to have a pull with them, but I certainly can inform of issues and recommend solutions. In your opinion, what should be done? I.e., what should be changed and where? Is it what the physical device reports? Or is it what the supporting library (the "driver") maps it to? And what should it (whatever "it" is) be mapped to instead?

@dengert
Copy link
Member

dengert commented Aug 23, 2017

@mouse07410 See:
OpenSC/OpenSC#1131 (comment)
as to how a vendor can make vendor_defined PKCS#11 types.
I see you have given it a "thumbs up" already.

I would suggest Yubico use 59554200 for YUB This gives then up to 256 different types.
It has a better change of being unique when use with p11-kit for example.

And these could be added to pkcs11-tool to at least list them. I would never add 0x80000004 to pkcs11-tool, as it has little chance of being unique across multiple modules.

@mouse07410
Copy link
Contributor Author

mouse07410 commented Aug 23, 2017

@dengert so if I understand correctly, you suggest

  • define Yubico vendor type code, e.g.,
    #define YUBICO_VENDOR_DEFINED 0x59554200
  • define this specific code as
    #define CKA_YUBICO_WHATEVER_OP ( CKA_VENDOR_DEFINED | YUBICO_VENDOR_DEFINED | 4UL)

Correct? (If so, I will certainly pass this to Yubico, and hopefully they'd do the right thing...)

@dengert
Copy link
Member

dengert commented Aug 24, 2017

Yes.

@dengert
Copy link
Member

dengert commented Aug 24, 2017

Except we were talking mechanisms, so it would be:
#define CKM_YUBICO_WHATEVER_MECH ( CKM_VENDOR_DEFINED | YUBICO_VENDOR_DEFINED | 4UL)

@mouse07410
Copy link
Contributor Author

Submitted. Let's wait and see now. ;-)

@mouse07410
Copy link
Contributor Author

mouse07410 commented Aug 25, 2017

I can close this issue. Here's what I learned:

  • To use private key (RSA with signing capabilities) with ID 0x0301 on YubiHSM2, the key URL should be pkcs11:token=YubiHSM;id=%03%01;type=private
  • To use private key SIGN key on Yubikey NEO or 4, the key URL should be pkcs11:manufacturer=piv_II;object=SIGN%20key;type=private

The problem: YubiHSM2 can sign using RSA-PSS, for example with SHA384-RSA-PKCS-PSS algorithm, via yubihsm-shell. But trying to access it via libp11 or OpenSC fails:

$ openssl dgst -sha384 -engine pkcs11 -keyform engine -sign "pkcs11:token=YubiHSM;id=%03%01;type=private" -sigopt rsa_padding_mode:pss -sigopt rsa_pss_saltlen:-1 -out t2570.dat.sig4 t2570.dat
engine "pkcs11" set.
Enter PKCS#11 token PIN for YubiHSM:
Error Signing Data
140736391603208:error:8207A070:PKCS#11 module:pkcs11_private_encrypt:Mechanism invalid:p11_rsa.c:120:
$

Doing the same thing with Yubikey NEO works:

$ openssl dgst -sha384 -engine pkcs11 -keyform engine -sign "pkcs11:manufacturer=piv_II;object=SIGN%20key;type=private" -sigopt rsa_padding_mode:pss -sigopt rsa_pss_saltlen:-1 -out t2570.dat.sig4 t2570.dat
engine "pkcs11" set.
Enter PKCS#11 token PIN for PIV Card Holder pin (PIV_II):
Enter PKCS#11 key PIN for SIGN key:
$ openssl dgst -sha384 -engine pkcs11 -keyform engine -verify "pkcs11:manufacturer=piv_II;object=SIGN%20pubkey;type=public" -sigopt rsa_padding_mode:pss -sigopt rsa_pss_saltlen:-1 -signature t2570.dat.sig4 t2570.dat
engine "pkcs11" set.
Verified OK
$

Verifying SHA384-RSA-PKCS-PSS signature made by yubihsm-shell with private key stored in YubiHSM2 also works fine:

$ openssl dgst -sha384 -verify 301-rsa.pem -keyform PEM -sigopt rsa_padding_mode:pss -signature t2570.dat.sig t2570.dat
Verified OK
$ openssl dgst -sha384 -engine pkcs11 -keyform engine -verify "pkcs11:token=YubiHSM;id=%03%01;type=public" -sigopt rsa_padding_mode:pss -signature t2570.dat.sig t2570.dat
engine "pkcs11" set.
Enter PKCS#11 token PIN for YubiHSM:
Verified OK
$ 

RSASSA PKCS v1.5 signatures also work with YubiHSM2:

$ openssl dgst -sha384 -engine pkcs11 -keyform engine -sign "pkcs11:token=YubiHSM;id=%03%01;type=private" -sigopt rsa_padding_mode:pkcs1 -out t2570.dat.sig2 t2570.dat
engine "pkcs11" set.
Enter PKCS#11 token PIN for YubiHSM:
$ openssl dgst -sha384 -engine pkcs11 -keyform engine -verify "pkcs11:token=YubiHSM;id=%03%01;type=public" -sigopt rsa_padding_mode:pkcs1 -signature t2570.dat.sig2 t2570.dat
engine "pkcs11" set.
Enter PKCS#11 token PIN for YubiHSM:
Verified OK
$ 

Any recommendations here?

@dengert
Copy link
Member

dengert commented Aug 25, 2017

Most likely the problem has to do with differences in the card's or libp11's capabilities. The PIV type devices support mechanism RSA-X509. The YubiHSM does not based on pkcs11-tool -M list above. With RSA-X509 (i.e. RAW RSA) upper levels of software (like OpenSSL) can do the padding. But the YubiHSM only supports RSA-PKCS and RSA-PKCS-PSS.

Run both PIV and YubiHSM with SPY traces to see what is going on.

@dengert
Copy link
Member

dengert commented Aug 25, 2017

And it looks like p11 does not support PSS.
look at p11_rsa.c: pkcs11_mechanism() It only understands CKM_RSA_PKCS, CKM_RSA_X_509 and CKM_RSA_X9_31.

@mouse07410
Copy link
Contributor Author

mouse07410 commented Aug 25, 2017

Most likely the problem has to do with differences in the card's or libp11's capabilities.

What I think you're saying is:

  • Yubikey NEO does only RSA-X509, therefore as OpenSSL/libp11/OpenSC figures that out, it performs RSA-PSS in software. So it works correctly.
  • YubiHSM2 does not support raw RSA-X509, advertising combined mechanisms like RSA-PKCS-PSS and RSA-PKCS. Therefore OpenSSL does not try to do PSS by itself in software, but passes the request to libp11 - where it fails.

Correct?

And a naive question: why it does not fail on signature verification, only on signing? Because the verification is actually done always in software (aka public key is extracted from the card, unlike private key operations)?

And it looks like p11 does not support PSS.

@mtrojnar what would it take to add CKM_RSA_PKCS_PSS mechanism in? With some kind of a check that would not abort the operation or try to ask the token to do PSS if all it supports is RSA-X509? So it won't break RSA-PSS for tokens (like PIV and Yubikey NEO) that do only RSA-X509 where it works now?

Run both PIV and YubiHSM with SPY traces to see what is going on.

Will do and post here.

Update
Successful RSA-PSS spy log with Yubikey NEO:
spy-neo.txt

Failing RSA-PSS spy log with YubiHSM2:
spy-yhsm2.txt

@mtrojnar So what I'm essentially after is libp11 enhancement to pass RSA-PSS requests to the token when the token supports it, and to keep requesting RSA-X509 with the tokens that don't support RSA-PSS. Is it feasible?

@mouse07410 mouse07410 changed the title How to access token via another library? Pass RSA-PSS operation to the token *when it supports it* Aug 28, 2017
@mouse07410
Copy link
Contributor Author

@mtrojnar ?

@mtrojnar
Copy link
Member

mtrojnar commented Aug 30, 2017

The spy logs confirm what I found during my investigation of the OpenSSL source code in March 2016:
OpenSSL never requests the PSS padding from an engine. Instead, it requests RSA_NO_PADDING (which translates into CKM_RSA_X_509 for PKCS#11), and handles the padding internally.

Apparently, YubiHSM2 doesn't support CKM_RSA_X_509:

[in] hSession = 0x0
pMechanism->type=CKM_RSA_X_509                
[in] hKey = 0x30301
Returned:  112 CKR_MECHANISM_INVALID

I don't think we can change the OpenSSL's implementation from within libp11 or the engine.

@mtrojnar
Copy link
Member

@mouse07410: We already discussed a very similar topic: #70 (comment)

@mouse07410
Copy link
Contributor Author

@mtrojnar yes indeed, thank you for reminding me. It was a great work!

Apparently, YubiHSM2 doesn't support CKM_RSA_X_509

I'm trying to convince the developers of HSM2 to add support for RSA-X-509.

I envision difficulties because HSM2 is capabilities-based. Each key is assigned immutable capabilities, such as asymmetric_sign_pss and/or asymmetric_sign_pkcs. So if they add support for RSA-X-509, one would be unable to restrict the key to PSS signatures only, and use OpenSSL/libp11 with it...

What's your opinion on this problem?

Thanks!

@mtrojnar
Copy link
Member

Allowing CKM_RSA_X_509 (raw RSA) for existing signature-only keys would be a bad idea. It would be better to use separate keys, which have a capability allowing for CKM_RSA_X_509.

An alternative approach would be to extend the OpenSSL engine API to optionally invoke additional mechanisms, such as RSA_PKCS1_PSS_PADDING, directly from an engine. Preferably, engines should be able to declare which mechanisms they directly support for a specific key. We would also need to extend libp11 to translate OpenSSL RSA_PKCS1_PSS_PADDING parameters into PKCS#11 CKM_RSA_PKCS_PSS parameters.

@dengert
Copy link
Member

dengert commented Aug 30, 2017

Here is another approach that does not require OpenSSL mods.

When the engine and/or libp11 sees an encryption to use CKM_RSA_X_509 but the PKCS#11 module does not support it, the block to be encrypted could be examined to see if it is looks like RSA_PKCS1_PSS_PADDING. Since it is not encrypted at this point it might be possible to get the information needed for CKM_RSA_PKCS_PSS parameters much like a verify operation does.

https://tools.ietf.org/html/rfc8017#section-9.1.2
shows what is needed to do a verify EMSA-PSS-VERIFY (M, EM, emBits)

This might require implementing ENGINE_set_digests so as to trap M, EM, and Hash.
But this would need to be done before the engine knew it would be a PSS operation, so every digest would be traped, and at least the last one saved.

(It might be easier to "extend the OpenSSL engine API" as @mtrojnar suggested and get OpenSSL to accept the changes. )

Another approach is to use LD_PRELOAD to replace RSA_padding_add_PKCS1_PSS to pass the parameters to populate the CKM_RSA_PKCS_PSS. But this may not work on all OSes.

But to develop this would require a PKCS#11 module that supported PSS in software or hardware.
And it is not clear if this would work.

@mouse07410 how bad do you need this?

@dengert
Copy link
Member

dengert commented Sep 28, 2017

@mtrojnar Here is another though, looking at the problem from the libp11 viewpoint.

You keep implying there should be a way to add PSS and OAEP to libp11 for users to call. The main issue with both of these is the mechanisms take a parameter. But pkcs11_private_encrypt, pkcs11_private_decrypt only pass in int padding and there is no way to pass in a parameter. (And OAEP also needs a pkcs11_public_encrypt.) pkcs11_mechanism makes a simple assumption based on padding as to the mechanism to be used.

The main issue that all these mods address is that depending on the token's ability to support several mechanisms. If it can support CKM_RSA_X_509 all the padding can be done in software. If it can only support CKM_RSA_PKCS_PSS and/or CKM_RSA_PKCS_OAEP, then all the padding must be done by the token.

The OpenSSL EVP_PKEY code passes to the engine the ability for the engine to choose if the padding can be done in software or hardware. The engine code could pass this choice to be implemented in the pkcs11_* routines based on what the token supports. The choice could be made in the pkcs11_mechanism routine based on the mechanisms supported by the token.

Would you consider an extended version of these routines, that have an additional parameter?

The original 'proof of concept" code made the choice up front then if the token supported CKM_RSA_PKCS_PSS and/or CKM_RSA_PKCS_OAEP it made the PKCS# calls. If not it let OpenSSL do the padding is software when ended up calling the pkcs11_* routines with padding==RSA_NO_PADDING to let the token do CKM_RSA_X_509.

The p11_evp.c would then make up the PSS or OAEP parameters, and call pkcs11_private_decrypt_extended, pkcs11_private_encrypt_extended or pkcs11_public_encrypt_extended.

The pkcs11_private_decrypt, pkcs11_private_encrypt and pkcs11_public_encrypt would just call the extended versions with a parameter=NULL.

@mouse07410
Copy link
Contributor Author

mouse07410 commented Sep 28, 2017

@mtrojnar I've moved the stuff to src/p11_pkey.c (my branch https://github.com/mouse07410/libp11/tree/rsa-oaep-prep)

The problem persists. It works fine with single invocation, and the appropriate pkcs11_pkey_rsa_decrypt() or such methods are called. However when another key is loaded on the same engine, it fails to proceed to the custom methods, and fails in pkcs11_private_decrypt() in p11_rsa.c with mechanism=3, and padding=3 (instead of 4).

I have no idea why after successful first loading of the key (public or private) with all the methods being properly mapped, subsequent loading of the keys find methods "unmapped".

Here's successful log of OpenSSL CLI (only loads key once) with debugging trace:

$ ~/openssl-1.1/bin/openssl pkeyutl -engine pkcs11 -keyform engine -encrypt -in t27.txt -out t27.txt.oaep -pubin -inkey "pkcs11:token=YubiHSM;id=%04%02;type=public" -pkeyopt rsa_padding_mode:oaep -pkeyopt rsa_mgf1_md:sha256 -pkeyopt rsa_oaep_md:sha256
engine "pkcs11" set.
Enter PKCS#11 token PIN for YubiHSM:
p11_pkey.c:454 pkcs11_pkey_rsa_encrypt(): out=0x7f820c015c50 *outlen=512  in=0x7f820c0156a0 inlen=27
p11_pkey.c:483 pkcs11_pkey_rsa_encrypt(): orig_pkey_rsa_encrypt returned 1 (512)
$ ~/openssl-1.1/bin/openssl pkeyutl -engine pkcs11 -keyform engine -decrypt -in t27.txt.oaep -inkey "pkcs11:token=YubiHSM;id=%04%02;type=private" -pkeyopt rsa_padding_mode:oaep -pkeyopt rsa_mgf1_md:sha256 -pkeyopt rsa_oaep_md:sha256
engine "pkcs11" set.
Enter PKCS#11 token PIN for YubiHSM:
p11_pkey.c:297 pkcs11_pkey_rsa_decrypt() out=0x7fdca254f820  *outlen=512 in=0x7fdca254f570 inlen=512
p11_pkey.c:323 padding=4
RSA_OAEP
p11_pkey.c:337 hashAlg=SHA256 mgf=MGF1-SHA256 
p11_pkey.c:509 hashAlg=SHA256 mgf=MGF1-SHA256 
12345678901234567890123456
$ cat t27.txt
12345678901234567890123456
$ 

Here's the failing trace of programmatic loading of the public key and performing (without re-creating the context) a bunch of encryptions, then loading the private key and failing because instead of having the correct (customized/mapped) method invoked it goes to the original one:

$ /tmp/OSSL-Tst3.dst/Users/ur20980/bin/OSSL-Tst3
Using YubiHSM device...

Generated random 256-bit AES key:
33 6a e5 8b 2a 8f 01 fd 6b 7a 8d d6 3d 22 f4 1f 
03 89 33 33 74 9a 8c fb b5 e9 e4 1a 98 54 e6 8e 

Enter PKCS#11 token PIN for YubiHSM:
wrap: created EVP_PKEY_CTX...
Symmetric key encrypted with RSA using RSA-OAEP padding (512 bytes):
2a 54 c4 66 b6 e5 df 68 26 d6 82 c5 d8 ca 4c 76 
7d ac 03 4d 95 16 0d c2 01 02 10 ee 5b 96 7c 92 
. . . . .
72 d5 3a 64 c4 72 05 07 72 6d 39 ae 6c e5 5b b0 

Generated random 256-bit AES key:
bf 7b b0 d1 f3 28 5f 01 34 e6 64 81 d0 e7 40 fa 
80 09 77 6e 06 89 14 e7 17 b1 10 42 31 ef 7c 65 

Symmetric key encrypted with RSA using RSA-OAEP padding (512 bytes):
16 6c fb 0a bc b2 cd 17 96 d5 2b d8 e2 fe a0 41
. . . . .
d8 42 70 9e 46 b4 f5 26 cc b5 bb 00 6d 19 9e 19 

unwrap: loaded privkey of size 512
unwrap: Created EVP_PKEY_CTX...
unwrap: allocating 512 bytes...
p11_rsa.c:162 pkcs11_private_decrypt: mech=3 padding=3
p11_rsa.c:183 pkcs11_private_decrypt: C_DecryptInit() returned 112
p11_rsa.c:193 pkcs11_private_decrypt: C_Decrypt() returned 112
p11_rsa.c:201 mechanism: 3 padding: 3
unwrap: failed to decrypt (rv=-1)
140735726228416:error:82079070:PKCS#11 module:pkcs11_private_decrypt:Mechanism invalid:p11_rsa.c:198:
Decryption failed! (rv=-1 olen=512)

OpenSSL crypto demo completed with errors!

It should be something obvious: something set that shouldn't be set, or something not set that was required for persistence of those mappings. I don't know enough to find that problem reasonably fast, but the symptoms seem very clear.

@mouse07410
Copy link
Contributor Author

mouse07410 commented Sep 28, 2017

@dengert frankly I don't like this:

   pmeth = EVP_PKEY_meth_new(EVP_PKEY_RSA, EVP_PKEY_FLAG_AUTOARGLEN)

because it means the caller is told to allocate the maximum buffer (in this case of 4096-bit RSA keys 512 bytes), while what's really needed is 27 bytes.

My code already handles this correctly. Do you still think we want AUTOARGLEN? Or should I just return it to 0, as both ...decrypt() and ...encrypt() do the right thing?

@dengert
Copy link
Member

dengert commented Sep 28, 2017

@mouse07410 The problem maybe caused by engine_unlocked_finish, as we may be missing a call to
ENGINE_up_ref (or whatever) for every pkey we have.

EVP_PKEY_free calls EVP_PKEY_free_it that calls ENGINE_finish(x->engine); This decrements some ref count.

With a debugger, look to see if pkey->engine is set.

This could be a problem in unmodified code too.

@dengert
Copy link
Member

dengert commented Sep 28, 2017

The problem, is there is one EVP_PKEY_FLAG_AUTOARGLEN flag for all the method entry points. We only replace 2 or 3 of them. All the others are copied from the defaults that expect the flag to have been set. You will need to replace all of them. (Why EVP_PKEY_FLAG_AUTOARGLEN was not the default, I don't know.)

Using the recommended flag will allow you to remove a lot of local code. Other padding methods or RSA RAW will expect 512 byte buffer. Note the man pages say "maximum" so OpenSSL has no problem with wasting some storage in their the default methods.

And the check for out==NULL is done before any of the method routines are called, so your routines should never see out==NULL.

@mouse07410
Copy link
Contributor Author

@mouse07410 The problem maybe caused by engine_unlocked_finish, as we may be missing a call to ENGINE_up_ref (or whatever) for every pkey we have. EVP_PKEY_free calls EVP_PKEY_free_it that calls ENGINE_finish(x->engine); This decrements some ref count.

I hear you (though I don't have understanding deep enough to follow completely). But the engine per se seems to work, and it still receives commands for the keys on the token. It just loses the method mappings that we created, so the operations get misdirected - but I think they stay within the engine.

With a debugger, look to see if pkey->engine is set.

Will try to check.

This could be a problem in unmodified code too.

Possibly. I don't know, as I don't have anything to compare to. Both the unmodified and the modified code seem to work OK with a PIV token, including programmatic access. The unmodified code never worked with the HSM device, and the modified one loses the method mappings after the load of another key... :-(

Using the recommended flag will allow you to remove a lot of local code.

Sure, but that local code doesn't bother me - and it serves as a "guard" against any possible problem with a silly call with a NULL buffer. "Belt and suspenders", if you will. And it's only a few lines of code per method (so approximately 15 lines of code total)... I don't know...

Other padding methods or RSA RAW will expect 512 byte buffer.

That's an important argument. Perhaps overruling most of what I said above related to AUTOARGLEN.

Note the man pages say "maximum" so OpenSSL has no problem with wasting some storage in their the default methods.

Well, we don't have to follow all the mis-features of OpenSSL. ;-)
Still, I see your point.

So you say "keep AUTOARGLEN, and get rid of the local code for NULL buffer"? I'm somewhat reluctant to get rid of that code (let's give it some time), but have no problem keeping AUTOARGLEN in. Thanks!

@mouse07410
Copy link
Contributor Author

mouse07410 commented Sep 29, 2017

@dengert I've did some tracing with the debugger, and I can't say I fully comprehend the results.

  1. e = load_engine("pkcs11"); engine loads fine, it has rsa_meth group named "libp11 RSA method" that contains rsa_pub_enc(), rsa_pub_dec() located in rsa_ossl.c from libcrypto.dylib, and rsa_priv_enc() and rsa_priv_dec() located in p11_rsa.c of pkcs11.dylib. It also has pkey_meths pointer pkey_meths ENGINE_PKEY_METHS_PTR (pkcs11.dylibPKCS11_pkey_meths at p11_pkey.c:123) 0x0000000100b4c4e0`.

  2. Proceeding with OAEP encryption. Since encryption is not done on the token anyway, it works regardless of almost everything.

  3. Proceeding to OAEP decryption. Loading private key: privkey = ENGINE_load_private_key(e, KeyManPrivKey, NULL, &cb_data); and as you suspected, privkey->engine = NULL. IMHO, it worked for PIV tokens, because they require OpenSSL software to do the padding, and the engine is registered as "default" for RSA operations. But when more is required from the engine - like with HSM - it fails.

What do we do now?

Update (with more details than given above)

  • privkey->engine == NULL
  • privkey->pkey->rsa->meth points at the "old" functions like rsa_priv_dec() defined in p11_rsa.c and rsa_pub_enc() defined in OpenSSL in rsa_ossl.c
  • privkey->pkey->rsa->engine == NULL

For the engine itself:

  • e->rsa_meth points at the set of the "old" functions (as above)
  • e->pkey_meths: pkey_meths ENGINE_PKEY_METHS_PTR (pkcs11.dylibPKCS11_pkey_meths at p11_pkey.c:123) 0x0000000100b4c4e0

@dengert
Copy link
Member

dengert commented Sep 29, 2017

It is not clear to me why the first key would work, but not the rest. This could be because of a reference count on the engine. I assume this is your application that is failing. You can set many breakpoints to see
what might be going on. For example set breakpoints for the pkcs11_pkey_rsa_* routines as well as the default pkey_rsa_* routines to see if and when each gets called. Look at the stack trace when you get a break.

Also break on ENGINE_init, ENGINE_get_pkey_meth_engine, EVP_PKEY_up_ref, ENGINE_get_pkey_meth
and int_ctx_new step through this last one and see what it does. See if ENGINE_get_pkey_meth_engine is called with an engine, and from where, and what is the id.

@mtrojnar @mouse07410 ctx_new creates the ENGINE_CTX, but it does does save a pointer to the engine in ENGINE_CTX. Do you see any problem with changing ctx_new() to ctx_new(ENGINE *e)
then adding ctx->engine = e; This might be needed to get the address of the engine in some future code.

With an EVP_PKEY, the modified methods are tied to the engine and will only be found if the EVP_PKEY
(evp_pkey_st in ./crypto/include/internal/evp_int.h.) has an engine.

But I don't see how to set it from ctx_load_key, ctx_load_pubkey, ctx_load_privkey.

@mouse07410 Note in the OpenSSL pkeyutl.c, init_ctx() they use the impl.
They then do:

385         int kdfnid = OBJ_sn2nid(kdfalg);
386         if (kdfnid == NID_undef)
387             goto end;
388         ctx = EVP_PKEY_CTX_new_id(kdfnid, impl);
389     } else {
390         if (pkey == NULL)
391             goto end;
392         *pkeysize = EVP_PKEY_size(pkey);
393         ctx = EVP_PKEY_CTX_new(pkey, impl);
394         EVP_PKEY_free(pkey);
395     }

It is not obvious why they do a EVP_PKEY_free, other then it was copied by the EVP_PKEY_CTX_new.

@mouse07410
Copy link
Contributor Author

mouse07410 commented Sep 29, 2017

@mouse07410 Note in the OpenSSL pkeyutl.c, init_ctx() they use the impl.

Yes. But it is unclear if it's set to "engine" or to "NULL" in our case. And since when I use "pkeyutl" I do not specify "-engine_impl" parameter - I suspect what's passed to EVP_PKEY_CTX_new() is NULL.

@mtrojnar
Copy link
Member

@dengert I don't think we should keep our reference or pointer to the engine. Anyway, let's try to diagnose the issue before we propose a fix.

I'm on my vacation for the next few days with no access to a real keyboard. I will look into it when I return.

@mtrojnar
Copy link
Member

Shouldn't load_privkey() in eng_front.c set the engine property of the returned evp_pkey_st object?

@mouse07410
Copy link
Contributor Author

mouse07410 commented Oct 1, 2017

Shouldn't load_privkey() in eng_front.c set the engine property of the returned evp_pkey_st object?

How would I know? ;-)

I assume this is your application that is failing.

Turns out that when I made a few modifications (see below for details), my app started working correctly.

Current status
When dealing with a PIV device (or a device that supports raw RSA), it doesn't matter whether you create the context with or without engine, i.e., both ctx = EVP_PKEY_CTX_new(pkey, engine); and
ctx = EVP_PKEY_CTX_new(pkey, NULL); worked equally fine.

When dealing with this new (to me) HSM device that does not do raw RSA at all, providing the engine in ctx = EVP_PKEY_CTX_new(pkey, engine); became a necessity. This (and some minor polishing of the code in p11_pkey.c) was sufficient to get encryption and decryption working OK.

With the signature, I found to my displeasure that the old way (that was fine before) doesn't work any more. I.e., the following fails:

            EVP_PKEY_CTX *ctx = NULL;

            privkey = ENGINE_load_private_key(e, SignPrivKey, NULL, &cb_data);
            md = EVP_sha256();
            md_ctx = EVP_MD_CTX_create();
            rv = EVP_DigestSignInit(md_ctx, &ctx, md, e, privkey);
            rv = EVP_PKEY_CTX_set_rsa_padding(ctx, padding);
            EVP_PKEY_CTX_set_rsa_mgf1_md(ctx, md);
            rv =  EVP_PKEY_CTX_set_rsa_pss_saltlen(ctx, -2);
            rv = EVP_DigestSignUpdate(md_ctx, in, inlen);
            olen = 0;
            rv = EVP_DigestSignFinal(md_ctx, NULL, &olen);
            *out = OPENSSL_malloc(&olen);
            rv = EVP_DigestSignFinal(md_ctx, *out, &olen);

I had replace the above with:

    md = EVP_sha256();
    ctx = EVP_PKEY_CTX_new(privkey, e);
    rv = EVP_PKEY_sign_init(ctx);
    rv = EVP_PKEY_CTX_set_rsa_padding(ctx, padding);
    EVP_PKEY_CTX_set_signature_md(ctx, md);
    EVP_PKEY_CTX_set_rsa_mgf1_md(ctx, md);
    EVP_PKEY_CTX_set_rsa_pss_saltlen(ctx, -2);
    rv = EVP_PKEY_sign(ctx, NULL, &olen, in, inlen);
    *out = OPENSSL_malloc(olen);
    rv = EVP_PKEY_sign(ctx, *out, &olen, in, inlen);

which means that now I can only sign data equal in length to the digest size (or pre-hash it myself).

Everything else appears to work fine, both OpenSSL CLI and my code.

Update
Now I'd like to backport this code to OpenSSL-1.0.2. Problem: some methods are missing. In particular:

  • EVP_PKEY_meth_get_sign
  • EVP_PKEY_meth_get_decrypt
  • EVP_PKEY_meth_get_encrypt

The methods they would return live in the structure evp_pkey_method_st. That structure is opaque (not available in a binary OpenSSL-1.0.2 installation), and only lives in crypto/include/internal/evp_int.h.

@mtrojnar are you OK if I include that file into libp11, or rip the structure definition from it and add to src/p11_pkey.h or such? Without that this new capability cannot be backported to 1.0.2, I think.

@dengert
Copy link
Member

dengert commented Oct 1, 2017

Glad you found thectx = EVP_PKEY_CTX_new(pkey, engine); That makes sense, and I bet if you look at the pkeyutil.c it does this too.

In your code:

            md_ctx = EVP_MD_CTX_create();
            rv = EVP_DigestSignInit(md_ctx, &ctx, md, e, privkey);
            rv = EVP_PKEY_CTX_set_rsa_padding(ctx, padding);
            EVP_PKEY_CTX_set_rsa_mgf1_md(ctx, md);
            rv =  EVP_PKEY_CTX_set_rsa_pss_saltlen(ctx, -2);

Did you try moving the EVP_DigestSignInit to after you provide the padding parameters?

            md_ctx = EVP_MD_CTX_create();
            rv = EVP_PKEY_CTX_set_rsa_padding(ctx, padding);
            EVP_PKEY_CTX_set_rsa_mgf1_md(ctx, md);
            rv =  EVP_PKEY_CTX_set_rsa_pss_saltlen(ctx, -2);
            rv = EVP_DigestSignInit(md_ctx, &ctx, md, e, privkey);

The "proof of concept code" took the approach that if the modified routine could not handle the request it would call the unmodified routine. This allowed for future changed in OpenSSL to be handled transparently.

If the modified routine in their do_original could do what the unmodified code would have done, then there is no need to use EVP_PKEY_meth_get_*

Or better still, for 1.0.2 only reproduce the unmodified versions of the three routines, and save pointers to them in the orig_pkey_rsa_*

@mouse07410
Copy link
Contributor Author

mouse07410 commented Oct 1, 2017

Glad you found the ctx = EVP_PKEY_CTX_new(pkey, engine);

:-)

That makes sense, and I bet if you look at the pkeyutil.c it does this too.

Yeah, but AFAIK only when -engine_impl is among the given arguments.

In fact, this -engine_impl came about as a result of my discussion with Mr. Levitte a couple of years ago. ;-)

Did you try moving the EVP_DigestSignInit to after you provide the padding parameters?

I haven't - will try. I didn't try that first because in that case ctx was created by EVP_DigestSignInit() rather than being created explicitly by ctx = EVP_PKEY_CTX_new(). But I'll try and report.

Or better still, for 1.0.2 only reproduce the unmodified versions of the three routines, and save pointers to them in the orig_pkey_rsa_*

By "unmodified" you mean the code in src/p11_rsa.c? rsa_priv_dec() and such?

Thanks!

@dengert
Copy link
Member

dengert commented Oct 1, 2017

You said "I haven't - will try." It looks like it should have worked the way you did it, and not the way I said to do it.
You could step through the code to see what is going on. EVP_DigestSignInit -> do_sigver_init
that does

22     if (ctx->pctx == NULL)
 23         ctx->pctx = EVP_PKEY_CTX_new(pkey, e);

But only till line 59 does it do *pctx = ctx->pctx; So my idea would not work.

You could keep stepping through the code to see where it uses the the pctx. It passes ctx->pctx to a lot of routines that may look for the PSS parameters which are not set yet. So even if you add them they might not be used.

Maybe line before line 22 there needs to be something like

if (pctx && *pctx)
    ctx->pctx = *pctx;

Also note do_sigver_init calls the pmeth->signctx_init and may call EVP_PKEY_sign_init. I don't think we modified that method. we might have too.

The unmodified code was the was the OpenSSL versions of the pkey_rsa_* routines that could be copied to some p11 source file to be used only for 1.0.2.

@dengert
Copy link
Member

dengert commented Oct 1, 2017

You said: "which means that now I can only sign data equal in length to the digest size (or pre-hash it myself)."
What else would you want to sign? PSS is expecting a hash. RSA signatures are always of a hash/digest
unless you use RSA RAW, which your HSM can not do.

@mouse07410
Copy link
Contributor Author

mouse07410 commented Oct 2, 2017

What else [besides hash] would you want to sign?

Well, the thing is that before I was able to feed a data buffer of arbitrary length (smaller or larger than the digest size), and the construct I showed above would automatically hash it. That's why EVP_DigestSignInit() rather than EVP_PKEY_sign_init(). It was convenient.

I understand that nothing stops me from adding a "prequel" step and running digest myself. Still, it's painful to lose convenience one got accustomed to. ;-)

The unmodified code was the was the OpenSSL versions of the pkey_rsa_* routines that could be copied to some p11 source file to be used only for 1.0.2.

Ah... So rip from the OpenSSL-1.0.2 sources the pkey_rsa_* routines, and add them to p11_pkey.c ifdef-ed to only OpenSSL-1.0.2 or earlier? And instead of getting the pointers from the original pmeth, just insert pointers to the ones I'd add to p11_pkey.c? OK, I think I got it...

Though it would be far easier and IMHO better if instead of copying the OpenSSL routines I'd just copy the definition of the evp_pkey_methods_st structure and used it to map members of PKEY_RSA_METHOD *pmeth object. You don't agree that this would be the preferred way?

Update
I've just implemented 1.0.2 support by adding evp_pkey_methods_st structure definition to p11_pkey.c. Total cost is about 25 lines - 19 lines of the structure itself, and three lines to use it (plus a few #ifdefs. I'd say it's the simplest/best way to address it. And I've tested it - it works. ;-!

@dengert
Copy link
Member

dengert commented Oct 2, 2017

OK, EVP_DigestSignInit should do the digest for you and use the engine to do the signature. You have the debugger and can step through the code to see if this is a bug or is not supported. This could be an issue with using a digest engine and the RSA engine and things are getting mixed up. The RSA engine does not support a signctx but does support a sign.

EVP_DigestSignInit is used in apps/dgst.c and req.c That might give you a clue.

With regards to the coping of the evp_pkey_method_st:

I did something similar with the libp11 ECDSA code, in that it needed access to some structures that were not exported. The ball was dropped and my bug report sat for years. So to get some code into libp11, a mod was added that compiled libp11 with the internal OpenSSL header file. This required one to provide a environment variable pointing at the header file. I was trying to be very careful to avoid any licence issues and exposing a structure they wanted to be kept private. (All that code is gone from libp11, as OpenSSL added the needed code so the structure did not need to be exposed.)

Sticking to version 1.1 would avoid the issue altogether. @mtrojnar comment?

@mouse07410
Copy link
Contributor Author

With regards to the coping of the evp_pkey_method_st:

...a mod was added that compiled libp11 with the internal OpenSSL header file. This required one to provide a environment variable pointing at the header file. I was trying to be very careful to avoid any license issues and exposing a structure they wanted to be kept private. (All that code is gone from libp11, as OpenSSL added the needed code so the structure did not need to be exposed.)

So the main concern here is the licensing issues, right? Perhaps @mtrojnar would be willing to accept that structure (and the associated copyright) for the time until 1.0.2 either falls out of use, or OpenSSL team remedies the situation on their end (like they eventually did in your case)?

@dengert
Copy link
Member

dengert commented Oct 2, 2017

Licensing issues (perceived by me at least) and wanting them to accept the patches I proposed or provide their own code.

https://wiki.openssl.org/index.php/OpenSSL_1.1.0_Changes
has
https://wiki.openssl.org/images/e/ed/Openssl-compat.tar.gz
You could submit your changes to OpenSSL to be added to it.

https://www.spinics.net/lists/openssh-unix-dev/msg04241.html
This mentions the *_meth_* functions, but does not address them.

@mouse07410
Copy link
Contributor Author

Submitted a PR to OpenSSL, let's see. If merged, it will be very easy to remove that structure from the code. ;-)

@mtrojnar
Copy link
Member

mtrojnar commented Oct 3, 2017

I'm still on my vacation. 8-)

We don't need to copy internal OpenSSL headers. We only need data structures with the same memory layout. I already implemented such solution to get EC running on older versions of OpenSSL.

@mouse07410
Copy link
Contributor Author

mouse07410 commented Oct 3, 2017

I'm still on my vacation. 8-)

Enjoy! ;-)

We don't need to copy internal OpenSSL headers. We only need data structures with the same memory layout.

OK, excellent. Would it be OK if those data structures would have the same names as the corresponding ones in OpenSSL?

Update
It looks like I'm in luck - the needed change appears to have been pushed to 1.0.2-stable. So as soon as the next release is out, it will hit the distros (and I might be able to now override what Macports has anyway without risk to break other Macports packages).

P.S. @dengert the problem with EVP_DigestSignInit() et co was mine. Modified engine exposed my errors. I've fixed it - no change to the engine or OpenSSL was needed for that.

I spoke too soon... Looks like you were right all along.

mtrojnar added a commit that referenced this issue Oct 5, 2017
mtrojnar added a commit that referenced this issue Oct 5, 2017
@mtrojnar
Copy link
Member

mtrojnar commented Oct 7, 2017

This feature is now implemented. Please open separate issues for each identified defect.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants