Bug 1538491

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?

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

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?

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.

Created attachment 1386291 [details]
Patch to properly handle leading zero in the certid option

Created attachment 1386292 [details]
Patch with some simple tests for hex_string_to_bin()

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.

(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.

Upstream ticket: http://krbdev.mit.edu/rt/Ticket/Display.html?id=8636

Yes, this seemed to resolve the issue.

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