Opened 9 years ago

Closed 15 months ago

#491 closed Bug / Defect (fixed)

PKCS#11 ID format is non-standard

Reported by: dwmw2 Owned by: Samuli Seppänen
Priority: major Milestone: release 2.4.10
Component: Packaging Version: OpenVPN git master branch (Community Ed)
Severity: Not set (select this one, unless your'e a OpenVPN developer) Keywords: windows
Cc: tct

Description

The string format output by the --show-pkcs11-ids option and accepted by --pkcs11-id is not standard. Please consider migrating to the format described in https://tools.ietf.org/html/draft-pechanec-pkcs11uri-16

For backward compatibility of course the old format could still be accepted, but all well-behaved applications should these days be using p11-kit for discovering modules (cf. #490) and accepting PKCS#11 URIs in the standard form.

Change History (23)

comment:1 Changed 9 years ago by Gert Döring

Type: Bug / DefectFeature Wish

As said in #490 - patches welcome :-)

comment:2 Changed 9 years ago by dwmw2

It turns out this is entirely outside OpenVPN. The serialization/deserialization is done in pkcs11-helper. And is fixed to use the standard PKCS#11 URI format by my pull request at https://github.com/OpenSC/pkcs11-helper/pull/4

comment:3 Changed 9 years ago by Gert Döring

Resolution: notabug
Status: newclosed

So if I understand your comment right, we can close this one? (I'll just go ahead and do so, and if I misunderstood, we'll just reopen)

comment:4 Changed 9 years ago by dwmw2

Right. But with the caveat that any binary builds you offer should ideally be built with a version of pkcs11-helper including the above patches. The PKCS#11 URI format is published as RFC7512 now, but the above pull request for pkcs11-helper still hasn't been merged upstream unfortunately.

comment:5 Changed 9 years ago by Gert Döring

Resolution: notabug
Status: closedreopened

comment:6 Changed 9 years ago by Gert Döring

Owner: set to Samuli Seppänen
Status: reopenedassigned

Assigning to Samuli, as he's the only one building installers...

comment:7 Changed 9 years ago by Samuli Seppänen

Keywords: windows added
Milestone: release 2.4

comment:8 Changed 8 years ago by Samuli Seppänen

The pull request to pkcs11-helper was created two years ago and has not been merged yet. Upgrading our dependencies (including pkcs11-helper) is a standard practce, so if/when the PR is merged, this issue will get fixed.

I think we can safely close this bug report again. Thoughts?

comment:9 Changed 8 years ago by dwmw2

Given that the pull request has been open for two years without progress, perhaps the better option is for OpenVPN to migrate away from pkcs11-helper to something less esoteric which is more proactively kept up to date.

Most people seem to use the OpenSSL PKCS#11 ENGINE, although that doesn't support the --show-pkcs11-ids option. However, if you're using standard RFC7512 IDs instead of your own non-standard nonsense, you shouldn't need your own way of listing them because the standard tools like p11tool --list-certs will work.

The alternative way, preserving the --show-pkcs11-ids option, would be to use libp11 directly as I do in OpenConnect. There are moves afoot to add native PKCS#11 support to OpenSSL 1.2 as a "first class citizen", which would be based on the libp11 API. Migration to OpenSSL 1.2 (when it eventually happens) would then be fairly simple.

The only reason I haven't already submitted patches to move to libp11 is because it only supports OpenSSL, and I don't have a solution for PolarSSL/mbedTLS. Would it be acceptable to keep pkcs11-helper support only for the mbedTLS case, or remove it completely and thus drop PKCS#11 support for mbedTLS? Neither of those options really fill me with joy...

comment:10 Changed 8 years ago by Samuli Seppänen

Are there any plans in libp11 to support non-OpenSSL libraries? I don't think we should move into confusing hybrid setup where RFC7512 and/or behavior of OpenVPN would be different depending on which SSL library is in use.

I'm not sure why your pkcs11-helper PR got stuck, but perhaps pinging alonbl would help?

comment:11 Changed 8 years ago by Samuli Seppänen

One more thing: what practical problems the lack of RFC7512 format cause? As in: what is the impact on our users?

comment:12 Changed 8 years ago by dwmw2

My plan is to obsolete libp11 by integrating its functionality as a first-class citizen within OpenSSL. Not to make it support non-OpenSSL libraries. I certainly agree that we should not have different behaviour depending on which SSL library is in use. Except that some libraries don't support PKCS#11 at all, of course. Would anyone actually scream if we added mbedTLS to that set?

Practically... the point in RFC7512 is to let users use a standard identifier string to reference the certificate on their crypto token (or indeed, in their GNOME keyring, or their NSS database).

In Fedora, for example, packaging guidelines state that if any application takes a filename for a cert on its command line or in a config file, it SHOULD also accept a RFC7512 PKCS#11 URI in place of the filename. (Fedora has resolved this for OpenVPN by patching pkcs11-helper with the patch from the above-referenced pull request, and in the medium term hopefully we'll be able to kill off pkcs11-helper completely as there are very few packages which use it.)

Before this, every application gave the user different hoops they had to jump through, and different object identifier formats, just to use the same certificate from their token.

comment:13 in reply to:  12 Changed 8 years ago by Samuli Seppänen

Replying to dwmw2:

My plan is to obsolete libp11 by integrating its functionality as a first-class citizen within OpenSSL. Not to make it support non-OpenSSL libraries. I certainly agree that we should not have different behaviour depending on which SSL library is in use. Except that some libraries don't support PKCS#11 at all, of course. Would anyone actually scream if we added mbedTLS to that set?

No idea. We would have to ask about this on openvpn-users/openvpn-devel mailing list.

In Fedora, for example, packaging guidelines state that if any application takes a filename for a cert on its command line or in a config file, it SHOULD also accept a RFC7512 PKCS#11 URI in place of the filename. (Fedora has resolved this for OpenVPN by patching pkcs11-helper with the patch from the above-referenced pull request, and in the medium term hopefully we'll be able to kill off pkcs11-helper completely as there are very few packages which use it.)

As a short-term solution we could move to using the Fedora-patched pkcs11-helper.
Do they have a fork of pkcs11-helper somewhere, or do they just bundle their extra patches with the SRPMs?

comment:14 Changed 8 years ago by Samuli Seppänen

Answering my own question: the extra Fedora patches are in this Git repository. I will send email to openvpn-devel about this.

comment:15 Changed 8 years ago by Samuli Seppänen

Mail sent, but not yet visible on mail-archive.com.

comment:16 Changed 8 years ago by Steffan Karger

I'd love OpenVPN to support the RFC7512 format, and deprecate the old (pkcs11-helper) format. (Even more, I don't like pkcs11-helper much at all, but I haven't yet found a better alternative.)

A few things though:

  1. Scream! Me, and many of my customers, depend on pkcs11 support in mbed TLS builds. Since I'm basically the one maintaining the crypto code, I would NAK a patch that breaks pkcs11 support for mbed TLS.
  2. We shall not break user configs. We need to be backwards compatible, at least long enough to warn users when they are using the 'old format', and give them time to migrate.
  3. We've been trying to make the mbed TLS and OpenSSL builds as compatible as possible. It would be a shame to add new incompatibilities. The config file is (obviously) user-visible, so we should really aim to add RFC7512 support to both, or none.

comment:17 Changed 7 years ago by Steffan Karger

Milestone: release 2.4release 2.5

This won't make 2.4, moving to 2.5.

comment:18 in reply to:  17 Changed 4 years ago by becm

The discussed patch (generated from outdated pull request) is actually used in Windows installers since OpenVPN 2.4.5 in an outdated/broken form with known encoding and parsing bugs.

Fixed version (updated RFC7512 patch) used in 2.5 releases (after beta1) should resolve issues.

Beware: 2nd bug is a serialization error which may result in wrong output of CertID.
Rerun openvpn.exe --show-pkcs11-ids <pkcs11-dll> after update to show correct RFC7512 URI.

comment:19 Changed 4 years ago by Gert Döring

Component: Generic / unclassifiedPackaging
Milestone: release 2.5release 2.4.10
Type: Feature WishBug / Defect

Since we are now shipping the new version/proper patch in 2.5, I suggest we build the next 2.4 NSIS release with the same versions of pkcs11-helper plus patch. (At least if we do not get lots of "BORKEN!" feedback)

Thus, changing Milestone to 2.4.10

comment:20 Changed 4 years ago by tct

Cc: tct added

comment:21 Changed 3 years ago by Gert Döring

As far as I understand, we did. That is, build 2.4.10 with an updated pkcs11-helper plus patch.

So, can we close this ticket now?

comment:22 Changed 15 months ago by becm

No documented complaints since v2.4.10 release (apart from open feedback request in #1075).
The latest version of RFC7512 for pkcs11-helper seems to have ironed out known formatting issues.

comment:23 Changed 15 months ago by Gert Döring

Resolution: fixed
Status: assignedclosed

hooray :-)

Note: See TracTickets for help on using tickets.