Migrate serialization format to standard PKCS#11 URIs compliant with RFC7512

[dwmw2]

This migrates the serialization format to conform to the PKCS#11 URI scheme as described in RFC7512.

The old form is still recognised for compatibility, but standard PKCS#11 URIs are now generated and accepted. Testing with OpenVPN now gives me the following output:

$ openvpn --show-pkcs11-ids /usr/lib64/pkcs11/gnome-keyring-pkcs11.so 

The following objects are available for use.
Each object shown below may be used as parameter to
--pkcs11-id option please remember to use single quote mark.

Certificate
       DN:             DC=com, DC=intel, DC=corp, DC=ger, OU=Workers, CN=Woodhouse, David, [email protected]
       Serial:         1EB2ECCF00030058F375
       Serialized id:  pkcs11:model=1.0;manufacturer=Gnome%20Keyring;serial=1%3aUSER%3aDEFAULT;token=Gnome2%20Key%20Storage;id=%59%ae%17%70%af%e8%af%9f%5b%94%fb%c6%89%f6%f1%4c%11%5c%36%0e

... and the PKCS#11 URI thus generated is also usable with the --pkcs11-id option.

[dwmw2]

Updated push: helps if I NUL-terminate the serialised strings.

[alonbl]

I am unsure what is the cutoff between these two patches, can you please explain?

[dwmw2]

I think this should address the feedback. I've made incremental commits for easy of review but obviously we can collapse this into the original commit(s) before merging if that's preferred.

I've made it use _pkcs11h_util_hexToBinary() as you suggest, for which I had to stop that function requiring that the string be NUL-terminated after the length it was asked to parse. Which was simple enough; there was only one other caller and I just made sure it checked for itself.

The loop parsing URI attributes no longer contains continue. Not quite sure what the problem is with that, but it's easy enough to remove it as you observed.

I've cleaned up the code parsing the legacy token/certificate IDs to be more symmetric with the URI parsing instead of duplicating the return paths.

I think you were asking why we keep pkcs11h_token_deserializeTokenId() if it's no longer used? It's an exported function. We no longer use it as a helper from pkcs11h_certificate_deserializeCertificateId() but we can't actually kill it.

I've made a table of the token ID fields which can now be used in three places — the parsing code, and twice during the serialization, which (following the lead of the existing code) was processing the fields twice: once for calculating the length of the string required, and again for actually generating the string. I've cleaned up some of the other code paths and duplication in the latter too.

I've prefixed static symbols with __ although I'm a little confused about that request. These really are static symbols within the C unit, not functions which are visible to other C units within the library, but intended to be private to the library. My understanding is that even if you're building without a decent linker that has visibility controls and symbols maps, a symbol which is declared as 'static' within a C file is really not going to be seen elsewhere. So you can call it absolutely anything you like... with the caveat that you should not use symbols starting with two underscores, because those are reserved for the system. But OK :)

I've defined URI_SCHEME and used it instead of the bare "pkcs11:" where appropriate.

[dwmw2]

Now that the PKCS#11 URI format has been published as RFC7512 it would be very good to get this merged... should I collapse the above fixes into the original commits and resubmit? After this length of time there doesn't seem much point in keeping them incremental any more...

[nmav]

The pkcs11: URI format is a standard (RFC7512) since this April. It would be nice if pkcs11-helper would support that format.

[dwmw2]

Updated patch with minor fix from https://bugzilla.redhat.com/show_bug.cgi?id=1264645

[dwmw2]

Ping. It would be really good to get this merged...

[alonbl]

Hi,
As I wrote, your implementation is much more complex than it should, it
won't get merged.
I need to rework this into something simpler, but this is extremely low
priority for me.
Thanks,
Alon

On 23 August 2016 at 16:47, dwmw2 [email protected] wrote:

Ping. It would be really good to get this merged...

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#4 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABNIrSZPebF7EBGQmup94jOCvJftFw2Jks5qivncgaJpZM4DGws9
.

[dwmw2]

It could be massively simplified if it were to just use libp11-kit for the URI parsing. Other than that, I'm not sure how it could be simpler.

[emmanuel-deloget]

Hi,

RFC7512 is now two years old. Could this patch (or any equivalent patch) be merged into pkcs11-helper ?

Best regards,

-- Emmanuel Deloget

[dwmw2]

Updated with bug fix from https://bugzilla.redhat.com/show_bug.cgi?id=1516474

[celesteking]

Can we get this crap merged finally so openvpn actualy works with smartcards properly via NetworkManager? Thanks.

[boomer41]

Any updates on this one?

[alonbl]

As I wrote, this patch is way to complex to achieve the desired task, and is partial as it supports only full pkcs11 url which as far as I understand is the opposite of reaching portability between applications that support the notation. I also got reports that the windows version of openvpn in which this patch is applied is failing to parse urls.

I am unsure how it is that important to network manager and am unsure why the openvpn management interface cannot be used to detect the available ids and apply id as opaque. And I keep reminding people that at daemon context openvpn should not have access to user's devices anyway, and the proper solution is to remove the entire pkcs11 support from the daemon and make sure the management interface is capable of delegating all private key operation to the user interface program, however, it seems like the openvpn project is on freeze.

[dwmw2]

Updated with fix for https://bugzilla.redhat.com/show_bug.cgi?id=1825496

As I wrote, this patch is way to complex to achieve the desired task,

I'm not quite sure how it could be simpler. I suppose it could take a library like libp11-kit as a dependency instead of doing the serialisation/deserialisation for itself?

and is partial as it supports only full pkcs11 url which as far as I understand is the opposite of reaching portability between applications that support the notation.

