Accept pkcs11: URIs for EAP certificates and private keys

[nickrbogdanov]

On systems where the libp11 "pkcs11" OpenSSL ENGINE plugin is installed, allow the user to request keys and certs from an HSM or smartcard instead of storing them in the clear on disk.

This is useful because revoking a leaked EAP server certificate can be highly disruptive, especially if there are many deployed devices and IoT gadgets that connect to the network exclusively via 802.1x. Storing the private key on a smartcard ensures that it cannot be extracted if the RADIUS server is compromised.

Performance-wise, I see penalties ranging from ~90ms per TLS handshake (ECC256) to 160ms (RSA2048) on a YubiKey 4. I would expect that higher-grade HSM hardware would be much faster, but I don't have access to it for testing.

To test this on Ubuntu 20.04 with a YubiKey, I set up a local installation with an on-disk private key, tested it with radiusd -X, and then moved it to the smartcard:

sudo apt install -y opensc opensc-pkcs11 libengine-pkcs11-openssl yubico-piv-tool
yubico-piv-tool --pin=123456 --action=import-key --slot=9a < radius.key
yubico-piv-tool --pin=123456 --action=import-certificate --slot=9a < radius.pem

(I had to use yubico-piv-tool instead of the standard pkcs11-tool because C_CreateObject is apparently unsupported.)

Then I edited the mods-enabled/eap file to add a PKCS#11 URI with the pin-value property set:

    #private_key_file = ${certdir}/rsa/radius.key
    private_key_file = "pkcs11:model=PKCS%2315%20emulated;id=%01;pin-value=123456;type=private"

I then restarted radiusd -X and used a test client / eapol_test to verify that the server still worked correctly.

I don't really expect most users to set things up so that interactive PIN entry is required, but I did test that case. When running radiusd -X from the command line, OpenSSL will prompt for a PIN; without -X the daemon will log an appropriate error and exit.

The one thing I wasn't able to figure out is how to import the entire certificate chain into the YubiKey's X.509 certificate slot (if such a thing is even possible). So I suspect that loading certificate_file from PKCS#11 is mainly useful if the server certificate is self-signed.

Also, I didn't test this against old versions of OpenSSL, and I didn't test the case where OpenSSL is missing ENGINE support (which I suspect will already break the existing fr_openssl_init() code).

[alandekok]

This definitely seems interesting. It's a tiny patch, which helps rather a lot in getting it merged.

[arr2036]

Yeah it's interesting, but from a usability perspective interacting with the PKCS11 engine by overloading the certificate configuration items isn't great. Is there any definition of the PKCS11 configuration items? Could we add a better defined interface?

[alandekok]

The main issue with using different configuration items is that much code assumes that certificate_file, etc. are not empty. We'd have to double-check that.

If we were to follow the more abstract representation, we might change the configuration options to more URL-style references?

ca = file:///blah

but that's a little fugly. As nothing else in the configuration uses file:/// refs to refer to files.

Just checking the rest of the configuration, pretty much only the TLS configuration uses _file to refer to file things. The detail file reader doesn't, for example. And neither does anything else using files.

So I'd be happy to remove the _file suffix from these configuration items. Then maybe add a separate configuration item which says where the certificates come from:

auto_chain = no
...
# files or pkcs11
storage = files

that seems a little clearer

[arr2036]

Probably wants more granularity from that. If you look at the patch it only adds the ability to load the server cert and the private key using the PKCS engine, intermediary CAs still come from files.

certificate {
    storage | format | source = (pem-file|der-file|asn1-file|pkcs11)
    file = "path"
    pkcs11 {
        <param>
    }
}

ca {
    storage | format | source = (pem-file|der-file|asn1-file|pkcs11)
    file = "path"
    pkcs11 {
        <param>
    }
}

private_key {
    storage | format | source = (pem-file|der-file|asn1-file|pkcs11)
    file = "path"
    password = "foo"
    pkcs11 {
        <param>
    }
}

client_ca {
    storage | format | source = (weird-hashing-dir-thing|pem-file|der-file|asn1-file|pkcs11)
    file = "path"
    pkcs11 {
        <param>
    }
}

Server certs/validation options and client certs/validation options probably want to go in our_cert their_cert sections or something equivalent.

[alandekok]

Yeah... it's probably useful to support multiple formats. I'll have to check if OpenSSL will automatically read the different formats.

The configuration already supports format = PEM|DER|ASN1, tho that isn't documented anywhere. adding pkcs11 is likely the best choice here.

[arr2036]

PKCS11 URI scheme https://tools.ietf.org/html/rfc7512

[nickrbogdanov]

Yeah it's interesting, but from a usability perspective interacting with the PKCS11 engine by overloading the certificate configuration items isn't great.

I modeled this on the client side (wpa_supplicant) configuration, where you can just replace the filenames with PKCS#11 URIs: https://superuser.com/questions/1510368/configuring-wpa-supplicant-to-use-tpm2-pkcs11-tools

