From 57697eda68a3343c2e54e5f8f3f4ce65a99383f5 Mon Sep 17 00:00:00 2001 From: Daiki Ueno Date: Wed, 31 Jan 2018 14:07:51 +0100 Subject: trust: Filter out duplicate extensions The trust policy module keeps all the objects in the database, while PKIX doesn't allow multiple extensions identified by the same OID can be attached to a certificate. Add a check to C_FindObjects to exclude any duplicates and only return the first matching object. It would be better if the module rejects such duplicates when loading, but it would make startup slower. https://bugzilla.redhat.com/show_bug.cgi?id=1141241 --- trust/input/extensions.p11-kit | 23 +++++++++++++++++++++ trust/input/extensions.pem | 13 ++++++++++++ trust/module.c | 42 ++++++++++++++++++++++++++++++++++++--- trust/test-module.c | 45 +++++++++++++++++++++++++++++++++++++++++- trust/test-token.c | 2 +- 5 files changed, 120 insertions(+), 5 deletions(-) create mode 100644 trust/input/extensions.p11-kit create mode 100644 trust/input/extensions.pem diff --git a/trust/input/extensions.p11-kit b/trust/input/extensions.p11-kit new file mode 100644 index 0000000..7a2fdb0 --- /dev/null +++ b/trust/input/extensions.p11-kit @@ -0,0 +1,23 @@ +[p11-kit-object-v1] +class: x-certificate-extension +label: "Example CA restriction for example.com and corp.example.com" +object-id: 2.5.29.30 +value: "%30%2e%06%03%55%1d%1e%04%27%30%25%a0%23%30%0d%82%0b%65%78%61%6d%70%6c%65%2e%63%6f%6d%30%12%82%10%63%6f%72%70%2e%65%78%61%6d%70%6c%65%2e%63%6f%6d" +-----BEGIN PUBLIC KEY----- +MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDRtTajie6qgC9T/RJ1PvN6ntav ++rwcYBBLJoETGlnj/kVsOAQ5J0ZX/dW8jYoQtjvUCoFaRS/sPoHw2U5Pl99LMg8I +sSaivWlhXWY5Yy8QcDX7B4UK/1cSwfSDHfnG06S2cCuAoUB/SE7ZreuAzM+SwdGD +ZAEjR469MZgFa2t8NwIDAQAB +-----END PUBLIC KEY----- + +[p11-kit-object-v1] +class: x-certificate-extension +label: "Example CA restriction for example.com and corp.example.org" +object-id: 2.5.29.30 +value: "%30%2e%06%03%55%1d%1e%04%27%30%25%a0%23%30%0d%82%0b%65%78%61%6d%70%6c%65%2e%63%6f%6d%30%12%82%10%63%6f%72%70%2e%65%78%61%6d%70%6c%65%2e%6f%72%67" +-----BEGIN PUBLIC KEY----- +MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDRtTajie6qgC9T/RJ1PvN6ntav ++rwcYBBLJoETGlnj/kVsOAQ5J0ZX/dW8jYoQtjvUCoFaRS/sPoHw2U5Pl99LMg8I +sSaivWlhXWY5Yy8QcDX7B4UK/1cSwfSDHfnG06S2cCuAoUB/SE7ZreuAzM+SwdGD +ZAEjR469MZgFa2t8NwIDAQAB +-----END PUBLIC KEY----- diff --git a/trust/input/extensions.pem b/trust/input/extensions.pem new file mode 100644 index 0000000..8369815 --- /dev/null +++ b/trust/input/extensions.pem @@ -0,0 +1,13 @@ +-----BEGIN CERTIFICATE----- +MIIB7DCCAVWgAwIBAgIIWRMNpygap1cwDQYJKoZIhvcNAQELBQAwFTETMBEGA1UE +AxMKRXhhbXBsZSBDQTAgFw0xNzA1MTAxMjU1MDVaGA85OTk5MTIzMTIzNTk1OVow +FTETMBEGA1UEAxMKRXhhbXBsZSBDQTCBnzANBgkqhkiG9w0BAQEFAAOBjQAwgYkC +gYEA0bU2o4nuqoAvU/0SdT7zep7Wr/q8HGAQSyaBExpZ4/5FbDgEOSdGV/3VvI2K +ELY71AqBWkUv7D6B8NlOT5ffSzIPCLEmor1pYV1mOWMvEHA1+weFCv9XEsH0gx35 +xtOktnArgKFAf0hO2a3rgMzPksHRg2QBI0eOvTGYBWtrfDcCAwEAAaNDMEEwDwYD +VR0TAQH/BAUwAwEB/zAPBgNVHQ8BAf8EBQMDBwQAMB0GA1UdDgQWBBTAf2LZgNFX +6uQKWnFh05Br9JgOUjANBgkqhkiG9w0BAQsFAAOBgQA0xZVI3WmyWaa56nTSiuco +3u0Cye7N8bSzlfi2kmyh8efA7/OCyBuUzCtvmiftsfcG6fPz3A8fdk5sA2oy0gyY +kJXukhHmLP0FHLVpa3vw1Sva5AlAkLGeQ25aSeYVZCASalMAAS72WAhsKdaD5TRS +ifWyno0SswLLpXIJsLW2Lw== +-----END CERTIFICATE----- diff --git a/trust/module.c b/trust/module.c index 7fce465..e6fb7a9 100644 --- a/trust/module.c +++ b/trust/module.c @@ -45,6 +45,7 @@ #include "library.h" #include "message.h" #include "module.h" +#include "oid.h" #include "parser.h" #include "path.h" #include "pkcs11.h" @@ -77,6 +78,8 @@ typedef struct _FindObjects { CK_ATTRIBUTE *match; CK_OBJECT_HANDLE *snapshot; CK_ULONG iterator; + CK_ATTRIBUTE *public_key; + p11_dict *extensions; } FindObjects; static CK_FUNCTION_LIST sys_function_list; @@ -87,6 +90,7 @@ find_objects_free (void *data) FindObjects *find = data; p11_attrs_free (find->match); free (find->snapshot); + p11_dict_free (find->extensions); free (find); } @@ -1147,6 +1151,7 @@ sys_C_FindObjectsInit (CK_SESSION_HANDLE handle, char *string; CK_RV rv; int n = 0; + CK_OBJECT_CLASS klass; if (p11_debugging) { string = p11_attrs_to_string (template, count); @@ -1190,6 +1195,14 @@ sys_C_FindObjectsInit (CK_SESSION_HANDLE handle, find->iterator = 0; find->snapshot = p11_index_snapshot (indices[0], indices[1], template, count); warn_if_fail (find->snapshot != NULL); + + if (p11_attrs_find_ulong (find->match, CKA_CLASS, &klass) && + klass == CKO_X_CERTIFICATE_EXTENSION) { + find->public_key = p11_attrs_find (find->match, CKA_PUBLIC_KEY_INFO); + find->extensions = p11_dict_new (p11_oid_hash, + p11_oid_equal, + free, NULL); + } } if (!find || !find->snapshot || !find->match) @@ -1243,10 +1256,10 @@ match_for_broken_nss_serial_number_lookups (CK_ATTRIBUTE *attr, static bool find_objects_match (CK_ATTRIBUTE *attrs, - CK_ATTRIBUTE *match) + FindObjects *find) { CK_OBJECT_CLASS klass; - CK_ATTRIBUTE *attr; + CK_ATTRIBUTE *attr, *match = find->match; for (; !p11_attrs_terminator (match); match++) { attr = p11_attrs_find ((CK_ATTRIBUTE *)attrs, match->type); @@ -1274,6 +1287,29 @@ find_objects_match (CK_ATTRIBUTE *attrs, return false; } + /* + * WORKAROUND: We keep all objects in the database, while PKIX + * doesn't allow multiple extensions identified by the same + * OID can be attached to a certificate. Check any duplicate + * and only return the first matching object. + */ + if (find->public_key && + p11_attrs_find_ulong (attrs, CKA_CLASS, &klass) && + klass == CKO_X_CERTIFICATE_EXTENSION) { + CK_ATTRIBUTE *oid = p11_attrs_find (attrs, CKA_OBJECT_ID); + if (oid) { + void *value; + if (p11_oid_simple (oid->pValue, oid->ulValueLen) && + p11_dict_get (find->extensions, oid->pValue)) { + p11_debug ("duplicate extension object"); + return false; + } + value = memdup (oid->pValue, oid->ulValueLen); + return_val_if_fail (value != NULL, false); + p11_dict_set (find->extensions, value, value); + } + } + return true; } @@ -1317,7 +1353,7 @@ sys_C_FindObjects (CK_SESSION_HANDLE handle, if (attrs == NULL) continue; - if (find_objects_match (attrs, find->match)) { + if (find_objects_match (attrs, find)) { objects[matched] = object; matched++; } diff --git a/trust/test-module.c b/trust/test-module.c index 1729b41..36fbfe4 100644 --- a/trust/test-module.c +++ b/trust/test-module.c @@ -623,13 +623,55 @@ test_find_certificates (void) CK_ULONG i; count = find_objects (match, sessions, objects, 16); - assert_num_eq (8, count); + assert_num_eq (9, count); for (i = 0; i < count; i++) check_certificate (sessions[i], objects[i]); } static void +test_find_extensions (void) +{ + CK_OBJECT_CLASS klass = CKO_X_CERTIFICATE_EXTENSION; + unsigned char spki[] = { + 0x30, 0x81, 0x9f, 0x30, 0x0d, 0x06, 0x09, 0x2a, + 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x01, 0x01, + 0x05, 0x00, 0x03, 0x81, 0x8d, 0x00, 0x30, 0x81, + 0x89, 0x02, 0x81, 0x81, 0x00, 0xd1, 0xb5, 0x36, + 0xa3, 0x89, 0xee, 0xaa, 0x80, 0x2f, 0x53, 0xfd, + 0x12, 0x75, 0x3e, 0xf3, 0x7a, 0x9e, 0xd6, 0xaf, + 0xfa, 0xbc, 0x1c, 0x60, 0x10, 0x4b, 0x26, 0x81, + 0x13, 0x1a, 0x59, 0xe3, 0xfe, 0x45, 0x6c, 0x38, + 0x04, 0x39, 0x27, 0x46, 0x57, 0xfd, 0xd5, 0xbc, + 0x8d, 0x8a, 0x10, 0xb6, 0x3b, 0xd4, 0x0a, 0x81, + 0x5a, 0x45, 0x2f, 0xec, 0x3e, 0x81, 0xf0, 0xd9, + 0x4e, 0x4f, 0x97, 0xdf, 0x4b, 0x32, 0x0f, 0x08, + 0xb1, 0x26, 0xa2, 0xbd, 0x69, 0x61, 0x5d, 0x66, + 0x39, 0x63, 0x2f, 0x10, 0x70, 0x35, 0xfb, 0x07, + 0x85, 0x0a, 0xff, 0x57, 0x12, 0xc1, 0xf4, 0x83, + 0x1d, 0xf9, 0xc6, 0xd3, 0xa4, 0xb6, 0x70, 0x2b, + 0x80, 0xa1, 0x40, 0x7f, 0x48, 0x4e, 0xd9, 0xad, + 0xeb, 0x80, 0xcc, 0xcf, 0x92, 0xc1, 0xd1, 0x83, + 0x64, 0x01, 0x23, 0x47, 0x8e, 0xbd, 0x31, 0x98, + 0x05, 0x6b, 0x6b, 0x7c, 0x37, 0x02, 0x03, 0x01, + 0x00, 0x01 + }; + + CK_ATTRIBUTE match[] = { + { CKA_CLASS, &klass, sizeof (klass) }, + { CKA_PUBLIC_KEY_INFO, spki, sizeof (spki) }, + { CKA_INVALID, } + }; + + CK_OBJECT_HANDLE objects[16]; + CK_SESSION_HANDLE sessions[16]; + CK_ULONG count; + + count = find_objects (match, sessions, objects, 16); + assert_num_eq (1, count); +} + +static void test_find_builtin (void) { CK_OBJECT_CLASS klass = CKO_NSS_BUILTIN_ROOT_LIST; @@ -1194,6 +1236,7 @@ main (int argc, p11_test (test_get_session_info, "/module/get_session_info"); p11_test (test_close_all_sessions, "/module/close_all_sessions"); p11_test (test_find_certificates, "/module/find_certificates"); + p11_test (test_find_extensions, "/module/find_extensions"); p11_test (test_find_builtin, "/module/find_builtin"); p11_test (test_lookup_invalid, "/module/lookup_invalid"); p11_test (test_remove_token, "/module/remove_token"); diff --git a/trust/test-token.c b/trust/test-token.c index 3e7d735..0206bc1 100644 --- a/trust/test-token.c +++ b/trust/test-token.c @@ -102,7 +102,7 @@ test_token_load (void *path) int count; count = p11_token_load (test.token); - assert_num_eq (6, count); + assert_num_eq (8, count); /* A certificate and trust object for each parsed object */ index = p11_token_index (test.token); -- cgit v1.1