Skip to content

pkcs11-tool to use OpenSSL libctx #2715

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.

Sign up for GitHub

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

Jump to bottom
Merged
Jakuje merged 3 commits into from
Mar 20, 2023
Merged

pkcs11-tool to use OpenSSL libctx #2715

Jakuje merged 3 commits into from
Mar 20, 2023

Conversation

@dengert
Copy link
Member

@dengert dengert commented Feb 21, 2023

This is in addition to "Introduce use of custom ossl libctx with OpenSSL >= 3.0" #2712

pkcs11-tool uses some functions found in libopensc, but does not create a sc_context like other OpenSC tools as the pkcs11 module can be any pkscs11 module.

There is one OpenSSL function "d2i_PUBKEY_bio" that does not have an equivalent "d2i_PUBKEY_ex_bio" in 3.0.8. It is listed in OpenSSL master. See: openssl/openssl#18427

On branch ossl_lib_ctx-pkcs11-tool
Changes to be committed:
modified: src/tools/pkcs11-tool.c

Checklist

@dengert
Copy link
Member Author

dengert commented Feb 21, 2023

https://github.com/simo5 can you have a look at this PR. And if you want, add to #2712?

@simo5
Copy link
Contributor

simo5 commented Feb 21, 2023 •
edited
Loading

I am in favor of this if you find it useful, but pkcs11-tool is an end-user applciation, so it can definitely use the default context as its use will never conflict with anything else (except bad libraries perhaps).

Jakuje
Jakuje reviewed Feb 23, 2023
View reviewed changes
dengert added a commit to dengert/OpenSC that referenced this pull request Feb 23, 2023
@dengert
pkcs11-tool.c comments on d2i_PUBKEY_ex_bio
f216aa8 
Requested OpenSC#2715 (review)

 On branch ossl_lib_ctx-pkcs11-tool
 Changes to be committed:
	modified:   src/tools/pkcs11-tool.c
dengert added 2 commits March 8, 2023 07:05
@dengert
pkcs11-tool to use OpenSSL libctx
5270315 
This is in addition to "Introduce use of custom ossl libctx with OpenSSL >= 3.0"
OpenSC#2712

pkcs11-tool uses some functions found in libopensc, but does not create a sc_context
like other OpenSC tools as the pkcs11 module can be any pkscs11 module.

There is one OpenSSL function "d2i_PUBKEY_bio" that does not have an equivalent
"d2i_PUBKEY_ex_bio" in 3.0.8.  It is listed in OpenSSL master.
See: openssl/openssl#18427

 On branch ossl_lib_ctx-pkcs11-tool
 Changes to be committed:
	modified:   src/tools/pkcs11-tool.c
@dengert
pkcs11-tool.c comments on d2i_PUBKEY_ex_bio
2e0cd43 
Requested OpenSC#2715 (review)

 On branch ossl_lib_ctx-pkcs11-tool
 Changes to be committed:
	modified:   src/tools/pkcs11-tool.c
@dengert dengert mentioned this pull request Mar 8, 2023
Merged
3 tasks
Jakuje
Jakuje approved these changes Mar 8, 2023
View reviewed changes
Copy link
Member

@Jakuje Jakuje left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

two minor comments, but generally looks ok. I did not check though all the OpenSSL uses.

src/tools/pkcs11-tool.c Outdated
#endif

#if OPENSSL_VERSION_NUMBER >= 0x30000000L
if (!osslctx) {
Copy link
Member

@Jakuje Jakuje Mar 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we are in main. Is this check needed? I think in this case, it will be always true, unless we will introduce some wild goto logic. The same for the default_provider check below

Copy link
Member Author

@dengert dengert Mar 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests could be removed if you want.

src/tools/pkcs11-tool.c Outdated
util_fatal("cannot parse EC_PARAMS");
EVP_PKEY_assign_EC_KEY(pkey, ec);
#else
/* TODO needs debugging */
Copy link
Member

@Jakuje Jakuje Mar 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this mean that it does not work now or something else? The comment is not much self-explanatory.

Copy link
Member Author

@dengert dengert Mar 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was meant that I did not test this and pkcs11-tool --test does not test ECDSA. I will try pkcs11-tool -sign with an ec key and verify the signature.

@dengert
pkcs11-tool.c
e183a0c 
Address comments

 On branch ossl_lib_ctx-pkcs11-tool
 Changes to be committed:
	modified:   pkcs11-tool.c
@dengert dengert force-pushed the ossl_lib_ctx-pkcs11-tool branch from f216aa8 to e183a0c Compare March 8, 2023 18:07
@dengert
Copy link
Member Author

dengert commented Mar 8, 2023

I addressed the two comments from above and forced pushed an update.

Also tested with OpenSSL 3.0.8 and development built with "no-deprecated":
/usr/bin/perl ../src/Configure --prefix=/opt/ossl-dev_ND -g -Wl,--enable-new-dtags,-rpath=$(LIBRPATH) -g no-deprecated linux-x86_64 shared

#!/bin/bash
set -xv 

# test pkcs11-tool to sign and verify with ECDSA
# Using NIST Beta Test Card 5

PATH=/opt/ossl-dev_ND/bin:$PATH

echo "Hello world!" > /tmp/sign-demo.data.txt

gdb --args pkcs11-tool --sign --login --id 01  --mechanism ECDSA-SHA256 --input /tmp/sign-demo.data.txt \
        --output-file /tmp/sign-demo.signature --signature-format openssl

pkcs11-tool --verify --id 01 --mechanism ECDSA-SHA256 --input /tmp/sign-demo.data.txt \
        --signature-file /tmp/sign-demo.signature --signature-format openssl

pkcs15-tool --read-public-key 01  > /tmp/sign-demo.pubkey.pem

openssl dgst -sha256  -verify /tmp/sign-demo.pubkey.pem \
        -signature /tmp/sign-demo.signature < /tmp/sign-demo.data.txt

Jakuje
Jakuje approved these changes Mar 8, 2023
View reviewed changes
Copy link
Member

@Jakuje Jakuje left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm. Thanks for the help!

@Jakuje Jakuje merged commit 58a1cbd into OpenSC:master Mar 20, 2023
Jakuje pushed a commit that referenced this pull request Mar 20, 2023
@dengert @Jakuje
pkcs11-tool.c comments on d2i_PUBKEY_ex_bio
540f3da 
Requested #2715 (review)

 On branch ossl_lib_ctx-pkcs11-tool
 Changes to be committed:
	modified:   src/tools/pkcs11-tool.c
@xhanulik xhanulik mentioned this pull request Jul 3, 2023
Closed
@Jakuje Jakuje mentioned this pull request Jul 16, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Jakuje Jakuje Jakuje approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@dengert @simo5 @Jakuje