certtool from GnuTLS works in a similar way, as does Apache httpd. Red Hat patched OpenSSH so that ssh -i <foo> can take either a private key pathname or PKCS#11 URI.

The OpenSSL CLI programs want you to specify -engine pkcs11 -keyform engine. The advantage of the extra verbosity is to let users specify which ENGINE to use (not just PKCS#11), but that's a little more cumbersome than just pasting a PKCS#11 URI into the private_key_file field.

[alandekok]

wpa_supplicant has:

    key_id="4"
    cert_id="4"
    ca_cert_id="1"

    # set the PIN code; leave this out to configure the PIN to be requested
    # interactively when needed (e.g., via wpa_gui or wpa_cli)
    pin="123456"

so we could do something similar here.

Putting all of the text into a large blob just seems bad: "pkcs11:model=PKCS%2315%20emulated;id=%01;pin-value=123456;type=private" that's opaque and difficult to get right.

[arr2036]

yeah there's no way I would push creating that URI correctly onto the user. Getting them to correctly escape the different values, and correctly infer the underlying data type is just a non-starter. I mean id=%01 ew... and we know whether we're requesting the certificate or private key based on the configuration item, why make the user repeat that in the URI.

[nickrbogdanov]

Putting all of the text into a large blob just seems bad: "pkcs11:model=PKCS%2315%20emulated;id=%01;pin-value=123456;type=private" that's opaque and difficult to get right.

Except for the pin-value part, you should be able to paste the URI directly from the p11tool --list-all output. Users shouldn't need to hand-edit these URIs, although they can if they want to.

For specifying the PIN, maybe overload private_key_password? I can't think of a scenario where you'd want a password-protected private key on disk but certs/pubkeys stored on a smartcard.

Edit: Here is a useful guide on finding PKCS#11 URIs. Also note that the ID field might not be as simple as id=%01; for instance, when I look at my SoftHSM keystore it generates long hex strings: pkcs11:model=SoftHSM%20v2;manufacturer=SoftHSM%20project;serial=1a9508f32aa1bb7b;token=first;id=%01%41%1f%fe%b0%44%95%2c%d0%eb%62%8a%1e%46%78%72%80%7d%18%18;object=myecc;type=private

[nickrbogdanov]

Some other examples, from playing with SoftHSM:

p11tool --login --set-pin=1111 --generate-ecc --label=key03 'pkcs11:token=first' autogenerates an id= and sets object= to the --label value:

pkcs11:model=SoftHSM%20v2;manufacturer=SoftHSM%20project;serial=1a9508f32aa1bb7b;token=first;id=%ca%2c%bc%8a%27%10%15%16%36%eb%84%53%c7%48%dc%cf%71%76%a1%a6;object=key03;type=private

p11tool --login --set-pin=1111 --generate-ecc --id=%04 --label=key04 'pkcs11:token=first' sets both id= and object=:

pkcs11:model=SoftHSM%20v2;manufacturer=SoftHSM%20project;serial=1a9508f32aa1bb7b;token=first;id=%04;object=key04;type=private

ID can be an arbitrary hex string, not just a single byte. p11tool --login --set-pin=1111 --generate-ecc --id=123456 --label=somelabel 'pkcs11:token=first' yields:

pkcs11:model=SoftHSM%20v2;manufacturer=SoftHSM%20project;serial=1a9508f32aa1bb7b;token=first;id=%12%34%56;object=somelabel;type=private

If we really want maximum flexibility, we could let the user pick a different OpenSSL ENGINE from pkcs11. For instance, the tpm2tss engine doesn't use pkcs11: URIs and wouldn't be usable with the current patch set.

I still think there is value, however, in maintaining a special case that lets a user just paste a standard pkcs11: URI into the private_key field with no further configuration, since that is such a common use case that works with a wide variety of cryptographic hardware.

[arr2036]

@nickrbogdanov that's definitely a more compelling argument to accept pkcs11 URIs.

certificate {
    format = (pem|der|asn1|pkcs11|pkcs11-uri)
   
    file = "path"
   
    pkcs11 {
        <param0> = <value0>
        <param1> = <value1>
        <paramN> = <valueN>
    }
   
    pkcs11_uri = "uri"
}

The config pairs you're overloading should have had the FR_TYPE_FILE_EXISTS flag set. It's now set and your original patch will no longer work. This is one of the reasons I don't like overloading the config pairs, it stops us using the generic validation functions that are part of the CONF_PARSER framework.

We can add logic to the CONF_PARSER entries which change which subset of configuration items are parsed depending on the value of a config pair. So for format=pem|der|asn1 it'll look for a file config pair, for format=pkcs11 it'll look for a pkcs11 config section with parameters, for format=pkcs11-uri it'll look for a pcks11_uri config pair. This scheme would allow us to expand the formats to include other engines with other configuration formats.

I'll take a proper look at implementing this tomorrow, it shouldn't be difficult.

[nickrbogdanov]

The overloading was inspired by this functionality from wpa_supplicant:

	/*
	 * If the engine isn't explicitly configured, and any of the
	 * cert/key fields are actually PKCS#11 URIs, then automatically
	 * use the PKCS#11 ENGINE.
	 */
	if (!engine_id || os_strcmp(engine_id, "pkcs11") == 0)
		can_pkcs11 = 1;

	if (!key_id && params->private_key && can_pkcs11 &&
	    os_strncmp(params->private_key, "pkcs11:", 7) == 0) {
		can_pkcs11 = 2;
		key_id = params->private_key;
	}

	if (!cert_id && params->client_cert && can_pkcs11 &&
	    os_strncmp(params->client_cert, "pkcs11:", 7) == 0) {
		can_pkcs11 = 2;
		cert_id = params->client_cert;
	}

	if (!ca_cert_id && params->ca_cert && can_pkcs11 &&
	    os_strncmp(params->ca_cert, "pkcs11:", 7) == 0) {
		can_pkcs11 = 2;
		ca_cert_id = params->ca_cert;
	}

	/* If we need to automatically enable the PKCS#11 ENGINE, do so. */
	if (can_pkcs11 == 2 && !engine_id)
		engine_id = "pkcs11";

They also have similar logic that autodetects TPM2 keys and sets the engine_id accordingly.

If we drop the overloading, I would suggest adding generic ENGINE support in lieu of a special case for pkcs11. For a file:

certificate {
    format = (pem|der|asn1)
    file = "path"
}

private_key {
    format = pkcs8
    file = "path"
    password = "password"
}

For ENGINE:

certificate {
    format = engine
    engine = "engine_name"
    id = "engine_specific_id"
    pin = "optional_pin"
}

private_key {
    format = engine
    engine = "engine_name"
    id = "engine_specific_id"
    pin = "optional_pin"
}

engine_name could be any ENGINE listed in the OpenSSL system configuration, including pkcs11, tpm2tss, and many others. engine_specific_id could be a URI in the case of pkcs11 but it could be some other string for other engine types. Sample configurations for pkcs11 (both of which resolved to the same key when I tested it here):

private_key {
    format = engine
    engine = "pkcs11"
    id = "1"
    pin = "123456"
}

private_key {
    format = engine
    engine = "pkcs11"
    id = "pkcs11:id=%01"
    pin = "123456"
}

If optional_pin is specified, FreeRADIUS would call ENGINE_ctrl_cmd(engine, "PIN", strlen(pin_code), pin_code, NULL, 0). This way, id can be set to the exact URI that the user copied from p11tool without either the user or FreeRADIUS having to modify it to add a pin-value field. The way I tested this locally, using my first patch as a baseline, was:

diff --git a/src/lib/tls/ctx.c b/src/lib/tls/ctx.c
index 3ce9cb20f8..c48e7e1146 100644
--- a/src/lib/tls/ctx.c
+++ b/src/lib/tls/ctx.c
@@ -188,7 +188,12 @@ static int tls_ctx_load_cert_chain(SSL_CTX *ctx, fr_tls_chain_conf_t const *chai
        }
 
        if (!strncmp("pkcs11:", chain->private_key_file, 7)) {
-               EVP_PKEY *pkey = ENGINE_load_private_key(pkcs11_engine, chain->private_key_file, NULL, NULL);
+               if (!ENGINE_ctrl_cmd(pkcs11_engine, "PIN", 6, "123456", NULL, 0)) {
+                       fr_tls_log_error(NULL, "Failed to set PIN");
+                       return -1;
+               }
+               //EVP_PKEY *pkey = ENGINE_load_private_key(pkcs11_engine, chain->private_key_file, NULL, NULL);
+               EVP_PKEY *pkey = ENGINE_load_private_key(pkcs11_engine, "1", NULL, NULL);
                if (!pkey) {
                        fr_tls_log_error(NULL, "Failed to load private key \"%s\"", chain->private_key_file);
                        return -1;

[arr2036]

Still working on this. I added a lot of the infrastructure needed for dynamically loaded engines in src/lib/tls/engine.c. I have the new config code mostly done too, but it's dependent on having an internal registry for OpenSSL engines.

[minfrin]

Definite +1 for supporting this functionality.

An observation about OpenSSL v3 and up - the engine interface has been deprecated in favour of the provider interface: https://wiki.openssl.org/index.php/OpenSSL_3.0

For this reason it would be best to avoid the word "engine" in the configuration syntax.

The URL syntax is immune to the goings on underneath, and is defined in an RFC.

[arr2036]

Conversations at the IETF have demonstrated why this URL scheme is useful. If someone wants to rework this for OpenSSL 3.x we'd accept the patches.