That doesn't seem to make much sense to me; can you explain what you mean? It will output the full URI when serialising, when listing all available objects. But obviously accepts only a partial URI (i.e. not redundantly specifying all attributes) when identifying an object. In that respect, it behaves like everything else does, and should.

I also got reports that the windows version of openvpn in which this patch is applied is failing to parse urls.

A specific bug report would be very much appreciated, if there is misbehaviour.

I am unsure how it is that important to network manager and am unsure why the openvpn management interface cannot be used to detect the available ids and apply id as opaque.

There is work on displaying the available objects through a UI (like in Seahorse) and that widget will emit the RFC7512 ID identifying the chosen object. For that, we need to be using the standard.

And I keep reminding people that at daemon context openvpn should not have access to user's devices anyway, and the proper solution is to remove the entire pkcs11 support from the daemon and make sure the management interface is capable of delegating all private key operation to the user interface program, however, it seems like the openvpn project is on freeze.

Yeah, it should do proxying from something in the user's session. But that's slightly orthogonal to the fact that we want to use a standard form to identify PKCS#11 objects.

[becm]

OpenVPN uses an out-of-date patch set which is bound to fail when maxing out certain token attribute entries, 16-byte serials being the most common case.

This was my first problem as well, then something else cropped up. Sadly neither seems to have made it in time for OpenVPN 2.4.9 release.

[williamcroberts]

Literally without this patch, things are broken. Fedora patches their stuff with this. I just bumped into this, can we somehow get this merged or get a clear path forward?

[becm]

The menioned problems with OpenVPN should be fixed by updated patch used in current builds for Windows.
Update of included pkcs11-helper version also addresses EC support detection problem and adds support for PSS padding.

[saper]

Seems like this change might break access to some HSMs

https://bugzilla.redhat.com/show_bug.cgi?id=2298882 alonbl/gnupg-pkcs11-scd#63

[saper]

your implementation is much more complex than it should

#69 is my attempt to see if this can be done simpler. For now, this branch can only use URL encoding instead of \x20.

[dwmw2]

Updated with fix for https://bugzilla.redhat.com/show_bug.cgi?id=2298882 (untested). Discussion in alonbl/gnupg-pkcs11-scd#63

[inorton-entrust]

Hi everyone. I've just spent a while tripping over this issue until just now when it occurred to me that Redhat might have patched their pkcs11-helper library build.

My problem is that I already have some data (including and others also much like gpg private key shadow files) that refer to the original serialized token info returned by pkcs11h_token_serializeTokenId(),

eg, CENTOS7, Debian, Ubuntu, compiled-from-source etc:

nCipher\x20Corp\x2E\x20Ltd//a4e2d7d6672ba9f2/my_cardset

Yet when I run my same binary on a RHEL8+ system due to the RFC7512 patch this comes back instead as:

pkcs11:model=;token=my_softcard;manufacturer=nCipher%20Corp.%20Ltd;serial=a502d2d6675ba9f2

I can 100% see the logic of the newer form, but It would really be nice if this was a configurable opt in/out, perhaps simply an environment variable to switch between the two schemes.

P.S. I'm one of the developers who maintains the nCipher PKCS#11 stack.

[dwmw2]

It continues to accept key identifiers in the old non-standard format; it now accepts both forms. It just generates the RFC7512 standard form rather than the legacy form.

There waa simple bug in handling the RFC7512 form of identifier (as an input) when the model= field is empty, because of the way the underlying token selection works in pkcs11-helper. Does the fix work for you?

[inorton-entrust]

It continues to accept key identifiers in the old non-standard format; it now accepts both forms. It just generates the RFC7512 standard form rather than the legacy form.

There waa simple bug in handling the RFC7512 form of identifier (as an input) when the model= field is empty, because of the way the underlying token selection works in pkcs11-helper. Does the fix work for you?

Thanks, I will build this PR branch and give it a try!

[inorton-entrust]

It continues to accept key identifiers in the old non-standard format; it now accepts both forms. It just generates the RFC7512 standard form rather than the legacy form.

There waa simple bug in handling the RFC7512 form of identifier (as an input) when the model= field is empty, because of the way the underlying token selection works in pkcs11-helper. Does the fix work for you?

Hi David, I've been able to build this branch and test it. Things fail early because of how the "serial number" of the smartcard is reported.

This is computed from the sha1 hash of the "serialized" token info that this branch adds. So now gpg thinks it has a different card in the reader.

I'll experiment and see what I can come up with.

[dwmw2]

For a moment I thought you meant the serial= part of the URI, from the serialNumber field of CK_TOKEN_INFO. But that isn't it, is it? You mean a serial number generated internally by GPG to match the token (or the key)?

[inorton-entrust]

For a moment I thought you meant the serial= part of the URI, from the serialNumber field of CK_TOKEN_INFO. But that isn't it, is it? You mean a serial number generated internally by GPG to match the token (or the key)?

The gnupg-pkcs11-scd app needs to return a value for it's SERIALNO command to the gpg agent. gnupg-pkcs11-scd gets this value by calling pkcs11h_token_serializeTokenId() and then feeds that string into sha1, it uses the first 8 hex chars of this sha1 hash to form the "card serial" . This substring is embedded into the middle of the SERIALNO output.

With the unmodified pkcs11-helper you get:

$ echo SERIALNO | gnupg-pkcs11-scd --server
S SERIALNO D27600012401115031312CF4EEE61111 0

With the uri-scheme output of pkcs11h_token_serializeTokenId() the middile 2CF4EEE6 substring changes when given the same token.