pkcs11-tool: symmetric encrypt/decrypt #2268

popovec · 2021-03-21T06:59:22Z

supported mechanisms: AES-ECB, AES-CBC, AES-CBC-PAD
support for initialization vector

Tested - softhsm2, (test-pkcs11-tool-sym-crypt-test.sh)

modified:   doc/tools/pkcs11-tool.1.xml
modified:   src/tools/pkcs11-tool.c
modified:   tests/Makefile.am
new file:   tests/test-pkcs11-tool-sym-crypt-test.sh

Jakuje

noticed just couple of typos, but otherwise looks good.

src/tools/pkcs11-tool.c

tests/test-pkcs11-tool-sym-crypt-test.sh

popovec · 2021-03-21T10:06:29Z

@Jakuje Thanks for the review, I will do a force push with more features - key unwrap and wrap.

Jakuje · 2021-03-22T07:22:15Z

doc/tools/pkcs11-tool.1.xml

+				</varlistentry>
+				<varlistentry>
+					<term>
+						<option>--Unwrap</option>,


lowercase 'u'

Thanks for the review, I'll fix it.

But I have question.. Is command line format (for unwrap) acceptable (especially use of --application-id --application-label)? Use key id 22 to unwrap file input.key as AES key with ID 90 and label unwraped-key:

pkcs11-tool --unwrap --mechanism RSA-PKCS --id 22 -i input.key --key-type AES: --application-id 90 --applicatin-label unwraped-key

If appropriate, I can add another switch for specification of the unwrapped key.

For the wrap operation, I have similar question.. I suggest the following command line format (use key id 22 to wrap key id 90):

pkcs11-tool --wrap --mechanism RSA-PKCS --id 22 --application-id 90 -o output.key

I'll add the the wrap code soon...

(sorry for a delay)
I do not like much the reuse of applicatin-id here. This is originally used for data objects. I think we will need a new switches for this not to confuse users. But I am not sure what would be the best terminology. --new-id and --new-label or --wrapped-key-id and --wrapped-key-label are just some ideas.

I am aware that --application_id and --application_labelis not used in its original meaning. As an alternative, I still like it the most:

a) unwrap: use the --new-label and --new-id switches (alternative --new-key-label, --new-key-id)
example:

pkcs11-tool --unwrap --mechanism RSA-PKCS --id 22 -i input.key --key-type AES: --new-key-id 90 --new-key-label unwraped-key

b) wrap: it is possible to use the keyword --wrap with an argument, where either the id or label of the key that will be the subject of the wrapping can be used.

example:

pkcs11-tool --wrap 90 --mechanism RSA-PKCS --id 22 -o output.key pkcs11-tool --wrap key_sign1 --mechanism RSA-PKCS --id 22 -o output.key

I would like other OpenSC developers to comment on this before applying this PR.

Thanks.

@frankmorgner what do you think?

doc/tools/pkcs11-tool.1.xml

src/tools/pkcs11-tool.c

Jakuje · 2021-04-01T08:54:51Z

tests/test-pkcs11-tool-unwrap-wrap-test.sh

+    exit 77;
+fi
+# The Ubuntu has old softhsm version not supporting this feature
+grep "Ubuntu 18.04" /etc/issue && echo "WARNING: Not supported on Ubuntu 18.04" && exit 77


Travis already supports also Ubuntu 20.04: https://docs.travis-ci.com/user/reference/focal/ Does it already work there?

Would it make sense to try to migrate the CI to new version or add one more target which would execute also this test case? Or just write a new one in github actions, which should allow running in more up-to-date systems?

We have new Github Actions in place already, based on Ubuntu 20 so this should work there.

popovec · 2021-04-02T08:09:47Z

Tests for symmetric encryption and wrap / unwrap operations are fully functional, tested on Ubuntu 20, against softhsm2.
More at https://travis-ci.org/github/OpenSC/OpenSC/jobs/765595173

One of the tests that is not affected by this PR does not work (FAIL: test-pkcs11-tool-allowed-mechanisms.sh).
The bug is probably on the softhsm2 side, probably a bug in the key listing. The following modification works for me on the Debian 10 system:

diff --git a/tests/test-pkcs11-tool-allowed-mechanisms.sh b/tests/test-pkcs11-tool-allowed-mechanisms.sh
index 4c9c62d5..7dcb5590 100755
--- a/tests/test-pkcs11-tool-allowed-mechanisms.sh
+++ b/tests/test-pkcs11-tool-allowed-mechanisms.sh
@@ -22,7 +22,7 @@ MECHANISMS="RSA-PKCS,SHA1-RSA-PKCS,RSA-PKCS-PSS"
 # Generate key pair
 $PKCS11_TOOL --keypairgen --key-type="RSA:" --login --pin=$PIN \
        --module="$P11LIB" --label="test" --id="$ID" \
