Bug 1710832 - Fedora patch for PKCS#11-URI should not require optional CKA_LABEL attribute
Summary: Fedora patch for PKCS#11-URI should not require optional CKA_LABEL attribute
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: openssh
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Jakub Jelen
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 2186838
TreeView+ depends on / blocked
 
Reported: 2019-05-16 12:07 UTC by thomas
Modified: 2023-04-14 15:35 UTC (History)
9 users (show)

Fixed In Version: openssh-8.0p1-3.fc30
Clone Of:
: 2186838 (view as bug list)
Environment:
Last Closed: 2019-06-08 00:58:00 UTC
Type: Bug
Embargoed:


Attachments (Terms of Use)

Description thomas 2019-05-16 12:07:56 UTC
Description of problem:
This[1] patch adds code that gets the attribute CKA_LABEL form the PKCS#11 module, which is fine.
But it shouldn't *require* it. A missing label is not an error.

I'm not a PKCS#11 expert, but the presence of the code:
        if (label_attrib->ulValueLen > 0 ) {
tells me that other parts of the patch treat this attribute as optional.

The offending parts of the patch are:

```
@@ -729,18 +863,19 @@ pkcs11_fetch_ecdsa_pubkey(struct pkcs11_provider *p, CK_ULONG slotidx,
         * XXX assumes CKA_ID is always first.
         */
        if (key_attr[1].ulValueLen == 0 ||
-           key_attr[2].ulValueLen == 0) {
+           key_attr[2].ulValueLen == 0 ||
+           key_attr[3].ulValueLen == 0) {
                error("invalid attribute length");
                return (NULL);
        }
```

```
@@ -850,18 +987,19 @@ pkcs11_fetch_rsa_pubkey(struct pkcs11_provider *p, CK_ULONG slotidx,
         * XXX assumes CKA_ID is always first.
         */
        if (key_attr[1].ulValueLen == 0 ||
-           key_attr[2].ulValueLen == 0) {
+           key_attr[2].ulValueLen == 0 ||
+           key_attr[3].ulValueLen == 0) {
                error("invalid attribute length");
                return (NULL);
        }
```

key_attr[1].ulValueLen (CKA_LABEL) should *NOT* be checked for zero-length.


Version-Release number of selected component (if applicable):


How reproducible:


Steps to Reproduce:
1. Install https://github.com/ThomasHabets/simple-tpm-pk11
2. Set up a key per its README
3. ssh  -v -v -v -oPKCS11Provider=/usr/local/lib/libsimple-tpm-pk11.so shell.example.com

Actual results:
Get errors:
"invalid attribute length" and "failed to fetch key" because Fedora OpenSSH requires CKA_LABEL.

Expected results:
It to "just work" for login, like with vanilla OpenSSH.


Additional info:

This broke simple-tpm-pk11:
https://github.com/ThomasHabets/simple-tpm-pk11/issues/48

[1] https://src.fedoraproject.org/rpms/openssh/blob/master/f/openssh-8.0p1-pkcs11-uri.patch

Comment 1 Jakub Jelen 2019-05-16 13:15:32 UTC
Thank you very much for the bug report. You are right, this was mistake. I will fix it with the next update of openssh.

Comment 2 thomas 2019-05-16 13:45:35 UTC
Thanks.

Right, I should add that in order to reproduce this with simple-tpm-pk11 you need to undo the workaround: https://github.com/ThomasHabets/simple-tpm-pk11/commit/949700211ab6f1c28a863c7a94dbd73eb99b0f41

Comment 3 Fedora Update System 2019-05-27 08:59:31 UTC
openssh-8.0p1-3.fc30 has been submitted as an update to Fedora 30. https://bodhi.fedoraproject.org/updates/FEDORA-2019-ba5e7fffc5

Comment 4 Fedora Update System 2019-05-28 02:27:53 UTC
openssh-8.0p1-3.fc30 has been pushed to the Fedora 30 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2019-ba5e7fffc5

Comment 5 Fedora Update System 2019-06-08 00:58:00 UTC
openssh-8.0p1-3.fc30 has been pushed to the Fedora 30 stable repository. If problems still persist, please make note of it in this bug report.

Comment 6 Max P 2023-04-05 19:12:00 UTC
Hello. I got same error on Centos 8-stream and Fedora 37, if token has only Private key and Certificate, without Public key:

debug1: identity file 'pkcs11:?module-path=/usr/lib64/libeToken.so' from pkcs#11
debug1: pkcs11_add_provider_by_uri: called, provider_uri = pkcs11:?module-path=/usr/lib64/libeToken.so
debug1: provider /usr/lib64/libeToken.so: manufacturerID <SafeNet, Inc.> cryptokiVersion 2.20 libraryDescription <SafeNet eToken PKCS#11> libraryVersion 10.8
debug1: provider pkcs11:?module-path=/usr/lib64/libeToken.so slot 0: label <Abcd Efgh> manufacturerID <SafeNet, Inc.> model <eToken> serial <0111a11b> flags 0x60d
invalid attribute length
failed to fetch key

I made a small patch (for centos 8-stream openssh-8.0):

@@ -1082,8 +1082,7 @@
         * ensure that none of the others are zero length.
         * XXX assumes CKA_ID is always first.
         */
-       if (cert_attr[1].ulValueLen == 0 ||
-           cert_attr[2].ulValueLen == 0 ||
+       if (cert_attr[2].ulValueLen == 0 ||
            cert_attr[3].ulValueLen == 0) {
                error("invalid attribute length");
                return (NULL);

Now, it works as expected.

Comment 7 Jakub Jelen 2023-04-11 09:58:23 UTC
(In reply to Max P from comment #6)
> Hello. I got same error on Centos 8-stream and Fedora 37, if token has only
> Private key and Certificate, without Public key:
> 
> debug1: identity file 'pkcs11:?module-path=/usr/lib64/libeToken.so' from
> pkcs#11
> debug1: pkcs11_add_provider_by_uri: called, provider_uri =
> pkcs11:?module-path=/usr/lib64/libeToken.so
> debug1: provider /usr/lib64/libeToken.so: manufacturerID <SafeNet, Inc.>
> cryptokiVersion 2.20 libraryDescription <SafeNet eToken PKCS#11>
> libraryVersion 10.8
> debug1: provider pkcs11:?module-path=/usr/lib64/libeToken.so slot 0: label
> <Abcd Efgh> manufacturerID <SafeNet, Inc.> model <eToken> serial <0111a11b>
> flags 0x60d
> invalid attribute length
> failed to fetch key
> 
> I made a small patch (for centos 8-stream openssh-8.0):
> 
> @@ -1082,8 +1082,7 @@
>          * ensure that none of the others are zero length.
>          * XXX assumes CKA_ID is always first.
>          */
> -       if (cert_attr[1].ulValueLen == 0 ||
> -           cert_attr[2].ulValueLen == 0 ||
> +       if (cert_attr[2].ulValueLen == 0 ||
>             cert_attr[3].ulValueLen == 0) {
>                 error("invalid attribute length");
>                 return (NULL);
> 
> Now, it works as expected.

Please, open an bug for RHEL8 so this can be handled in RHEL8 too. Using clone button above should do that.


Note You need to log in before you can comment on or make changes to this bug.