Revert "pkcs11: Add support for 'serial' in PKCS#11 URI"

This reverts commit e39f11e77c.

The patch has some problems (the pkcs11 downstream test is failing)
and needs more investigation
This commit is contained in:
Norbert Pocs 2023-06-13 14:37:42 +02:00
parent c5082a3f81
commit 8f5b8fd2c5
2 changed files with 34 additions and 67 deletions

View File

@ -534,7 +534,7 @@ diff -up openssh-8.7p1/regress/unittests/Makefile.pkcs11-uri openssh-8.7p1/regre
diff -up openssh-8.7p1/regress/unittests/pkcs11/tests.c.pkcs11-uri openssh-8.7p1/regress/unittests/pkcs11/tests.c
--- openssh-8.7p1/regress/unittests/pkcs11/tests.c.pkcs11-uri 2021-08-30 13:07:43.664700104 +0200
+++ openssh-8.7p1/regress/unittests/pkcs11/tests.c 2021-08-30 13:07:43.664700104 +0200
@@ -0,0 +1,342 @@
@@ -0,0 +1,337 @@
+/*
+ * Copyright (c) 2017 Red Hat
+ *
@ -563,7 +563,7 @@ diff -up openssh-8.7p1/regress/unittests/pkcs11/tests.c.pkcs11-uri openssh-8.7p1
+#include "sshbuf.h"
+#include "ssh-pkcs11-uri.h"
+
+#define EMPTY_URI compose_uri(NULL, 0, NULL, NULL, NULL, NULL, NULL, NULL, NULL)
+#define EMPTY_URI compose_uri(NULL, 0, NULL, NULL, NULL, NULL, NULL, NULL)
+
+/* prototypes are not public -- specify them here internally for tests */
+struct sshbuf *percent_encode(const char *, size_t, char *);
@ -596,10 +596,6 @@ diff -up openssh-8.7p1/regress/unittests/pkcs11/tests.c.pkcs11-uri openssh-8.7p1
+ ASSERT_STRING_EQ(a->lib_manuf, b->lib_manuf);
+ else /* both should be null */
+ ASSERT_PTR_EQ(a->lib_manuf, b->lib_manuf);
+ if (b->serial != NULL)
+ ASSERT_STRING_EQ(a->serial, b->serial);
+ else /* both should be null */
+ ASSERT_PTR_EQ(a->serial, b->serial);
+}
+
+void
@ -634,7 +630,7 @@ diff -up openssh-8.7p1/regress/unittests/pkcs11/tests.c.pkcs11-uri openssh-8.7p1
+
+struct pkcs11_uri *
+compose_uri(unsigned char *id, size_t id_len, char *token, char *lib_manuf,
+ char *manuf, char *serial, char *module_path, char *object, char *pin)
+ char *manuf, char *module_path, char *object, char *pin)
+{
+ struct pkcs11_uri *uri = pkcs11_uri_init();
+ if (id_len > 0) {
@ -645,7 +641,6 @@ diff -up openssh-8.7p1/regress/unittests/pkcs11/tests.c.pkcs11-uri openssh-8.7p1
+ uri->token = token;
+ uri->lib_manuf = lib_manuf;
+ uri->manuf = manuf;
+ uri->serial = serial;
+ uri->object = object;
+ uri->pin = pin;
+ return uri;
@ -656,47 +651,47 @@ diff -up openssh-8.7p1/regress/unittests/pkcs11/tests.c.pkcs11-uri openssh-8.7p1
+{
+ /* path arguments */
+ check_parse("pkcs11:id=%01",
+ compose_uri("\x01", 1, NULL, NULL, NULL, NULL, NULL, NULL, NULL));
+ compose_uri("\x01", 1, NULL, NULL, NULL, NULL, NULL, NULL));
+ check_parse("pkcs11:id=%00%01",
+ compose_uri("\x00\x01", 2, NULL, NULL, NULL, NULL, NULL, NULL, NULL));
+ compose_uri("\x00\x01", 2, NULL, NULL, NULL, NULL, NULL, NULL));
+ check_parse("pkcs11:token=SSH%20Keys",
+ compose_uri(NULL, 0, "SSH Keys", NULL, NULL, NULL, NULL, NULL, NULL));
+ compose_uri(NULL, 0, "SSH Keys", NULL, NULL, NULL, NULL, NULL));
+ check_parse("pkcs11:library-manufacturer=OpenSC",
+ compose_uri(NULL, 0, NULL, "OpenSC", NULL, NULL, NULL, NULL, NULL));
+ compose_uri(NULL, 0, NULL, "OpenSC", NULL, NULL, NULL, NULL));
+ check_parse("pkcs11:manufacturer=piv_II",
+ compose_uri(NULL, 0, NULL, NULL, "piv_II", NULL, NULL, NULL, NULL));
+ compose_uri(NULL, 0, NULL, NULL, "piv_II", NULL, NULL, NULL));
+ check_parse("pkcs11:object=SIGN%20Key",
+ compose_uri(NULL, 0, NULL, NULL, NULL, NULL, NULL, "SIGN Key", NULL));
+ compose_uri(NULL, 0, NULL, NULL, NULL, NULL, "SIGN Key", NULL));
+ /* query arguments */
+ check_parse("pkcs11:?module-path=/usr/lib64/p11-kit-proxy.so",
+ compose_uri(NULL, 0, NULL, NULL, NULL, NULL, "/usr/lib64/p11-kit-proxy.so", NULL, NULL));
+ compose_uri(NULL, 0, NULL, NULL, NULL, "/usr/lib64/p11-kit-proxy.so", NULL, NULL));
+ check_parse("pkcs11:?pin-value=123456",
+ compose_uri(NULL, 0, NULL, NULL, NULL, NULL, NULL, NULL, "123456"));
+ compose_uri(NULL, 0, NULL, NULL, NULL, NULL, NULL, "123456"));
+
+ /* combinations */
+ /* ID SHOULD be percent encoded */
+ check_parse("pkcs11:token=SSH%20Key;id=0",
+ compose_uri("0", 1, "SSH Key", NULL, NULL, NULL, NULL, NULL, NULL));
+ compose_uri("0", 1, "SSH Key", NULL, NULL, NULL, NULL, NULL));
+ check_parse(
+ "pkcs11:manufacturer=CAC?module-path=/usr/lib64/p11-kit-proxy.so",
+ compose_uri(NULL, 0, NULL, NULL, "CAC", NULL,
+ compose_uri(NULL, 0, NULL, NULL, "CAC",
+ "/usr/lib64/p11-kit-proxy.so", NULL, NULL));
+ check_parse(
+ "pkcs11:object=RSA%20Key?module-path=/usr/lib64/pkcs11/opencryptoki.so",
+ compose_uri(NULL, 0, NULL, NULL, NULL, NULL,
+ compose_uri(NULL, 0, NULL, NULL, NULL,
+ "/usr/lib64/pkcs11/opencryptoki.so", "RSA Key", NULL));
+ check_parse("pkcs11:?module-path=/usr/lib64/p11-kit-proxy.so&pin-value=123456",
+ compose_uri(NULL, 0, NULL, NULL, NULL, NULL, "/usr/lib64/p11-kit-proxy.so", NULL, "123456"));
+ compose_uri(NULL, 0, NULL, NULL, NULL, "/usr/lib64/p11-kit-proxy.so", NULL, "123456"));
+
+ /* empty path component matches everything */
+ check_parse("pkcs11:", EMPTY_URI);
+
+ /* empty string is a valid to match against (and different from NULL) */
+ check_parse("pkcs11:token=",
+ compose_uri(NULL, 0, "", NULL, NULL, NULL, NULL, NULL, NULL));
+ compose_uri(NULL, 0, "", NULL, NULL, NULL, NULL, NULL));
+ /* Percent character needs to be percent-encoded */
+ check_parse("pkcs11:token=%25",
+ compose_uri(NULL, 0, "%", NULL, NULL, NULL, NULL, NULL, NULL));
+ compose_uri(NULL, 0, "%", NULL, NULL, NULL, NULL, NULL));
+}
+
+static void
@ -708,7 +703,7 @@ diff -up openssh-8.7p1/regress/unittests/pkcs11/tests.c.pkcs11-uri openssh-8.7p1
+ check_parse_rv("pkcs11:id=%ZZ", EMPTY_URI, -1);
+ /* Space MUST be percent encoded -- XXX not enforced yet */
+ check_parse("pkcs11:token=SSH Keys",
+ compose_uri(NULL, 0, "SSH Keys", NULL, NULL, NULL, NULL, NULL, NULL));
+ compose_uri(NULL, 0, "SSH Keys", NULL, NULL, NULL, NULL, NULL));
+ /* MUST NOT contain duplicate attributes of the same name */
+ check_parse_rv("pkcs11:id=%01;id=%02", EMPTY_URI, -1);
+ /* MUST NOT contain duplicate attributes of the same name */
@ -739,29 +734,29 @@ diff -up openssh-8.7p1/regress/unittests/pkcs11/tests.c.pkcs11-uri openssh-8.7p1
+{
+ /* path arguments */
+ check_gen("pkcs11:id=%01",
+ compose_uri("\x01", 1, NULL, NULL, NULL, NULL, NULL, NULL, NULL));
+ compose_uri("\x01", 1, NULL, NULL, NULL, NULL, NULL, NULL));
+ check_gen("pkcs11:id=%00%01",
+ compose_uri("\x00\x01", 2, NULL, NULL, NULL, NULL, NULL, NULL, NULL));
+ compose_uri("\x00\x01", 2, NULL, NULL, NULL, NULL, NULL, NULL));
+ check_gen("pkcs11:token=SSH%20Keys", /* space must be percent encoded */
+ compose_uri(NULL, 0, "SSH Keys", NULL, NULL, NULL, NULL, NULL, NULL));
+ compose_uri(NULL, 0, "SSH Keys", NULL, NULL, NULL, NULL, NULL));
+ /* library-manufacturer is not implmented now */
+ /*check_gen("pkcs11:library-manufacturer=OpenSC",
+ compose_uri(NULL, 0, NULL, "OpenSC", NULL, NULL, NULL, NULL, NULL));*/
+ compose_uri(NULL, 0, NULL, "OpenSC", NULL, NULL, NULL, NULL));*/
+ check_gen("pkcs11:manufacturer=piv_II",
+ compose_uri(NULL, 0, NULL, NULL, "piv_II", NULL, NULL, NULL, NULL));
+ compose_uri(NULL, 0, NULL, NULL, "piv_II", NULL, NULL, NULL));
+ check_gen("pkcs11:object=RSA%20Key",
+ compose_uri(NULL, 0, NULL, NULL, NULL, NULL, NULL, "RSA Key", NULL));
+ compose_uri(NULL, 0, NULL, NULL, NULL, NULL, "RSA Key", NULL));
+ /* query arguments */
+ check_gen("pkcs11:?module-path=/usr/lib64/p11-kit-proxy.so",
+ compose_uri(NULL, 0, NULL, NULL, NULL, NULL, "/usr/lib64/p11-kit-proxy.so", NULL, NULL));
+ compose_uri(NULL, 0, NULL, NULL, NULL, "/usr/lib64/p11-kit-proxy.so", NULL, NULL));
+
+ /* combinations */
+ check_gen("pkcs11:id=%02;token=SSH%20Keys",
+ compose_uri("\x02", 1, "SSH Keys", NULL, NULL, NULL, NULL, NULL, NULL));
+ compose_uri("\x02", 1, "SSH Keys", NULL, NULL, NULL, NULL, NULL));
+ check_gen("pkcs11:id=%EE%02?module-path=/usr/lib64/p11-kit-proxy.so",
+ compose_uri("\xEE\x02", 2, NULL, NULL, NULL, NULL, "/usr/lib64/p11-kit-proxy.so", NULL, NULL));
+ compose_uri("\xEE\x02", 2, NULL, NULL, NULL, "/usr/lib64/p11-kit-proxy.so", NULL, NULL));
+ check_gen("pkcs11:object=Encryption%20Key;manufacturer=piv_II",
+ compose_uri(NULL, 0, NULL, NULL, "piv_II", NULL, NULL, "Encryption Key", NULL));
+ compose_uri(NULL, 0, NULL, NULL, "piv_II", NULL, "Encryption Key", NULL));
+
+ /* empty path component matches everything */
+ check_gen("pkcs11:", EMPTY_URI);
@ -1550,7 +1545,7 @@ diff -up openssh-8.7p1/ssh-pkcs11.c.pkcs11-uri openssh-8.7p1/ssh-pkcs11.c
}
static RSA_METHOD *rsa_method;
@@ -195,6 +286,56 @@ static EC_KEY_METHOD *ec_key_method;
@@ -195,6 +286,55 @@ static EC_KEY_METHOD *ec_key_method;
static int ec_key_idx = 0;
#endif /* OPENSSL_HAS_ECC && HAVE_EC_KEY_METHOD_NEW */
@ -1592,7 +1587,6 @@ diff -up openssh-8.7p1/ssh-pkcs11.c.pkcs11-uri openssh-8.7p1/ssh-pkcs11.c
+ uri.module_path = k11->provider->module->module_path;
+ uri.lib_manuf = k11->provider->module->info.manufacturerID;
+ uri.manuf = k11->provider->module->slotinfo[k11->slotidx].token.manufacturerID;
+ uri.serial = k11->provider->module->slotinfo[k11->slotidx].token.serialNumber;
+
+ p = pkcs11_uri_get(&uri);
+ /* do not cleanup -- we do not allocate here, only reference */
@ -2163,7 +2157,7 @@ diff -up openssh-8.7p1/ssh-pkcs11.c.pkcs11-uri openssh-8.7p1/ssh-pkcs11.c
int ret = -1;
struct pkcs11_provider *p = NULL;
void *handle = NULL;
@@ -1517,164 +1702,305 @@ pkcs11_register_provider(char *provider_
@@ -1517,164 +1702,298 @@ pkcs11_register_provider(char *provider_
CK_FUNCTION_LIST *f = NULL;
CK_TOKEN_INFO *token;
CK_ULONG i;
@ -2407,13 +2401,6 @@ diff -up openssh-8.7p1/ssh-pkcs11.c.pkcs11-uri openssh-8.7p1/ssh-pkcs11.c
+ "manufacturerID (%s) specified by PKCS#11 URI in "
+ "slot %lu", token->manufacturerID, (unsigned long)i);
+ continue;
+ }
+ if (uri->serial != NULL &&
+ strcmp(token->serialNumber, uri->serial) != 0) {
+ debug2_f("ignoring token not matching requrested "
+ "serialNumber (%s) specified by PKCS#11 URI in "
+ "slot %lu", token->serialNumber, (unsigned long)i);
+ continue;
+ }
debug("provider %s slot %lu: label <%s> manufacturerID <%s> "
"model <%s> serial <%s> flags 0x%lx",
@ -2604,7 +2591,7 @@ diff -up openssh-8.7p1/ssh-pkcs11.h.pkcs11-uri openssh-8.7p1/ssh-pkcs11.h
diff -up openssh-8.7p1/ssh-pkcs11-uri.c.pkcs11-uri openssh-8.7p1/ssh-pkcs11-uri.c
--- openssh-8.7p1/ssh-pkcs11-uri.c.pkcs11-uri 2021-08-30 13:07:43.667700130 +0200
+++ openssh-8.7p1/ssh-pkcs11-uri.c 2021-08-30 13:07:43.667700130 +0200
@@ -0,0 +1,437 @@
@@ -0,0 +1,419 @@
+/*
+ * Copyright (c) 2017 Red Hat
+ *
@ -2647,14 +2634,13 @@ diff -up openssh-8.7p1/ssh-pkcs11-uri.c.pkcs11-uri openssh-8.7p1/ssh-pkcs11-uri.
+#define PKCS11_URI_OBJECT "object"
+#define PKCS11_URI_LIB_MANUF "library-manufacturer"
+#define PKCS11_URI_MANUF "manufacturer"
+#define PKCS11_URI_SERIAL "serial"
+#define PKCS11_URI_MODULE_PATH "module-path"
+#define PKCS11_URI_PIN_VALUE "pin-value"
+
+/* Keyword tokens. */
+typedef enum {
+ pId, pToken, pObject, pLibraryManufacturer, pManufacturer, pSerial,
+ pModulePath, pPinValue, pBadOption
+ pId, pToken, pObject, pLibraryManufacturer, pManufacturer, pModulePath,
+ pPinValue, pBadOption
+} pkcs11uriOpCodes;
+
+/* Textual representation of the tokens. */
@ -2667,7 +2653,6 @@ diff -up openssh-8.7p1/ssh-pkcs11-uri.c.pkcs11-uri openssh-8.7p1/ssh-pkcs11-uri.
+ { PKCS11_URI_OBJECT, pObject },
+ { PKCS11_URI_LIB_MANUF, pLibraryManufacturer },
+ { PKCS11_URI_MANUF, pManufacturer },
+ { PKCS11_URI_SERIAL, pSerial },
+ { PKCS11_URI_MODULE_PATH, pModulePath },
+ { PKCS11_URI_PIN_VALUE, pPinValue },
+ { NULL, pBadOption }
@ -2826,16 +2811,6 @@ diff -up openssh-8.7p1/ssh-pkcs11-uri.c.pkcs11-uri openssh-8.7p1/ssh-pkcs11-uri.
+ goto err;
+ }
+
+ /* Write serial */
+ if (uri->serial) {
+ struct sshbuf *serial = percent_encode(uri->serial,
+ strlen(uri->serial), PKCS11_URI_WHITELIST);
+ path = pkcs11_uri_append(path, PKCS11_URI_PATH_SEPARATOR,
+ PKCS11_URI_SERIAL, serial);
+ if (path == NULL)
+ goto err;
+ }
+
+ /* Write module_path */
+ if (uri->module_path) {
+ struct sshbuf *module = percent_encode(uri->module_path,
@ -2878,7 +2853,6 @@ diff -up openssh-8.7p1/ssh-pkcs11-uri.c.pkcs11-uri openssh-8.7p1/ssh-pkcs11-uri.
+ free(pkcs11->object);
+ free(pkcs11->lib_manuf);
+ free(pkcs11->manuf);
+ free(pkcs11->serial);
+ if (pkcs11->pin)
+ freezero(pkcs11->pin, strlen(pkcs11->pin));
+ free(pkcs11);
@ -2974,11 +2948,6 @@ diff -up openssh-8.7p1/ssh-pkcs11-uri.c.pkcs11-uri openssh-8.7p1/ssh-pkcs11-uri.
+ charptr = &pkcs11->manuf;
+ goto parse_string;
+
+ case pSerial:
+ /* CK_TOKEN_INFO -> serialNumber */
+ charptr = &pkcs11->serial;
+ goto parse_string;
+
+ case pLibraryManufacturer:
+ /* CK_INFO -> manufacturerID */
+ charptr = &pkcs11->lib_manuf;
@ -3045,7 +3014,7 @@ diff -up openssh-8.7p1/ssh-pkcs11-uri.c.pkcs11-uri openssh-8.7p1/ssh-pkcs11-uri.
diff -up openssh-8.7p1/ssh-pkcs11-uri.h.pkcs11-uri openssh-8.7p1/ssh-pkcs11-uri.h
--- openssh-8.7p1/ssh-pkcs11-uri.h.pkcs11-uri 2021-08-30 13:07:43.667700130 +0200
+++ openssh-8.7p1/ssh-pkcs11-uri.h 2021-08-30 13:07:43.667700130 +0200
@@ -0,0 +1,43 @@
@@ -0,0 +1,42 @@
+/*
+ * Copyright (c) 2017 Red Hat
+ *
@ -3077,7 +3046,6 @@ diff -up openssh-8.7p1/ssh-pkcs11-uri.h.pkcs11-uri openssh-8.7p1/ssh-pkcs11-uri.
+ char *object;
+ char *lib_manuf;
+ char *manuf;
+ char *serial;
+ /* query */
+ char *module_path;
+ char *pin; /* Only parsed, but not printed */

View File

@ -753,7 +753,6 @@ test -f %{sysconfig_anaconda} && \
* Wed May 24 2023 Norbert Pocs <npocs@redhat.com> - 9.0p1-18
- Fix pkcs11 issue with the recent changes
- Add support for 'serial' in PKCS#11 URI
- Clarify HostKeyAlgorithms relation with crypto-policies
* Fri Apr 14 2023 Dmitry Belyavskiy <dbelyavs@redhat.com> - 9.0p1-17