Bug 1538491 - certid option of PKINIT plugin does not handle leading zeros
Summary: certid option of PKINIT plugin does not handle leading zeros
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise Linux 7
Classification: Red Hat
Component: krb5
Version: 7.4
Hardware: Unspecified
OS: Unspecified
high
high
Target Milestone: rc
: ---
Assignee: Robbie Harwood
QA Contact: Patrik Kis
URL: https://github.com/krb5/krb5/pull/723
Whiteboard:
Depends On:
Blocks: 1550132 1565517
TreeView+ depends on / blocked
 
Reported: 2018-01-25 08:18 UTC by Sumit Bose
Modified: 2018-10-30 08:08 UTC (History)
10 users (show)

Fixed In Version: krb5-1.15.1-20.el7
Doc Type: Bug Fix
Doc Text:
Cause: krb5 uses openssl bignums for storing cert ids from smartcards. Consequence: smartcards with certids that have a leading 0 were not properly handled. Fix: krb5 uses its own hex decoding straight to binary, and skips bignum processing completely. Result: leading 0s on certids are now handled.
Clone Of:
: 1565517 (view as bug list)
Environment:
Last Closed: 2018-10-30 08:08:00 UTC


Attachments (Terms of Use)
Patch to properly handle leading zero in the certid option (2.75 KB, patch)
2018-01-25 20:30 UTC, Sumit Bose
no flags Details | Diff
Patch with some simple tests for hex_string_to_bin() (2.58 KB, patch)
2018-01-25 20:31 UTC, Sumit Bose
no flags Details | Diff


Links
System ID Priority Status Summary Last Updated
Red Hat Product Errata RHSA-2018:3071 None None None 2018-10-30 08:08:49 UTC

Description Sumit Bose 2018-01-25 08:18:45 UTC
Description of problem:

If the ID of a certificate on a Smartcard starts with a zero (0x00) it cannot be selected with the certid option because the PKINIT plugin converts the input internally into an OpenSSL BIGNUM and the loading 0x00 is lost.

Comment 2 Sumit Bose 2018-01-25 08:27:47 UTC
Please note, this might not be a bug in the plugin at all because I do not know if it is expected from the PKCS#11 point of view that there are leading zeros in the CKA_ID. If CKA_ID should not start with 0x00 there might be an issue in coolkey because if was reported that p11tool shows output like:

# p11tool --provider /usr/lib64/pkcs11/libcoolkeypk11.so --list-all-certs
Object 0:                                                  
        URL: pkcs11:model=;manufacturer=;serial=;token=....;id=%00%01;object=...;type=cert                                                                                                            
        Type: X.509 Certificate                            
        Label: ...                          
        ID: 00:01

Object 1:                                                  
        URL: pkcs11:model=;manufacturer=;serial=;token=....;id=%00%02;object=...;type=cert                                                                                                            
        Type: X.509 Certificate                            
        Label: ...                          
        ID: 00:02

Object 2:                                                  
        URL: pkcs11:model=;manufacturer=;serial=;token=....;id=%00%03;object=...;type=cert                                                                                                            
        Type: X.509 Certificate                            
        Label: ...                          
        ID: 00:03

(sorry, I do not have the output with OpenSC yet).

Jakub, does PKCS#11 that CKA_ID starts with 0x00?

Comment 3 Jakub Jelen 2018-01-25 08:48:24 UTC
The CKA_ID does not expose any limitation to the values. It is defined as a byte array [1], not a generic number and the leading zero is for some reason quite common in existing cards.

The PKCS#11 selects and compares the keys based on the bytes passed so if some of your tools (pkinit) converts the bytes to bignum, it is a bug there. This data should be treated as a byte array.

[1] http://docs.oasis-open.org/pkcs11/pkcs11-base/v2.40/os/pkcs11-base-v2.40-os.html#_Toc416959712

Comment 4 Sumit Bose 2018-01-25 09:16:25 UTC
Hi Jakub,

thank you for the swift reply and the reference to the specs.

Robbie, can you handle this or shall I have a look?

Comment 6 Alexander Bokovoy 2018-01-25 15:58:43 UTC
Looks like that the following code in pkinit_get_certs_pkcs11() in the pkinit plugin uses openssl's bignum purely to assist with hex to bin conversion:

    /* Convert the ascii cert_id string into a binary blob */
    if (idopts->cert_id_string != NULL) {
        BIGNUM *bn = NULL;
        BN_hex2bn(&bn, idopts->cert_id_string);
        if (bn == NULL)
            return ENOMEM;
        id_cryptoctx->cert_id_len = BN_num_bytes(bn);
        id_cryptoctx->cert_id = malloc((size_t) id_cryptoctx->cert_id_len);
        if (id_cryptoctx->cert_id == NULL) {
            BN_free(bn);
            return ENOMEM;
        }
        BN_bn2bin(bn, id_cryptoctx->cert_id);
        BN_free(bn);
    }

Looking into openssl, there is BN_bn2binpad() which optionally adds leading zeros when length of the binary blob is known.

Unfortunately, BN_hex2bn() assumes plain string of hex, e.g. 0001020304... This means %00%01%02%03%04... would not be properly converted by this code. So we need to either use a different method to convert to bignum or avoid using bignum at all.

Comment 9 Sumit Bose 2018-01-25 20:30:33 UTC
Created attachment 1386291 [details]
Patch to properly handle leading zero in the certid option

Comment 10 Sumit Bose 2018-01-25 20:31:23 UTC
Created attachment 1386292 [details]
Patch with some simple tests for hex_string_to_bin()

Comment 11 Sumit Bose 2018-01-25 20:34:43 UTC
I attached a patch to the ticket which replace BIGNUM based conversion with a byte-by-byte one. This version work well with some existing cards I have and as well with a newly created one with a certificate ID of 00:01.

I attached a test build with this patch to the linked case as well.

Comment 12 Sumit Bose 2018-01-25 20:35:31 UTC
(In reply to Sumit Bose from comment #11)
> I attached a patch to the ticket which replace BIGNUM based conversion with
> a byte-by-byte one. This version work well with some existing cards I have
> and as well with a newly created one with a certificate ID of 00:01.
> 
> I attached a test build with this patch to the linked case as well.

ah, I forgot, comments and suggestions are welcome.

Comment 13 Sumit Bose 2018-01-26 19:39:29 UTC
Upstream ticket: http://krbdev.mit.edu/rt/Ticket/Display.html?id=8636

Comment 35 aheverle 2018-05-29 15:07:12 UTC
Yes, this seemed to resolve the issue.

Comment 40 errata-xmlrpc 2018-10-30 08:08:00 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHSA-2018:3071


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