From 24a33b1d93b2663faaca3fb36097d5bffc4797b7 Mon Sep 17 00:00:00 2001 From: Stef Walter Date: Mon, 16 Jul 2012 17:56:24 +0200 Subject: [PATCH] Use '.module' extension on module configs * And want alphanumeric/_.- filenames * Currently this is just a warning, soon it will be enforced * The name of a module does not include the extension Andreas Metzler and Ubuntu both worked on this patch, and I've made some more changes. See https://bugs.launchpad.net/ubuntu/+source/p11-kit/+bug/911436 https://bugs.freedesktop.org/show_bug.cgi?id=52158 --- doc/p11-kit-config.xml | 7 ++- p11-kit/conf.c | 56 ++++++++++++++++++-- tests/conf-test.c | 23 +++++--- tests/files/system-modules/{one => one.module} | 0 tests/files/system-modules/two | 5 -- .../{two-duplicate => two-duplicate.module} | 0 tests/files/system-modules/two.badname | 7 +++ tests/files/user-modules/{one => one.module} | 0 tests/files/user-modules/{three => three.module} | 0 tests/test-modules.c | 4 +- 10 files changed, 82 insertions(+), 20 deletions(-) rename tests/files/system-modules/{one => one.module} (100%) delete mode 100644 tests/files/system-modules/two rename tests/files/system-modules/{two-duplicate => two-duplicate.module} (100%) create mode 100644 tests/files/system-modules/two.badname rename tests/files/user-modules/{one => one.module} (100%) rename tests/files/user-modules/{three => three.module} (100%) diff --git a/doc/p11-kit-config.xml b/doc/p11-kit-config.xml index 11fb41f..a10cb7e 100644 --- a/doc/p11-kit-config.xml +++ b/doc/p11-kit-config.xml @@ -113,8 +113,11 @@ critical: yes Module Configuration Each configured PKCS#11 module has its own config file. These files - can be placed in various locations. - Most importantly each config file specifies the path of the PKCS#11 module to + can be placed in various locations. + The filename of the configuration file may consist of upper and lowercase letters + underscore, comma, dash and dots. The first characters needs to be an alphanumeric, + the filename should end with a .module extension. + Most importantly each config file specifies the path of the PKCS#11 module to load. A module config file has the following fields: diff --git a/p11-kit/conf.c b/p11-kit/conf.c index fdb591d..db730a7 100644 --- a/p11-kit/conf.c +++ b/p11-kit/conf.c @@ -458,6 +458,46 @@ finished: return result; } +static char * +calc_name_from_filename (const char *fname) +{ + /* We eventually want to settle on .module */ + static const char *suffix = ".module"; + static size_t suffix_len = 7; + const char *c = fname; + size_t fname_len; + size_t name_len; + char *name; + + assert (fname); + + /* Make sure the filename starts with an alphanumeric */ + if (!isalnum(*c)) + return NULL; + ++c; + + /* Only allow alnum, _, -, and . */ + while (*c) { + if (!isalnum(*c) && *c != '_' && *c != '-' && *c != '.') + return NULL; + ++c; + } + + /* Make sure we have one of the suffixes */ + fname_len = strlen (fname); + if (suffix_len >= fname_len) + return NULL; + name_len = (fname_len - suffix_len); + if (strcmp (fname + name_len, suffix) != 0) + return NULL; + + name = malloc (name_len + 1); + return_val_if_fail (name != NULL, NULL); + memcpy (name, fname, name_len); + name[name_len] = 0; + return name; +} + static int load_config_from_file (const char *configfile, const char *name, hashmap *configs) { @@ -468,20 +508,28 @@ load_config_from_file (const char *configfile, const char *name, hashmap *config assert (configfile); + key = calc_name_from_filename (name); + if (key == NULL) { + _p11_message ("invalid config filename, will be ignored in the future: %s", configfile); + key = strdup (name); + return_val_if_fail (key != NULL, -1); + } + config = _p11_conf_parse_file (configfile, 0); - if (!config) + if (!config) { + free (key); return -1; + } - prev = _p11_hash_get (configs, name); + prev = _p11_hash_get (configs, key); if (prev == NULL) { - key = strdup (name); - return_val_if_fail (key != NULL, -1); if (!_p11_hash_set (configs, key, config)) return_val_if_reached (-1); config = NULL; } else { if (_p11_conf_merge_defaults (prev, config) < 0) error = errno; + free (key); } /* If still set */ diff --git a/tests/conf-test.c b/tests/conf-test.c index eb04c71..92f7205 100644 --- a/tests/conf-test.c +++ b/tests/conf-test.c @@ -246,6 +246,15 @@ test_load_globals_user_sets_invalid (CuTest *tc) _p11_hash_free (config); } +static int +assert_msg_contains (const char *msg, + const char *text) +{ + if (msg == NULL) + return 0; + return strstr (msg, text) ? 1 : 0; +} + static void test_load_modules_merge (CuTest *tc) { @@ -258,14 +267,14 @@ test_load_modules_merge (CuTest *tc) SRCDIR "/files/system-modules", SRCDIR "/files/user-modules"); CuAssertPtrNotNull (tc, configs); - CuAssertStrEquals (tc, NULL, p11_kit_message ()); + CuAssertTrue (tc, assert_msg_contains (p11_kit_message (), "invalid config filename")); config = _p11_hash_get (configs, "one"); CuAssertPtrNotNull (tc, config); CuAssertStrEquals (tc, "mock-one.so", _p11_hash_get (config, "module")); CuAssertStrEquals (tc, _p11_hash_get (config, "setting"), "user1"); - config = _p11_hash_get (configs, "two"); + config = _p11_hash_get (configs, "two.badname"); CuAssertPtrNotNull (tc, config); CuAssertStrEquals (tc, "mock-two.so", _p11_hash_get (config, "module")); CuAssertStrEquals (tc, _p11_hash_get (config, "setting"), "system2"); @@ -290,14 +299,14 @@ test_load_modules_user_none (CuTest *tc) SRCDIR "/files/system-modules", SRCDIR "/files/user-modules"); CuAssertPtrNotNull (tc, configs); - CuAssertStrEquals (tc, NULL, p11_kit_message ()); + CuAssertTrue (tc, assert_msg_contains (p11_kit_message (), "invalid config filename")); config = _p11_hash_get (configs, "one"); CuAssertPtrNotNull (tc, config); CuAssertStrEquals (tc, "mock-one.so", _p11_hash_get (config, "module")); CuAssertStrEquals (tc, _p11_hash_get (config, "setting"), "system1"); - config = _p11_hash_get (configs, "two"); + config = _p11_hash_get (configs, "two.badname"); CuAssertPtrNotNull (tc, config); CuAssertStrEquals (tc, "mock-two.so", _p11_hash_get (config, "module")); CuAssertStrEquals (tc, _p11_hash_get (config, "setting"), "system2"); @@ -327,7 +336,7 @@ test_load_modules_user_only (CuTest *tc) CuAssertStrEquals (tc, _p11_hash_get (config, "module"), NULL); CuAssertStrEquals (tc, _p11_hash_get (config, "setting"), "user1"); - config = _p11_hash_get (configs, "two"); + config = _p11_hash_get (configs, "two.badname"); CuAssertPtrEquals (tc, NULL, config); config = _p11_hash_get (configs, "three"); @@ -350,14 +359,14 @@ test_load_modules_no_user (CuTest *tc) SRCDIR "/files/system-modules", SRCDIR "/files/non-existant"); CuAssertPtrNotNull (tc, configs); - CuAssertStrEquals (tc, NULL, p11_kit_message ()); + CuAssertTrue (tc, assert_msg_contains (p11_kit_message (), "invalid config filename")); config = _p11_hash_get (configs, "one"); CuAssertPtrNotNull (tc, config); CuAssertStrEquals (tc, "mock-one.so", _p11_hash_get (config, "module")); CuAssertStrEquals (tc, _p11_hash_get (config, "setting"), "system1"); - config = _p11_hash_get (configs, "two"); + config = _p11_hash_get (configs, "two.badname"); CuAssertPtrNotNull (tc, config); CuAssertStrEquals (tc, "mock-two.so", _p11_hash_get (config, "module")); CuAssertStrEquals (tc, _p11_hash_get (config, "setting"), "system2"); diff --git a/tests/files/system-modules/one b/tests/files/system-modules/one.module similarity index 100% rename from tests/files/system-modules/one rename to tests/files/system-modules/one.module diff --git a/tests/files/system-modules/two b/tests/files/system-modules/two deleted file mode 100644 index f391757..0000000 --- a/tests/files/system-modules/two +++ /dev/null @@ -1,5 +0,0 @@ - -module: mock-two.so -setting: system2 - -disable-in: test-disable, test-other \ No newline at end of file diff --git a/tests/files/system-modules/two-duplicate b/tests/files/system-modules/two-duplicate.module similarity index 100% rename from tests/files/system-modules/two-duplicate rename to tests/files/system-modules/two-duplicate.module diff --git a/tests/files/system-modules/two.badname b/tests/files/system-modules/two.badname new file mode 100644 index 0000000..b6d0f7f --- /dev/null +++ b/tests/files/system-modules/two.badname @@ -0,0 +1,7 @@ +# This module doesn't have a .module extension, but p11-kit doesn't yet +# enforce the naming, just warns, so it should still be loaded + +module: mock-two.so +setting: system2 + +disable-in: test-disable, test-other \ No newline at end of file diff --git a/tests/files/user-modules/one b/tests/files/user-modules/one.module similarity index 100% rename from tests/files/user-modules/one rename to tests/files/user-modules/one.module diff --git a/tests/files/user-modules/three b/tests/files/user-modules/three.module similarity index 100% rename from tests/files/user-modules/three rename to tests/files/user-modules/three.module diff --git a/tests/test-modules.c b/tests/test-modules.c index 1debed0..f755298 100644 --- a/tests/test-modules.c +++ b/tests/test-modules.c @@ -142,7 +142,7 @@ test_disable (CuTest *tc) */ modules = initialize_and_get_modules (tc); - CuAssertTrue (tc, lookup_module_with_name (tc, modules, "two") != NULL); + CuAssertTrue (tc, lookup_module_with_name (tc, modules, "two.badname") != NULL); finalize_and_free_modules (tc, modules); /* @@ -155,7 +155,7 @@ test_disable (CuTest *tc) p11_kit_set_progname ("test-disable"); modules = initialize_and_get_modules (tc); - CuAssertTrue (tc, lookup_module_with_name (tc, modules, "two") == NULL); + CuAssertTrue (tc, lookup_module_with_name (tc, modules, "two.badname") == NULL); finalize_and_free_modules (tc, modules); p11_kit_set_progname (NULL); -- 1.7.10.4