-
Notifications
You must be signed in to change notification settings - Fork 183
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
Comments
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?
|
Update
But when I try to use this
I symlinked
Another thing I haven't figured out is how exactly to tie all these together. I.e., how to inform It looks like I have to stick with engine |
Does p11tool show anything for that module? My experience with yubi pkcs11 modules is that they are often incomplete and half baked.
…On August 20, 2017 4:23:37 AM GMT+02:00, Mouse ***@***.***> wrote:
**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: 05684076
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
```
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
|
https://www.yubico.com/products/yubihsm/
says: "Offers encryption with a Message Authentication Code (MAC), HMAC-SHA1 hashing, AES encryption/decryption, and cryptographic True Random Number Generation."
This sounds like it only supports symmetric keys.
AFAIK libp11 and its engine only support the use of asymmetric keys RSA and EC via the engine.
(Sounds like a future project.)
Can you get OpenSC pkcs11-tool to list the mechanisms supported by the /usr/local/lib/yubihsm_pkcs11.so?
I don't see on github.com/yubico a yubihsm so it may not be open source.
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.
This might show it fails to support RSA, which I suspect is why it the engine code is failing.
…On 8/18/2017 3:45 PM, Mouse wrote:
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 <https://github.com/nmav> perhaps you could step in too?
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub <#176>, or mute the thread
<https://github.com/notifications/unsubscribe-auth/AA00MV7a1Ka5zESct7b1K7hycuUBxCEfks5sZffigaJpZM4O8BoA>.
--
Douglas E. Engert <[email protected]>
|
No argument from me here! ;-)
Yep:
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).
Yep, you bet:
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?
No, not yet at least (but that's up to Yubico to decide).
To do that I would need to set |
It looks like you have two ways to use the yubihsm with OpenSSL as an engine.
Is libyhsm2.dylib an engine module? Does it call PKCS#11? The other choice is to use libp11's engine, and let it use p11-kit or a PKCS#11 module. But I don't think symmetric keys or random are supported by libp11 either. |
Wel, that's my ugly hack. I took
All the normal things - it's a copy of working libp11... The MacOS version is
Since my current installation has libp11 using
Could you give an example of a symmetric mechanism, so I know what to look/check for? I.e., how would I tell Thanks! |
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. |
@Mouse you asked: "what would pkcs11-tool -M report for symmetric mechanisms: pkcs11-tool.c maps CKM_* via the You said you were working with some beta version of the yubihsm. https://www.yubico.com/products/yubihsm/ So why does it not support AES? (If the module supported it, pkcs11-tool -M should have listed it.) 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. |
@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. |
Yep. It is pre-production. Exact specs not finalized yet.
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.
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? |
@mouse07410 See: I would suggest Yubico use 59554200 for YUB This gives then up to 256 different types. 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. |
@dengert so if I understand correctly, you suggest
Correct? (If so, I will certainly pass this to Yubico, and hopefully they'd do the right thing...) |
Yes. |
Except we were talking mechanisms, so it would be: |
Submitted. Let's wait and see now. ;-) |
I can close this issue. Here's what I learned:
The problem: YubiHSM2 can sign using RSA-PSS, for example with SHA384-RSA-PKCS-PSS algorithm, via
Doing the same thing with Yubikey NEO works:
Verifying SHA384-RSA-PKCS-PSS signature made by
RSASSA PKCS v1.5 signatures also work with YubiHSM2:
Any recommendations here? |
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 Run both PIV and YubiHSM with SPY traces to see what is going on. |
And it looks like p11 does not support PSS. |
What I think you're saying is:
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)?
@mtrojnar what would it take to add
Will do and post here. Update Failing RSA-PSS spy log with YubiHSM2: @mtrojnar So what I'm essentially after is |
The spy logs confirm what I found during my investigation of the OpenSSL source code in March 2016: Apparently, YubiHSM2 doesn't support CKM_RSA_X_509:
I don't think we can change the OpenSSL's implementation from within libp11 or the engine. |
@mouse07410: We already discussed a very similar topic: #70 (comment) |
@mtrojnar yes indeed, thank you for reminding me. It was a great work!
I'm trying to convince the developers of HSM2 to add support for I envision difficulties because HSM2 is capabilities-based. Each key is assigned immutable capabilities, such as What's your opinion on this problem? Thanks! |
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. |
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 This might require implementing ENGINE_set_digests so as to trap M, EM, and Hash. (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. @mouse07410 how bad do you need this? |
@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 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 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 The |
@mtrojnar I've moved the stuff to The problem persists. It works fine with single invocation, and the appropriate 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:
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:
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. |
@dengert frankly I don't like this:
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 |
@mouse07410 The problem maybe caused by engine_unlocked_finish, as we may be missing a call to 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. |
The problem, is there is one 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. |
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.
Will try to check.
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... :-(
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...
That's an important argument. Perhaps overruling most of what I said above related to AUTOARGLEN.
Well, we don't have to follow all the mis-features of OpenSSL. ;-) 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! |
@dengert I've did some tracing with the debugger, and I can't say I fully comprehend the results.
What do we do now? Update (with more details than given above)
For the engine itself:
|
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 Also break on ENGINE_init, ENGINE_get_pkey_meth_engine, EVP_PKEY_up_ref, ENGINE_get_pkey_meth @mtrojnar @mouse07410 With an EVP_PKEY, the modified methods are tied to the engine and will only be found if the EVP_PKEY 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.
It is not obvious why they do a EVP_PKEY_free, other then it was copied by the EVP_PKEY_CTX_new. |
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 |
@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. |
Shouldn't |
How would I know? ;-)
Turns out that when I made a few modifications (see below for details), my app started working correctly. Current status When dealing with this new (to me) HSM device that does not do raw RSA at all, providing the engine in 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:
I had replace the above with:
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
The methods they would return live in the structure @mtrojnar are you OK if I include that file into |
Glad you found the In your code:
Did you try moving the EVP_DigestSignInit to after you provide the padding parameters?
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 Or better still, for 1.0.2 only reproduce the unmodified versions of the three routines, and save pointers to them in the |
:-)
Yeah, but AFAIK only when In fact, this
I haven't - will try. I didn't try that first because in that case
By "unmodified" you mean the code in Thanks! |
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.
But only till line 59 does it do 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
Also note do_sigver_init calls the 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. |
You said: "which means that now I can only sign data equal in length to the digest size (or pre-hash it myself)." |
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 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. ;-)
Ah... So rip from the OpenSSL-1.0.2 sources the Though it would be far easier and IMHO better if instead of copying the OpenSSL routines I'd just copy the definition of the Update |
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 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? |
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)? |
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 https://www.spinics.net/lists/openssh-unix-dev/msg04241.html |
Submitted a PR to OpenSSL, let's see. If merged, it will be very easy to remove that structure from the code. ;-) |
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. |
Enjoy! ;-)
OK, excellent. Would it be OK if those data structures would have the same names as the corresponding ones in OpenSSL? Update P.S. @dengert I spoke too soon... Looks like you were right all along. |
This feature is now implemented. Please open separate issues for each identified defect. |
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
orlibp11
or OpenSSL so thatlibp11
would use the above library upon request?Thank you!
@nmav perhaps you could step in too?
The text was updated successfully, but these errors were encountered: