Skip to content
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.

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

pkcs11_get_ec() assumes DER encodig of pubkeys - fix proposed #79

Closed
DDvO opened this issue Apr 18, 2016 · 11 comments
Closed

pkcs11_get_ec() assumes DER encodig of pubkeys - fix proposed #79

DDvO opened this issue Apr 18, 2016 · 11 comments

Comments

@DDvO
Copy link

DDvO commented Apr 18, 2016

Apparently due to some misconception, pkcs11_get_ec() assumes that the PCKS#11 device returns EC_POINT data of public keys in ASN.1 encoding. At least my CardOS v5.0 test card does not work this way, and I suspect that also other cards return plain binary data instead.

In my fix given both as attachment: p11_ec_point_no_asn1.patch.txt and in my pull request #80
I improve error handling and disable that part of the code in pkcs11_get_ec() used to convert from ASN.1 (by using #ifdef PKCS11_EC_POINT_ASN1).

@mtrojnar
Copy link
Member

Page 217 of the PKCS#11 v2.20 spec (28 June 2004), section 12.3.3, table 55 "Eliptic Curve Public Key Object Attributes":

CKA_EC_POINT byte array DER-encoding of ANSI X9.62 ECPoint value Q.

Reportedly, some implementation do not comply with the standard.
An interesting discussion on this topic: https://bugzilla.mozilla.org/show_bug.cgi?id=480280

@dengert
Copy link
Member

dengert commented Apr 18, 2016

What PKCS#11 module are you using to access your CardOS v5.0 test card?

The problem may be in the module, not in libp11.

@mouse07410
Copy link
Contributor

I think at least some cards (e.g., Yubikey) do return EC PK in DER.

@dengert
Copy link
Member

dengert commented Apr 19, 2016

The issue is not what the card returns. The issue is what the PKCS#11 module returns, and does it meet teh PKCS#11 specs..

@DDvO
Copy link
Author

DDvO commented Apr 19, 2016

As just mentioned in the other thread #78, I'm using some (official) Card OS 5.3 middleware.

It came to my surprise that PKCS#11 in this case requires an ASN.1 octet (DER) encoding, while for RSA key material this is not the case. I confirmed this from ftp://ftp.rsasecurity.com/pub/pkcs/pkcs-11/v2-30/pkcs-11v2-30m1-d7.pdf. Judging also from the discussion on Bugzilla there are several implementations that are non-compliant in the same way as my card.

Thus, my proposal now is to handle both cases, like it as been decided for Mozilla/NSS.
The following code first tries to parse the point data in the official (DER) encoding, and if this fails, it then tries without that encoding, in order to support also non-compliant PKCS#11 implementations:

    if (!key_getattr_alloc(pubkey, CKA_EC_POINT, &point, &point_len)) {
        const unsigned char *a;
        ASN1_OCTET_STRING *os;

        /* PKCS#11 should return DER encoding (ASN1_OCTET_STRING) */
        a = point;
        os = d2i_ASN1_OCTET_STRING(NULL, &a, (long)point_len);
        a = os->data;
        if (!os || !o2i_ECPublicKey(&ec, &a, os->length)) {
            /* otherwise, for the benefit of non-compliant modules/devices, try parsing plain content */
            a = point;
            if (!o2i_ECPublicKey(&ec, &a, point_len)) {
                EC_KEY_free(ec);
                ec = NULL;
            }
        }
        if (os) {
            ASN1_STRING_free(os);
        }
        OPENSSL_free(point);
    } else {
        EC_KEY_free(ec);
        ec = NULL;
    }

I successfully tested this patch on a libEST client enrolling a EC cert for a private key on the card.
BTW, the weird assignments to a are crucial; passing instead a pointer the value of pointor os->data directly to o2i_ECPublicKey() does not work.
Note that this code along the way adds some error handling missing in the original one.
Like for the other open issue #78, I'd be happy to provide this as a proper pull request.

I mentioned this issue to the CardOS team as well.

BTW, also XCA, a nice graphical front-end to OpenSSL key&cert management functionality, simply assumes that there is no DER encoding.

So this seems to be a pretty common mistake, and IMHO it would be of much help if libp11 handles it gracefully.

@DDvO DDvO changed the title pkcs11_get_ec() wrongly assumes ASN.1 encodig - fix proposed pkcs11_get_ec() assumes DER encodig of pubkeys - fix proposed Apr 19, 2016
@mouse07410
Copy link
Contributor

I successfully used libp11 as is with EC-based cards (Yubikey 4 configured with keys on P-384 curve), performing operations via OpenSC and OpenSSL-1.0.2g+ and 1.1.0-pre*.

Thus I question whether this fix should apply to libp11 or to the articular middleware in question. Also, I wonder if OpenSC would work for your card (as middleware), alleviating the need to use the "(official) Card OS 5.3 middleware".

@DDvO
Copy link
Author

DDvO commented Apr 20, 2016

Good idea to try opensc-pkcs11.dll, but it does not work with the CardOS card.
It finds the card in the right slot, but does not give reasonable token info, it cannot find any objects on the card, and login (with the correct PIN) results in CKR_USER_PIN_NOT_INITIALIZED.

@mouse07410
Copy link
Contributor

Do you perhaps need a working tokend? Can it be an issue with the "stock" (aka OpenSC/OpenSC.tokend) not dealing properly with your card? Do you see anything usable in opensc-debug.log file (assuming you enabled logging)?

@dengert
Copy link
Member

dengert commented Apr 20, 2016

In all of your reports, what woulf be very helpful is a OpenSC PKCS11_SPY trace (works with other PKCS#11 modules), or if using OpenSC the opensc-debug.log.
It could be OpenSC does not support your CardOS 5.0 system. It could also be the PKCS#11 module you are using is not returning the point in the expected format.

For b oth types of traces see:
https://github.com/OpenSC/OpenSC/wiki/Using-OpenSC

@mtrojnar
Copy link
Member

Please open a CardOS 5.0 issue in https://github.com/OpenSC/OpenSC/issues

The proposed solution, i.e. fallback to "plain" encoding, seems reasonable. I'm going to implement it.

Currently, pkcs11_get_ec() does best-effort retrieval and it does not fail if it was unable to retrieve some data. Maybe we should enforce a more strict error checking. I appreciate your comments on this topic.

@DDvO
Copy link
Author

DDvO commented Apr 21, 2016

@mtrojnar, thanks for swiftly taking over the workaround.

@dengert and @mouse07410, thanks for the hints regarding tracing and using the cardos driver.

I knew of PKCS11_SPY and I'm already heavily using it, but did not now how to increase the debug output level. Setting OPENSC_DEBUG=9 and using the OpenSC module prints lots of output, while not writing a opensc-debug.log file (unfortunately, I'm not on real Linux, but Cygwin) and I can't find anything like 'tokend' in the output or anywhere else in the OpenSC files.

Looking at the debug trace, I can confirm that the cardos ("Siemens CardOS") driver is chosen.
Indeed, OpenSC works for my CardOS 4.2 card (which does not have EC support, though), but not for my CardOS 5.0 card.

As recommended, I'll open issues for OpenSC regarding that cardos driver and CardOS 5.0, giving the respective traces there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants