From acf8c4a91a76bf8049f6bfbd95b04e2e36bae4ea Mon Sep 17 00:00:00 2001 From: Daiki Ueno Date: Thu, 18 May 2017 10:45:26 +0200 Subject: [PATCH 1/2] Revert "trust: Honor "modifiable" setting in persist file" This reverts commit 8eed1e60b0921d05872e2f43eee9088cef038d7e, which broke "trust anchor --remove". --- trust/input/verisign-v1.p11-kit | 1 - trust/parser.c | 10 +--------- trust/test-parser.c | 1 - 3 files changed, 1 insertion(+), 11 deletions(-) diff --git a/trust/input/verisign-v1.p11-kit b/trust/input/verisign-v1.p11-kit index aea49ea..eaa080d 100644 --- a/trust/input/verisign-v1.p11-kit +++ b/trust/input/verisign-v1.p11-kit @@ -1,6 +1,5 @@ [p11-kit-object-v1] trusted: true -modifiable: false -----BEGIN CERTIFICATE----- MIICPDCCAaUCED9pHoGc8JpK83P/uUii5N0wDQYJKoZIhvcNAQEFBQAwXzELMAkG diff --git a/trust/parser.c b/trust/parser.c index 52d1128..41513d4 100644 --- a/trust/parser.c +++ b/trust/parser.c @@ -610,7 +610,6 @@ p11_parser_format_persist (p11_parser *parser, { CK_BBOOL modifiablev = CK_TRUE; CK_ATTRIBUTE *attrs; - CK_ATTRIBUTE *attr; p11_array *objects; bool ret; int i; @@ -631,14 +630,7 @@ p11_parser_format_persist (p11_parser *parser, ret = p11_persist_read (parser->persist, parser->basename, data, length, objects); if (ret) { for (i = 0; i < objects->num; i++) { - /* By default, we mark objects read from a persist - * file as modifiable, as the persist format is - * writable. However, if CKA_MODIFIABLE is explictly - * set in the file, respect the setting. */ - attrs = objects->elem[i]; - attr = p11_attrs_find_valid (objects->elem[i], CKA_MODIFIABLE); - if (!attr) - attrs = p11_attrs_build (attrs, &modifiable, NULL); + attrs = p11_attrs_build (objects->elem[i], &modifiable, NULL); sink_object (parser, attrs); } } diff --git a/trust/test-parser.c b/trust/test-parser.c index 088cff9..b5c2525 100644 --- a/trust/test-parser.c +++ b/trust/test-parser.c @@ -168,7 +168,6 @@ test_parse_p11_kit_persist (void) { CKA_CLASS, &certificate, sizeof (certificate) }, { CKA_VALUE, (void *)verisign_v1_ca, sizeof (verisign_v1_ca) }, { CKA_TRUSTED, &truev, sizeof (truev) }, - { CKA_MODIFIABLE, &falsev, sizeof (falsev) }, { CKA_X_DISTRUSTED, &falsev, sizeof (falsev) }, { CKA_INVALID }, }; -- 2.9.4 From 66c6a7e912d39d66cd4cc91375ac7be418bf7176 Mon Sep 17 00:00:00 2001 From: Daiki Ueno Date: Thu, 18 May 2017 11:11:45 +0200 Subject: [PATCH 2/2] trust: Check magic comment in persist file for modifiablity A persistent file written by the trust module starts with the line "# This file has been auto-generated and written by p11-kit". This can be used as a magic word to determine whether the objects read from a .p11-kit file are read-only. --- trust/parser.c | 6 +++++- trust/persist.c | 9 ++++++++- trust/test-token.c | 1 + 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/trust/parser.c b/trust/parser.c index 41513d4..abe86fc 100644 --- a/trust/parser.c +++ b/trust/parser.c @@ -49,6 +49,7 @@ #include "pem.h" #include "pkcs11x.h" #include "persist.h" +#include "types.h" #include "x509.h" #include @@ -630,7 +631,10 @@ p11_parser_format_persist (p11_parser *parser, ret = p11_persist_read (parser->persist, parser->basename, data, length, objects); if (ret) { for (i = 0; i < objects->num; i++) { - attrs = p11_attrs_build (objects->elem[i], &modifiable, NULL); + CK_BBOOL generatedv; + attrs = objects->elem[i]; + if (p11_attrs_find_bool (attrs, CKA_X_GENERATED, &generatedv) && generatedv) + attrs = p11_attrs_build (attrs, &modifiable, NULL); sink_object (parser, attrs); } } diff --git a/trust/persist.c b/trust/persist.c index 63a531e..928260e 100644 --- a/trust/persist.c +++ b/trust/persist.c @@ -631,6 +631,9 @@ p11_persist_read (p11_persist *persist, CK_ATTRIBUTE *attrs; bool failed; bool skip; + CK_BBOOL generatedv = CK_FALSE; + CK_ATTRIBUTE generated = { CKA_X_GENERATED, &generatedv, sizeof (generatedv) }; + static const char comment[] = "# This file has been auto-generated and written by p11-kit."; return_val_if_fail (persist != NULL, false); return_val_if_fail (objects != NULL, false); @@ -639,6 +642,10 @@ p11_persist_read (p11_persist *persist, attrs = NULL; failed = false; + if (length >= sizeof (comment) - 1 && + memcmp ((const char *)data, comment, sizeof (comment) - 1) == 0) + generatedv = CK_TRUE; + p11_lexer_init (&lexer, filename, (const char *)data, length); while (p11_lexer_next (&lexer, &failed)) { switch (lexer.tok_type) { @@ -650,7 +657,7 @@ p11_persist_read (p11_persist *persist, p11_lexer_msg (&lexer, "unrecognized or invalid section header"); skip = true; } else { - attrs = p11_attrs_build (NULL, NULL); + attrs = p11_attrs_build (NULL, &generated, NULL); return_val_if_fail (attrs != NULL, false); skip = false; } diff --git a/trust/test-token.c b/trust/test-token.c index ad22fcb..3e7d735 100644 --- a/trust/test-token.c +++ b/trust/test-token.c @@ -610,6 +610,7 @@ static void test_modify_multiple (void) { const char *test_data = + "# This file has been auto-generated and written by p11-kit.\n" "[p11-kit-object-v1]\n" "class: data\n" "label: \"first\"\n" -- 2.9.4 From d661194319f2375c1764125b449bf924c0cbc8a1 Mon Sep 17 00:00:00 2001 From: Daiki Ueno Date: Thu, 18 May 2017 14:27:36 +0200 Subject: [PATCH] trust: Simplify the check for the magic Instead of reusing the CKA_X_GENERATED attribute, check the file contents directly in the caller side. --- trust/parser.c | 7 +++---- trust/persist.c | 19 +++++++++++-------- trust/persist.h | 3 +++ 3 files changed, 17 insertions(+), 12 deletions(-) diff --git a/trust/parser.c b/trust/parser.c index abe86fc..f92cdc9 100644 --- a/trust/parser.c +++ b/trust/parser.c @@ -630,11 +630,10 @@ p11_parser_format_persist (p11_parser *parser, ret = p11_persist_read (parser->persist, parser->basename, data, length, objects); if (ret) { + if (!p11_persist_is_generated (data, length)) + modifiablev = CK_FALSE; for (i = 0; i < objects->num; i++) { - CK_BBOOL generatedv; - attrs = objects->elem[i]; - if (p11_attrs_find_bool (attrs, CKA_X_GENERATED, &generatedv) && generatedv) - attrs = p11_attrs_build (attrs, &modifiable, NULL); + attrs = p11_attrs_build (objects->elem[i], &modifiable, NULL); sink_object (parser, attrs); } } diff --git a/trust/persist.c b/trust/persist.c index 928260e..887b316 100644 --- a/trust/persist.c +++ b/trust/persist.c @@ -70,6 +70,16 @@ p11_persist_magic (const unsigned char *data, return (strnstr ((char *)data, "[" PERSIST_HEADER "]", length) != NULL); } +bool +p11_persist_is_generated (const unsigned char *data, + size_t length) +{ + static const char comment[] = + "# This file has been auto-generated and written by p11-kit."; + return length >= sizeof (comment) - 1 && + memcmp ((const char *)data, comment, sizeof (comment) - 1) == 0; +} + p11_persist * p11_persist_new (void) { @@ -631,9 +641,6 @@ p11_persist_read (p11_persist *persist, CK_ATTRIBUTE *attrs; bool failed; bool skip; - CK_BBOOL generatedv = CK_FALSE; - CK_ATTRIBUTE generated = { CKA_X_GENERATED, &generatedv, sizeof (generatedv) }; - static const char comment[] = "# This file has been auto-generated and written by p11-kit."; return_val_if_fail (persist != NULL, false); return_val_if_fail (objects != NULL, false); @@ -642,10 +649,6 @@ p11_persist_read (p11_persist *persist, attrs = NULL; failed = false; - if (length >= sizeof (comment) - 1 && - memcmp ((const char *)data, comment, sizeof (comment) - 1) == 0) - generatedv = CK_TRUE; - p11_lexer_init (&lexer, filename, (const char *)data, length); while (p11_lexer_next (&lexer, &failed)) { switch (lexer.tok_type) { @@ -657,7 +660,7 @@ p11_persist_read (p11_persist *persist, p11_lexer_msg (&lexer, "unrecognized or invalid section header"); skip = true; } else { - attrs = p11_attrs_build (NULL, &generated, NULL); + attrs = p11_attrs_build (NULL, NULL); return_val_if_fail (attrs != NULL, false); skip = false; } diff --git a/trust/persist.h b/trust/persist.h index 0ef142c..6344e4e 100644 --- a/trust/persist.h +++ b/trust/persist.h @@ -60,4 +60,7 @@ bool p11_persist_write (p11_persist *persist, void p11_persist_free (p11_persist *persist); +bool p11_persist_is_generated (const unsigned char *data, + size_t length); + #endif /* P11_PERSIST_H_ */ -- 2.9.4