[pacman-dev] Implement SigLevel config parsing, v2.0
These revised patches make some fixes based on Dave's suggestions and also fix some problems I noticed after submitting the original patches. I don't think Optional is actually the default if no SigLevel is specified, but it's probably a good default going forward. -Kerrick Staley
The ALPM_SIG_PACKAGE_HASH_OK field indicates that secure hashes are to be acceptable as signatures. Also, having ALPM_SIG_USE_DEFAULT = (1 << 31) will cause alpm_siglevel_t to be treated as an unsigned value on machines where int is 32 bits, meaning negative numbers cannot be returned on error conditions. So, make it (1 << 30). Still, there's no guarantee that signed semantics will be used unless one of the enum values is negative. So, supply ALPM_SIG_ERROR = -1 as a sort of dummy value (it can be used in code, but "-1" is probably just as meaningful and is more consistent with conventions used throughout the rest of the library). Signed-off-by: Kerrick Staley <mail@kerrickstaley.com> --- lib/libalpm/alpm.h | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h index 6e1e4bc..ecd9173 100644 --- a/lib/libalpm/alpm.h +++ b/lib/libalpm/alpm.h @@ -94,13 +94,14 @@ typedef enum _alpm_siglevel_t { ALPM_SIG_PACKAGE_OPTIONAL = (1 << 1), ALPM_SIG_PACKAGE_MARGINAL_OK = (1 << 2), ALPM_SIG_PACKAGE_UNKNOWN_OK = (1 << 3), + ALPM_SIG_PACKAGE_HASH_OK = (1 << 4), ALPM_SIG_DATABASE = (1 << 10), ALPM_SIG_DATABASE_OPTIONAL = (1 << 11), ALPM_SIG_DATABASE_MARGINAL_OK = (1 << 12), ALPM_SIG_DATABASE_UNKNOWN_OK = (1 << 13), - ALPM_SIG_USE_DEFAULT = (1 << 31) + ALPM_SIG_USE_DEFAULT = (1 << 30) } alpm_siglevel_t; /** -- 1.7.6
And... I didn't actually hit save, so this is missing the ALPM_SIG_ERROR part. Here's the fixed version. Revise siglevel_t, adding PACKAGE_HASH_OK field The ALPM_SIG_PACKAGE_HASH_OK field indicates that secure hashes are to be acceptable as signatures. Also, having ALPM_SIG_USE_DEFAULT = (1 << 31) will cause alpm_siglevel_t to be treated as an unsigned value on machines where int is 32 bits, meaning negative numbers cannot be returned on error conditions. So, make it (1 << 30). Still, there's no guarantee that signed semantics will be used unless one of the enum values is negative. So, supply ALPM_SIG_ERROR = -1 as a sort of dummy value (it can be used in code, but "-1" is probably just as meaningful and is more consistent with conventions used throughout the rest of the library). Signed-off-by: Kerrick Staley <mail@kerrickstaley.com> --- lib/libalpm/alpm.h | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h index 6e1e4bc..93ffa89 100644 --- a/lib/libalpm/alpm.h +++ b/lib/libalpm/alpm.h @@ -90,17 +90,19 @@ typedef enum _alpm_fileconflicttype_t { * PGP signature verification options */ typedef enum _alpm_siglevel_t { + ALPM_SIG_ERROR = -1, ALPM_SIG_PACKAGE = (1 << 0), ALPM_SIG_PACKAGE_OPTIONAL = (1 << 1), ALPM_SIG_PACKAGE_MARGINAL_OK = (1 << 2), ALPM_SIG_PACKAGE_UNKNOWN_OK = (1 << 3), + ALPM_SIG_PACKAGE_HASH_OK = (1 << 4), ALPM_SIG_DATABASE = (1 << 10), ALPM_SIG_DATABASE_OPTIONAL = (1 << 11), ALPM_SIG_DATABASE_MARGINAL_OK = (1 << 12), ALPM_SIG_DATABASE_UNKNOWN_OK = (1 << 13), - ALPM_SIG_USE_DEFAULT = (1 << 31) + ALPM_SIG_USE_DEFAULT = (1 << 30) } alpm_siglevel_t; /**
On 18/07/11 16:59, Kerrick Staley wrote:
And... I didn't actually hit save, so this is missing the ALPM_SIG_ERROR part. Here's the fixed version.
Revise siglevel_t, adding PACKAGE_HASH_OK field
The ALPM_SIG_PACKAGE_HASH_OK field indicates that secure hashes are to be acceptable as signatures.
I do not understand how is this a useful option. There is always a hash in the repo database assuming it is created using repo-add (md5sum gets used as a download check, and sha256sums are there but do nothing). So this is the same as setting signature checking as "Optional" or "None". Also, is md5sum is a secure hash? Allan
The SigLevel config option replaces the VerifySig option, and has similar semantics, but adds a set of advanced configuration options that correspond to the recently introduced alpm_siglevel_t fields. Signed-off-by: Kerrick Staley <mail@kerrickstaley.com> --- doc/pacman.conf.5.txt | 20 ++++++++++++++++++++ 1 files changed, 20 insertions(+), 0 deletions(-) diff --git a/doc/pacman.conf.5.txt b/doc/pacman.conf.5.txt index a28e00f..19cd6e3 100644 --- a/doc/pacman.conf.5.txt +++ b/doc/pacman.conf.5.txt @@ -156,6 +156,26 @@ Options packages are only cleaned if not installed locally and not present in any known sync database. +*SigLevel =* ...:: + If set to `Optional` (the default), signatures will be checked if present, + but unsigned databases/packages will also be allowed. Setting to `Required` + will cause signatures to be required on all packages and databases. `Never` + will prevent all signature checking. + Alternatively, you get more fine-grained control by combining some of + the options described below. + `PackageRequired` works like `Required`, but only causes checks to + be performed on packages. `PackageOptional` works like `Optional` + but also for packages only, and it can't be specified along with + `PackageRequired`. `PackageMarginal` causes signatures from marginally + trusted keys to be accepted on packages. `PackageUnknown` causes + signatures made with an unknown key to be accepted on packages. All + of these `PackageX` options have corresponding `DatabaseX` + options. Lastly, `PackageHash` causes a secure hash in a database to + be accepted as a package signature. It probably should be combined with + `DatabaseRequired`. This `PackageHash`+`DatabaseRequired` combination is + reasonably secure and is a good compromise when signing every package is + too difficult for a distribution's maintainers. + *UseSyslog*:: Log action messages through syslog(). This will insert log entries into +{localstatedir}/log/messages+ or equivalent. -- 1.7.6
On 18/07/11 16:35, Kerrick Staley wrote:
The SigLevel config option replaces the VerifySig option, and has similar semantics, but adds a set of advanced configuration options that correspond to the recently introduced alpm_siglevel_t fields.
Signed-off-by: Kerrick Staley<mail@kerrickstaley.com> --- doc/pacman.conf.5.txt | 20 ++++++++++++++++++++ 1 files changed, 20 insertions(+), 0 deletions(-)
diff --git a/doc/pacman.conf.5.txt b/doc/pacman.conf.5.txt index a28e00f..19cd6e3 100644 --- a/doc/pacman.conf.5.txt +++ b/doc/pacman.conf.5.txt @@ -156,6 +156,26 @@ Options packages are only cleaned if not installed locally and not present in any known sync database.
+*SigLevel =* ...::
I was fairly sure previous the discussion indicated we would go with CheckLevel for the name of this. Anyway, I think it would be good to get a complete list of options sorted before documenting and implementing... Currently it seems incomplete. Global options: Required (or Always?), Optional, Never - what about global controls for allowing marginal and unknown signatures. Also, we need to determine exactly how the suboptions will work. Do the PackageFoo options override global options? E.g. are these all equivalent (require database signatures but not package signatures)? CheckLevel = Required PackageOptional CheckLevel = Optional DatabaseRequired CheckLevel = DatabaseRequired PackageOptional Allan
Required, Optional, and Never aren't to be mixed with the other settings. The original version said this, but I thought the whole thing was getting long winded, so I reworded it in a way that I thought would make this implicit (guess not :). Required, Optional, and Never will be sufficient for like 90% of users (which is probably why they were the only options until recently), so I separated them out from the more "advanced" settings (many people won't [and shouldn't have to] know what e.g. marginally trusted signatures are). -Kerrick Staley On Jul 18, 2011 2:55 AM, "Allan McRae" <allan@archlinux.org> wrote:
On 18/07/11 16:35, Kerrick Staley wrote:
The SigLevel config option replaces the VerifySig option, and has similar semantics, but adds a set of advanced configuration options that correspond to the recently introduced alpm_siglevel_t fields.
Signed-off-by: Kerrick Staley<mail@kerrickstaley.com> --- doc/pacman.conf.5.txt | 20 ++++++++++++++++++++ 1 files changed, 20 insertions(+), 0 deletions(-)
diff --git a/doc/pacman.conf.5.txt b/doc/pacman.conf.5.txt index a28e00f..19cd6e3 100644 --- a/doc/pacman.conf.5.txt +++ b/doc/pacman.conf.5.txt @@ -156,6 +156,26 @@ Options packages are only cleaned if not installed locally and not present in any known sync database.
+*SigLevel =* ...::
I was fairly sure previous the discussion indicated we would go with CheckLevel for the name of this.
Anyway, I think it would be good to get a complete list of options sorted before documenting and implementing... Currently it seems incomplete.
Global options: Required (or Always?), Optional, Never - what about global controls for allowing marginal and unknown signatures.
Also, we need to determine exactly how the suboptions will work. Do the PackageFoo options override global options? E.g. are these all equivalent (require database signatures but not package signatures)?
CheckLevel = Required PackageOptional CheckLevel = Optional DatabaseRequired CheckLevel = DatabaseRequired PackageOptional
Allan
Add code to conf.c that parses the new SigLevel directive. Signed-off-by: Kerrick Staley <mail@kerrickstaley.com> --- src/pacman/conf.c | 97 +++++++++++++++++++++++++++++++++++++++++++--------- 1 files changed, 80 insertions(+), 17 deletions(-) diff --git a/src/pacman/conf.c b/src/pacman/conf.c index fac6da3..de5b463 100644 --- a/src/pacman/conf.c +++ b/src/pacman/conf.c @@ -222,20 +222,83 @@ int config_set_arch(const char *arch) return 0; } -static alpm_siglevel_t option_verifysig(const char *value) +/** + * Parse a signature verification level. Destroys the passed string. + * @param str the string listing the siglevel options + * @return -1 if value was empty, -2 if value contained unknown + * options, -3 if some of the options were contradictory, the siglevel + * value otherwise. + */ +static alpm_siglevel_t option_siglevel(char *str) { - alpm_siglevel_t level; - if(strcmp(value, "Always") == 0) { - level = ALPM_SIG_PACKAGE | ALPM_SIG_DATABASE; - } else if(strcmp(value, "Optional") == 0) { - level = ALPM_SIG_PACKAGE | ALPM_SIG_PACKAGE_OPTIONAL | - ALPM_SIG_DATABASE | ALPM_SIG_DATABASE_OPTIONAL; - } else if(strcmp(value, "Never") == 0) { - level = 0; - } else { + alpm_siglevel_t level = 0; + + char *strtok_state; + char *tok = strtok_r(str, " \t", &strtok_state); + + if (!tok) { return -1; } - pm_printf(ALPM_LOG_DEBUG, "config: VerifySig = %s (%d)\n", value, level); + + do { + if(strcmp(tok, "Never") == 0) { + level = level ? -3 : 0; + break; + } else if(strcmp(tok, "Optional") == 0) { + level = level ? -3 : ALPM_SIG_PACKAGE | ALPM_SIG_PACKAGE_OPTIONAL | + ALPM_SIG_DATABASE | ALPM_SIG_DATABASE_OPTIONAL; + break; + } else if(strcmp(tok, "Required") == 0) { + level = level ? -3 : ALPM_SIG_PACKAGE | ALPM_SIG_DATABASE; + break; + } else if(strcmp(tok, "PackageHash") == 0) { + if (level & ALPM_SIG_DATABASE_OPTIONAL) { + level = -3; + break; + } + level |= ALPM_SIG_PACKAGE_HASH_OK; + } else if(strcmp(tok, "PackageRequired") == 0) { + if (level & ALPM_SIG_PACKAGE_OPTIONAL) { + level = -3; + break; + } + level |= ALPM_SIG_PACKAGE; + } else if(strcmp(tok, "DatabaseRequired") == 0) { + if (level & ALPM_SIG_DATABASE_OPTIONAL) { + level = -3; + break; + } + level |= ALPM_SIG_DATABASE; + } else if(strcmp(tok, "PackageOptional") == 0) { + if (level & ALPM_SIG_PACKAGE) { + level = -3; + break; + } + level |= ALPM_SIG_PACKAGE_OPTIONAL; + } else if(strcmp(tok, "DatabaseOptional") == 0) { + if (level & ALPM_SIG_PACKAGE) { + level = -3; + break; + } + level |= ALPM_SIG_DATABASE_OPTIONAL; + } else if(strcmp(tok, "PackageAllowMarginal") == 0) { + level |= ALPM_SIG_PACKAGE_MARGINAL_OK; + } else if(strcmp(tok, "DatabaseAllowMarginal") == 0) { + level |= ALPM_SIG_DATABASE_MARGINAL_OK; + } else if(strcmp(tok, "PackageAllowUnknown") == 0) { + level |= ALPM_SIG_PACKAGE_UNKNOWN_OK; + } else if(strcmp(tok, "DatabaseAllowUnknown") == 0) { + level |= ALPM_SIG_DATABASE_UNKNOWN_OK; + } else { + level = -2; + break; + } + } while ((tok = strtok_r(NULL, " \t", &strtok_state))); + + if (level >= 0) { + pm_printf(ALPM_LOG_DEBUG, "config: SigLevel = %s (%d)\n", str, level); + } + return level; } @@ -359,9 +422,9 @@ static int _parse_options(const char *key, char *value, return 1; } FREELIST(methods); - } else if(strcmp(key, "VerifySig") == 0) { - alpm_siglevel_t level = option_verifysig(value); - if(level != -1) { + } else if(strcmp(key, "SigLevel") == 0) { + alpm_siglevel_t level = option_siglevel(value); + if(level >= 0) { config->siglevel = level; } else { pm_printf(ALPM_LOG_ERROR, @@ -726,9 +789,9 @@ static int _parseconfig(const char *file, struct section_t *section, goto cleanup; } section->servers = alpm_list_add(section->servers, strdup(value)); - } else if(strcmp(key, "VerifySig") == 0) { - alpm_siglevel_t level = option_verifysig(value); - if(level != -1) { + } else if(strcmp(key, "SigLevel") == 0) { + alpm_siglevel_t level = option_siglevel(value); + if(level >= 0) { section->siglevel = level; } else { pm_printf(ALPM_LOG_ERROR, -- 1.7.6
On Mon, Jul 18, 2011 at 1:35 AM, Kerrick Staley <mail@kerrickstaley.com> wrote:
Add code to conf.c that parses the new SigLevel directive.
Signed-off-by: Kerrick Staley <mail@kerrickstaley.com> --- src/pacman/conf.c | 97 +++++++++++++++++++++++++++++++++++++++++++--------- 1 files changed, 80 insertions(+), 17 deletions(-)
diff --git a/src/pacman/conf.c b/src/pacman/conf.c index fac6da3..de5b463 100644 --- a/src/pacman/conf.c +++ b/src/pacman/conf.c @@ -222,20 +222,83 @@ int config_set_arch(const char *arch) return 0; }
-static alpm_siglevel_t option_verifysig(const char *value) +/** + * Parse a signature verification level. Destroys the passed string. + * @param str the string listing the siglevel options + * @return -1 if value was empty, -2 if value contained unknown Shouldn't 0 be empty? Considering this method wouldn't even be called if you never had a LHS option key of SigLevel.
+ * options, -3 if some of the options were contradictory, the siglevel + * value otherwise. + */ +static alpm_siglevel_t option_siglevel(char *str) { - alpm_siglevel_t level; - if(strcmp(value, "Always") == 0) { - level = ALPM_SIG_PACKAGE | ALPM_SIG_DATABASE; - } else if(strcmp(value, "Optional") == 0) { - level = ALPM_SIG_PACKAGE | ALPM_SIG_PACKAGE_OPTIONAL | - ALPM_SIG_DATABASE | ALPM_SIG_DATABASE_OPTIONAL; - } else if(strcmp(value, "Never") == 0) { - level = 0; - } else { + alpm_siglevel_t level = 0; + + char *strtok_state; + char *tok = strtok_r(str, " \t", &strtok_state); If we're taking tabs here, then we should take them everywhere, and it should be done in a separate patch. See the strchr() call in setrepeatingoption().
I'd rather not reinvent the tokenization and parsing wheel here. Please use setrepeatingoption() and refactor this method to be more like process_cleanmethods().
+ + if (!tok) { return -1; } - pm_printf(ALPM_LOG_DEBUG, "config: VerifySig = %s (%d)\n", value, level); + + do { + if(strcmp(tok, "Never") == 0) { + level = level ? -3 : 0; + break; Why in the heck are you breaking out of the loop? This means SigLevel = Never asdlfs;aiophgewioyh432i89 fkhg qiew5 o089a wl; is now valid, no?
+ } else if(strcmp(tok, "Optional") == 0) { + level = level ? -3 : ALPM_SIG_PACKAGE | ALPM_SIG_PACKAGE_OPTIONAL | + ALPM_SIG_DATABASE | ALPM_SIG_DATABASE_OPTIONAL; + break; + } else if(strcmp(tok, "Required") == 0) { + level = level ? -3 : ALPM_SIG_PACKAGE | ALPM_SIG_DATABASE; + break; I disagree with every one of these breaks. Always finish parsing what you start.
+ } else if(strcmp(tok, "PackageHash") == 0) { + if (level & ALPM_SIG_DATABASE_OPTIONAL) { + level = -3; + break; + } + level |= ALPM_SIG_PACKAGE_HASH_OK; Kill this block, out of scope here.
+ } else if(strcmp(tok, "PackageRequired") == 0) { + if (level & ALPM_SIG_PACKAGE_OPTIONAL) { + level = -3; + break; Also disagreeing with these breaks. If we are going to have the non-package and non-database options, allowing one to override makes perfect sense to me and should not be rejected. SigLevel = Always PackageOptional should work fine. I see it is getting rejected below. You realize PACKAGE and PACKAGE_OPTIONAL are not mutually exclusive, right?
+ } + level |= ALPM_SIG_PACKAGE; + } else if(strcmp(tok, "DatabaseRequired") == 0) { + if (level & ALPM_SIG_DATABASE_OPTIONAL) { + level = -3; + break; + } + level |= ALPM_SIG_DATABASE; + } else if(strcmp(tok, "PackageOptional") == 0) { + if (level & ALPM_SIG_PACKAGE) { + level = -3; + break; + } + level |= ALPM_SIG_PACKAGE_OPTIONAL; + } else if(strcmp(tok, "DatabaseOptional") == 0) { + if (level & ALPM_SIG_PACKAGE) { + level = -3; + break; + } + level |= ALPM_SIG_DATABASE_OPTIONAL; + } else if(strcmp(tok, "PackageAllowMarginal") == 0) { + level |= ALPM_SIG_PACKAGE_MARGINAL_OK; + } else if(strcmp(tok, "DatabaseAllowMarginal") == 0) { + level |= ALPM_SIG_DATABASE_MARGINAL_OK; + } else if(strcmp(tok, "PackageAllowUnknown") == 0) { + level |= ALPM_SIG_PACKAGE_UNKNOWN_OK; + } else if(strcmp(tok, "DatabaseAllowUnknown") == 0) { + level |= ALPM_SIG_DATABASE_UNKNOWN_OK; + } else { + level = -2; + break; + } + } while ((tok = strtok_r(NULL, " \t", &strtok_state))); + + if (level >= 0) { + pm_printf(ALPM_LOG_DEBUG, "config: SigLevel = %s (%d)\n", str, level); + } No need to restrict this printout as it is a debug message.
+ return level; }
@@ -359,9 +422,9 @@ static int _parse_options(const char *key, char *value, return 1; } FREELIST(methods); - } else if(strcmp(key, "VerifySig") == 0) { - alpm_siglevel_t level = option_verifysig(value); - if(level != -1) { + } else if(strcmp(key, "SigLevel") == 0) { + alpm_siglevel_t level = option_siglevel(value); + if(level >= 0) { config->siglevel = level; } else { pm_printf(ALPM_LOG_ERROR, This error message won't work at all as you just finished trashing the contents of value.
@@ -726,9 +789,9 @@ static int _parseconfig(const char *file, struct section_t *section, goto cleanup; } section->servers = alpm_list_add(section->servers, strdup(value)); - } else if(strcmp(key, "VerifySig") == 0) { - alpm_siglevel_t level = option_verifysig(value); - if(level != -1) { + } else if(strcmp(key, "SigLevel") == 0) { + alpm_siglevel_t level = option_siglevel(value); + if(level >= 0) { This now rejects a 'VerifySig = ' line, which perhaps is odd, but I'm not sure is correct (and I don't think we did it before?). One would assume this would use the default value or something.
section->siglevel = level; } else { pm_printf(ALPM_LOG_ERROR, -- 1.7.6
Add code to conf.c that parses the new SigLevel directive. An overwhelming number of options are presented, but most users will still be fine with the Never/Optional/Required trio. More advanced users can combine these or any of the other options on a 'SigLevel = ' line, which is parsed in a left-to-right fashion and flags turned on and off accordingly. For example, all three of these will net the same config: SigLevel = Required PackageOptional SigLevel = Optional DatabaseRequired SigLevel = DatabaseRequired PackageOptional Additionally, database-specific lines assume you wish to start with any global default that has been set. For example, if any of the above lines were in the [options] section, something such as: SigLevel = PackageRequired PackageAllowMarginal Would continue to enforce required database signatures. Inspiration-by: Kerrick Staley <mail@kerrickstaley.com> Signed-off-by: Dan McGee <dan@archlinux.org> --- src/pacman/conf.c | 113 ++++++++++++++++++++++++++++++++++++++------------- test/pacman/util.py | 2 +- 2 files changed, 85 insertions(+), 30 deletions(-) diff --git a/src/pacman/conf.c b/src/pacman/conf.c index fac6da3..d654361 100644 --- a/src/pacman/conf.c +++ b/src/pacman/conf.c @@ -222,21 +222,80 @@ int config_set_arch(const char *arch) return 0; } -static alpm_siglevel_t option_verifysig(const char *value) +/** + * Parse a signature verification level line. + * @param values the list of parsed option values + * @return 0 on success, 1 on any parsing error + */ +static int process_siglevel(alpm_list_t *values, alpm_siglevel_t startval, + alpm_siglevel_t *storage) { - alpm_siglevel_t level; - if(strcmp(value, "Always") == 0) { - level = ALPM_SIG_PACKAGE | ALPM_SIG_DATABASE; - } else if(strcmp(value, "Optional") == 0) { - level = ALPM_SIG_PACKAGE | ALPM_SIG_PACKAGE_OPTIONAL | - ALPM_SIG_DATABASE | ALPM_SIG_DATABASE_OPTIONAL; - } else if(strcmp(value, "Never") == 0) { - level = 0; - } else { - return -1; + alpm_siglevel_t level = startval; + alpm_list_t *i; + + /* Collapse the option names into a single bitmasked value */ + for(i = values; i; i = alpm_list_next(i)) { + const char *value = i->data; + if(strcmp(value, "Never") == 0) { + level &= ~ALPM_SIG_PACKAGE; + level &= ~ALPM_SIG_DATABASE; + } else if(strcmp(value, "Optional") == 0) { + level |= ALPM_SIG_PACKAGE; + level |= ALPM_SIG_DATABASE; + level |= ALPM_SIG_PACKAGE_OPTIONAL; + level |= ALPM_SIG_DATABASE_OPTIONAL; + } else if(strcmp(value, "Required") == 0) { + level |= ALPM_SIG_PACKAGE; + level |= ALPM_SIG_DATABASE; + level &= ~ALPM_SIG_PACKAGE_OPTIONAL; + level &= ~ALPM_SIG_DATABASE_OPTIONAL; + } else if(strcmp(value, "TrustedOnly") == 0) { + level &= ~ALPM_SIG_PACKAGE_MARGINAL_OK; + level &= ~ALPM_SIG_DATABASE_MARGINAL_OK; + level &= ~ALPM_SIG_PACKAGE_UNKNOWN_OK; + level &= ~ALPM_SIG_DATABASE_UNKNOWN_OK; + } else if(strcmp(value, "AllowMarginal") == 0) { + level |= ALPM_SIG_PACKAGE_MARGINAL_OK; + level |= ALPM_SIG_DATABASE_MARGINAL_OK; + } else if(strcmp(value, "AllowUnknown") == 0) { + level |= ALPM_SIG_PACKAGE_UNKNOWN_OK; + level |= ALPM_SIG_DATABASE_UNKNOWN_OK; + } else if(strcmp(value, "PackageOptional") == 0) { + level |= ALPM_SIG_PACKAGE; + level |= ALPM_SIG_PACKAGE_OPTIONAL; + } else if(strcmp(value, "DatabaseOptional") == 0) { + level |= ALPM_SIG_DATABASE; + level |= ALPM_SIG_DATABASE_OPTIONAL; + } else if(strcmp(value, "PackageRequired") == 0) { + level |= ALPM_SIG_PACKAGE; + level &= ~ALPM_SIG_PACKAGE_OPTIONAL; + } else if(strcmp(value, "DatabaseRequired") == 0) { + level |= ALPM_SIG_DATABASE; + level &= ~ALPM_SIG_DATABASE_OPTIONAL; + } else if(strcmp(value, "PackageTrustedOnly") == 0) { + level &= ~ALPM_SIG_PACKAGE_MARGINAL_OK; + level &= ~ALPM_SIG_PACKAGE_UNKNOWN_OK; + } else if(strcmp(value, "DatabaseTrustedOnly") == 0) { + level &= ~ALPM_SIG_DATABASE_MARGINAL_OK; + level &= ~ALPM_SIG_DATABASE_UNKNOWN_OK; + } else if(strcmp(value, "PackageAllowMarginal") == 0) { + level |= ALPM_SIG_PACKAGE_MARGINAL_OK; + } else if(strcmp(value, "DatabaseAllowMarginal") == 0) { + level |= ALPM_SIG_DATABASE_MARGINAL_OK; + } else if(strcmp(value, "PackageAllowUnknown") == 0) { + level |= ALPM_SIG_PACKAGE_UNKNOWN_OK; + } else if(strcmp(value, "DatabaseAllowUnknown") == 0) { + level |= ALPM_SIG_DATABASE_UNKNOWN_OK; + } else { + pm_printf(ALPM_LOG_ERROR, _("invalid value for 'SigLevel' : '%s'\n"), + value); + return 1; + } + level &= ~ALPM_SIG_USE_DEFAULT; } - pm_printf(ALPM_LOG_DEBUG, "config: VerifySig = %s (%d)\n", value, level); - return level; + + *storage = level; + return 0; } static int process_cleanmethods(alpm_list_t *values) { @@ -359,16 +418,14 @@ static int _parse_options(const char *key, char *value, return 1; } FREELIST(methods); - } else if(strcmp(key, "VerifySig") == 0) { - alpm_siglevel_t level = option_verifysig(value); - if(level != -1) { - config->siglevel = level; - } else { - pm_printf(ALPM_LOG_ERROR, - _("config file %s, line %d: directive '%s' has invalid value '%s'\n"), - file, linenum, key, value); + } else if(strcmp(key, "SigLevel") == 0) { + alpm_list_t *values = NULL; + setrepeatingoption(value, "SigLevel", &values); + if(process_siglevel(values, ALPM_SIG_USE_DEFAULT, &(config->siglevel))) { + FREELIST(values); return 1; } + FREELIST(values); } else { pm_printf(ALPM_LOG_WARNING, _("config file %s, line %d: directive '%s' in section '%s' not recognized.\n"), @@ -726,17 +783,15 @@ static int _parseconfig(const char *file, struct section_t *section, goto cleanup; } section->servers = alpm_list_add(section->servers, strdup(value)); - } else if(strcmp(key, "VerifySig") == 0) { - alpm_siglevel_t level = option_verifysig(value); - if(level != -1) { - section->siglevel = level; - } else { - pm_printf(ALPM_LOG_ERROR, - _("config file %s, line %d: directive '%s' has invalid value '%s'\n"), - file, linenum, key, value); + } else if(strcmp(key, "SigLevel") == 0) { + alpm_list_t *values = NULL; + setrepeatingoption(value, "SigLevel", &values); + if(process_siglevel(values, config->siglevel, &(section->siglevel))) { + FREELIST(values); ret = 1; goto cleanup; } + FREELIST(values); } else { pm_printf(ALPM_LOG_WARNING, _("config file %s, line %d: directive '%s' in section '%s' not recognized.\n"), diff --git a/test/pacman/util.py b/test/pacman/util.py index dbe416f..60dea9e 100644 --- a/test/pacman/util.py +++ b/test/pacman/util.py @@ -118,7 +118,7 @@ def mkcfgfile(filename, root, option, db): if key != "local": value = db[key] data.append("[%s]\n" \ - "VerifySig = %s\n" \ + "SigLevel = %s\n" \ "Server = file://%s" \ % (value.treename, value.getverify(), \ os.path.join(root, SYNCREPO, value.treename))) -- 1.7.6
On 19/07/11 11:06, Dan McGee wrote:
Add code to conf.c that parses the new SigLevel directive. An overwhelming number of options are presented, but most users will still be fine with the Never/Optional/Required trio. More advanced users can combine these or any of the other options on a 'SigLevel = ' line, which is parsed in a left-to-right fashion and flags turned on and off accordingly. For example, all three of these will net the same config:
SigLevel = Required PackageOptional SigLevel = Optional DatabaseRequired SigLevel = DatabaseRequired PackageOptional
Additionally, database-specific lines assume you wish to start with any global default that has been set. For example, if any of the above lines were in the [options] section, something such as:
SigLevel = PackageRequired PackageAllowMarginal
Would continue to enforce required database signatures.
Inspiration-by: Kerrick Staley<mail@kerrickstaley.com> Signed-off-by: Dan McGee<dan@archlinux.org> ---
Signed-off-by: Allan Tested with the following in my pacman.conf [options] SigLevel = Optional (also with this commented out) [allanbrokeit] SigLevel = Required [kernel64] SigLevel = DatabaseRequired (Arch repos without SigLevel specified) Everything seems to work as expected. The [allanbrokeit] repo gives all sorts of failures without signatures and the [kernel64] repo failed when there was no repo signature but is happy about the lack of package signatures within the repo. Allan
Dan, Thanks for the feedback, especially regarding the code layout (your version is a lot cleaner) and the attempt to print the mutilated string. I do think that there are too many directives in your version: it'd be better to have a few directives that address the most likely cases, and then just provide one directive for each internal flag to allow advanced configuration. This also makes for a conciser description in the manpage. For the signed vs. unsigned issue, one solution that would work just as well (I think) would be to typedef int alpm_siglevel_t; and then use #defines for ALPM_SIG_* and have process_siglevel() return negative values on error conditions like before (ALPM_SIG_USE_DEFAULT would have to become 1 << 30). It kills the type safety though. Just an idea: what do you think? -Kerrick Staley
participants (4)
-
Allan McRae
-
Dan McGee
-
Dan McGee
-
Kerrick Staley