From 8f5b8fd2c568e5e0bee703818f555d958008be5e Mon Sep 17 00:00:00 2001 From: Norbert Pocs Date: Tue, 13 Jun 2023 14:37:42 +0200 Subject: [PATCH] Revert "pkcs11: Add support for 'serial' in PKCS#11 URI" This reverts commit e39f11e77cda116cf5de254bbc0f4b0097cdc43b. The patch has some problems (the pkcs11 downstream test is failing) and needs more investigation --- openssh-8.0p1-pkcs11-uri.patch | 100 +++++++++++---------------------- openssh.spec | 1 - 2 files changed, 34 insertions(+), 67 deletions(-) diff --git a/openssh-8.0p1-pkcs11-uri.patch b/openssh-8.0p1-pkcs11-uri.patch index be38500..affdd72 100644 --- a/openssh-8.0p1-pkcs11-uri.patch +++ b/openssh-8.0p1-pkcs11-uri.patch @@ -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 */ diff --git a/openssh.spec b/openssh.spec index 6eab8da..8da21d4 100644 --- a/openssh.spec +++ b/openssh.spec @@ -753,7 +753,6 @@ test -f %{sysconfig_anaconda} && \ * Wed May 24 2023 Norbert Pocs - 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 - 9.0p1-17