-       --allowed-mechanisms="$MECHANISMS"
+       --allowed-mechanisms="${MECHANISMS},SHA384-RSA-PKCS"
 assert $? "Failed to Generate RSA key pair"
 
 # Check the attributes are visible

If necessary, I can include it in this PR.

Jakuje · 2021-04-06T10:42:23Z

Thank you for checking the new version. It looks like some bug in softhsm in Ubuntu 20. Locally in Fedora 34 it works for me fine (with or without your patch) so please, include it in the PR so we have it passing in CI.

I also see that even though the check fails, the pipeline passes, which is wrong. If I am right, it is because of the log is printed, which mangles the exit code. Something like this should address this problem:

diff --git a/.travis.yml b/.travis.yml
index e449f182..b87751d7 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -251,7 +251,7 @@ script:
       fi;
     fi
   - if [ -z "$HOST" -a "${DO_COVERITY_SCAN}" != "yes" -a -z "$DO_SIMULATION" ]; then
-      make check && make dist || cat tests/*log;
+      make check && make dist || (cat tests/*log src/tests/unittests/*log && exit 1);
     fi
   - if [ ! -z "$HOST" -a "${DO_COVERITY_SCAN}" != "yes" ]; then
       make install;
@@ -336,7 +336,7 @@ script:
     fi
   - if [ "${DO_SIMULATION}" = "cac" ]; then
       cd $TRAVIS_BUILD_DIR;
-      make check && sudo make install || (cat tests/*log src/tests/unittests/*log && exit);
+      make check && sudo make install || (cat tests/*log src/tests/unittests/*log && exit 1);
       export LD_LIBRARY_PATH=/usr/local/lib;
       cd src/tests/p11test/;
       ./p11test -s 0 -p 12345678 -i &

…h error Thanks to Jakub Jelen, OpenSC#2268 (comment)

Jakuje · 2021-08-24T12:46:59Z

Can you rebase the changes on current master, which introduced HMAC sigh&verify to pkcs11-tool? Otherwise I think this is good to go for the next release.

supported mechanisms: AES-ECB, AES-CBC, AES-CBC-PAD support for initialization vector Tested - softhsm2, (test-pkcs11-tool-sym-crypt-test.sh) modified: doc/tools/pkcs11-tool.1.xml modified: src/tools/pkcs11-tool.c modified: tests/Makefile.am new file: tests/test-pkcs11-tool-sym-crypt-test.sh

support for unwrapping GENERIC SECRET / AES key Tested: softhsm, RSA-PKCS mechanism. Tested: MyEID 4.5.5, only partial test due to missing support for symmetric encryption/decryption modified: doc/tools/pkcs11-tool.1.xml modified: src/tools/pkcs11-tool.c modified: tests/Makefile.am new file: tests/test-pkcs11-tool-unwrap-test.sh

support for key wrap operation Tested: softhsm, GENERIC SECRET wrapped by RSA-PKCS mechanism modified: doc/tools/pkcs11-tool.1.xml modified: src/tools/pkcs11-tool.c modified: tests/Makefile.am renamed: tests/test-pkcs11-tool-unwrap-test.sh -> tests/test-pkcs11-tool-unwrap-wrap-test.sh

Use C_DecryptUpdate/C_EncryptUpdate for large input files. modified: src/tools/pkcs11-tool.c modified: tests/test-pkcs11-tool-sym-crypt-test.sh

modified: .github/test-oseid.sh

When support for unwrapping secret keys was added in 9136878 (as part of OpenSC#2268), the `--usage-decrypt` and `--usage-wrap` options were used to toggle the `CKA_{ENCRYPT,DECRYPT}` and `CKA_{WRAP,UNWRAP}` attributes in the object template passed to the module. In contrast, when a secret key object is generated (`--keygen`) or created (`--write-object`), the same attributes are unconditionally set to true or omitted respectively, regardless of any specified `--usage-*` option. To make this handling consistent, use the approach introduced by the unwrap command and let the user specify the attributes, defaulting to only setting `CKA_{ENCRYPT,DECRYPT}` if no usage was specified. The documentation was amended to reflect the behavior of `--usage-decrypt`.

When support for unwrapping secret keys was added in 9136878 (as part of #2268), the `--usage-decrypt` and `--usage-wrap` options were used to toggle the `CKA_{ENCRYPT,DECRYPT}` and `CKA_{WRAP,UNWRAP}` attributes in the object template passed to the module. In contrast, when a secret key object is generated (`--keygen`) or created (`--write-object`), the same attributes are unconditionally set to true or omitted respectively, regardless of any specified `--usage-*` option. To make this handling consistent, use the approach introduced by the unwrap command and let the user specify the attributes, defaulting to only setting `CKA_{ENCRYPT,DECRYPT}` if no usage was specified. The documentation was amended to reflect the behavior of `--usage-decrypt`.

When support for unwrapping secret keys was added in 9136878 (as part of OpenSC#2268), the `--usage-decrypt` and `--usage-wrap` options were used to toggle the `CKA_{ENCRYPT,DECRYPT}` and `CKA_{WRAP,UNWRAP}` attributes in the object template passed to the module. In contrast, when a secret key object is generated (`--keygen`) or created (`--write-object`), the same attributes are unconditionally set to true or omitted respectively, regardless of any specified `--usage-*` option. To make this handling consistent, use the approach introduced by the unwrap command and let the user specify the attributes, defaulting to only setting `CKA_{ENCRYPT,DECRYPT}` if no usage was specified. The documentation was amended to reflect the behavior of `--usage-decrypt`.

Jakuje reviewed Mar 21, 2021

View reviewed changes

popovec force-pushed the pkcs11-tool-sym-crypt branch from 355ac6c to 09d068b Compare March 21, 2021 16:26

Jakuje reviewed Mar 22, 2021

View reviewed changes

popovec force-pushed the pkcs11-tool-sym-crypt branch 2 times, most recently from 98b8d43 to e9dc9dc Compare March 27, 2021 18:28

Jakuje reviewed Apr 1, 2021

View reviewed changes

popovec force-pushed the pkcs11-tool-sym-crypt branch 3 times, most recently from 06810ef to b0d1db1 Compare April 2, 2021 07:19

popovec added a commit to popovec/OpenSC that referenced this pull request Apr 6, 2021

travis CI: if "make check" did not pass, then terminate the build wit…

7965cf0

…h error Thanks to Jakub Jelen, OpenSC#2268 (comment)

popovec force-pushed the pkcs11-tool-sym-crypt branch 3 times, most recently from f865bfe to 0a03a6d Compare April 13, 2021 14:28

popovec mentioned this pull request May 21, 2021

Towards new release 0.22.0 #2247

Closed

popovec added 5 commits August 24, 2021 16:03

pkcs11-tool: encrypt/decrypt - removed fixed limit for data size

d3b2222

Use C_DecryptUpdate/C_EncryptUpdate for large input files. modified: src/tools/pkcs11-tool.c modified: tests/test-pkcs11-tool-sym-crypt-test.sh

github actions: pkcs11-tool - wrap/unwrap tests (using OsEID simulator)

3ceba4b

modified: .github/test-oseid.sh

popovec force-pushed the pkcs11-tool-sym-crypt branch from 0a03a6d to 3ceba4b Compare August 24, 2021 14:45

frankmorgner approved these changes Aug 31, 2021

View reviewed changes

bgermann mentioned this pull request Sep 13, 2021

cli: symmetric encryption provider support randombit/botan#2402

Open

Jakuje merged commit 8b94d04 into OpenSC:master Nov 19, 2021

LDVG mentioned this pull request Jan 26, 2022

pkcs11-tool: consistent handling of secret key attributes #2497

Merged

5 tasks

xhanulik mentioned this pull request Jul 12, 2022

Prepare for release 0.23.0 #2426

Closed

pkcs11-tool: symmetric encrypt/decrypt #2268

pkcs11-tool: symmetric encrypt/decrypt #2268

Uh oh!

Conversation

popovec commented Mar 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Jakuje left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

popovec commented Mar 21, 2021

Uh oh!

Jakuje Mar 22, 2021

Choose a reason for hiding this comment

Uh oh!

popovec Mar 22, 2021

Choose a reason for hiding this comment

Uh oh!

Jakuje Aug 24, 2021

Choose a reason for hiding this comment

Uh oh!

popovec Aug 31, 2021

Choose a reason for hiding this comment

Uh oh!

Jakuje Sep 1, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Jakuje Apr 1, 2021

Choose a reason for hiding this comment

Uh oh!

Jakuje Aug 24, 2021

Choose a reason for hiding this comment

Uh oh!

popovec commented Apr 2, 2021

Uh oh!

Jakuje commented Apr 6, 2021

Uh oh!

Jakuje commented Aug 24, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

popovec commented Mar 21, 2021 •

edited

Loading