[pacman-dev] [PATCH 1/2] support ALPM_SIG_USE_DEFAULT for file siglevels

newer
[pacman-dev] [PATCH] alpm_handle:...

Andrew Gregory

28 Oct 2013 28 Oct '13

1:58 p.m.

This brings file siglevels in line with how db siglevels are handled. Signed-off-by: Andrew Gregory --- As an aside, why is alpm storing these values at all? They're only used by the frontend. lib/libalpm/handle.c | 12 ++++++++++-- src/pacman/conf.c | 4 +--- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/lib/libalpm/handle.c b/lib/libalpm/handle.c index 53c86c5..ea033b2 100644 --- a/lib/libalpm/handle.c +++ b/lib/libalpm/handle.c @@ -650,7 +650,11 @@ int SYMEXPORT alpm_option_set_local_file_siglevel(alpm_handle_t *handle, alpm_siglevel_t SYMEXPORT alpm_option_get_local_file_siglevel(alpm_handle_t *handle) { CHECK_HANDLE(handle, return -1); - return handle->localfilesiglevel; + if(handle->localfilesiglevel & ALPM_SIG_USE_DEFAULT) { + return handle->siglevel; + } else { + return handle->localfilesiglevel; + } } int SYMEXPORT alpm_option_set_remote_file_siglevel(alpm_handle_t *handle, @@ -670,7 +674,11 @@ int SYMEXPORT alpm_option_set_remote_file_siglevel(alpm_handle_t *handle, alpm_siglevel_t SYMEXPORT alpm_option_get_remote_file_siglevel(alpm_handle_t *handle) { CHECK_HANDLE(handle, return -1); - return handle->remotefilesiglevel; + if(handle->remotefilesiglevel & ALPM_SIG_USE_DEFAULT) { + return handle->siglevel; + } else { + return handle->remotefilesiglevel; + } } /* vim: set ts=2 sw=2 noet: */ diff --git a/src/pacman/conf.c b/src/pacman/conf.c index cd357ab..231fe2a 100644 --- a/src/pacman/conf.c +++ b/src/pacman/conf.c @@ -405,9 +405,7 @@ static int process_siglevel(alpm_list_t *values, alpm_siglevel_t *storage, static void merge_siglevel(alpm_siglevel_t *base, alpm_siglevel_t *over) { alpm_siglevel_t level = *over; - if(level & ALPM_SIG_USE_DEFAULT) { - level = *base; - } else { + if(!(level & ALPM_SIG_USE_DEFAULT)) { if(!(level & ALPM_SIG_PACKAGE_SET)) { level |= *base & ALPM_SIG_PACKAGE; level |= *base & ALPM_SIG_PACKAGE_OPTIONAL; -- 1.8.4.1

Show replies by thread

Andrew Gregory

28 Oct 28 Oct

1:58 p.m.

New subject: [pacman-dev] [PATCH 2/2] copy file siglevels from SigLevel when first seen

Both repo-specific siglevels and file siglevels used the default siglevel as their base. Previously, repo siglevels inherited when the repo was parsed, but file siglevels inherited after config parsing was complete. Having both options inherit from the default when they are first parsed is more intuitive and reduces parser complexity. Signed-off-by: Andrew Gregory --- This may change how some existing config files are parsed, but I doubt anybody is purposefully using the old behavior. lib/libalpm/alpm.h | 3 --- lib/libalpm/be_sync.c | 2 +- src/pacman/conf.c | 36 ++++++------------------------------ 3 files changed, 7 insertions(+), 34 deletions(-) diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h index b049007..034b155 100644 --- a/lib/libalpm/alpm.h +++ b/lib/libalpm/alpm.h @@ -115,9 +115,6 @@ typedef enum _alpm_siglevel_t { ALPM_SIG_DATABASE_MARGINAL_OK = (1 << 12), ALPM_SIG_DATABASE_UNKNOWN_OK = (1 << 13), - ALPM_SIG_PACKAGE_SET = (1 << 27), - ALPM_SIG_PACKAGE_TRUST_SET = (1 << 28), - ALPM_SIG_USE_DEFAULT = (1 << 31) } alpm_siglevel_t; diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c index 123d953..a4d6aff 100644 --- a/lib/libalpm/be_sync.c +++ b/lib/libalpm/be_sync.c @@ -694,7 +694,7 @@ alpm_db_t *_alpm_db_register_sync(alpm_handle_t *handle, const char *treename, _alpm_log(handle, ALPM_LOG_DEBUG, "registering sync database '%s'\n", treename); #ifndef HAVE_LIBGPGME - if((level &= ~ALPM_SIG_PACKAGE_SET) != 0 && level != ALPM_SIG_USE_DEFAULT) { + if(level != 0 && level != ALPM_SIG_USE_DEFAULT) { RET_ERR(handle, ALPM_ERR_WRONG_ARGS, NULL); } #endif diff --git a/src/pacman/conf.c b/src/pacman/conf.c index 231fe2a..387ade4 100644 --- a/src/pacman/conf.c +++ b/src/pacman/conf.c @@ -328,7 +328,6 @@ static int process_siglevel(alpm_list_t *values, alpm_siglevel_t *storage, if(strcmp(value, "Never") == 0) { if(package) { level &= ~ALPM_SIG_PACKAGE; - level |= ALPM_SIG_PACKAGE_SET; } if(database) { level &= ~ALPM_SIG_DATABASE; @@ -337,7 +336,6 @@ static int process_siglevel(alpm_list_t *values, alpm_siglevel_t *storage, if(package) { level |= ALPM_SIG_PACKAGE; level |= ALPM_SIG_PACKAGE_OPTIONAL; - level |= ALPM_SIG_PACKAGE_SET; } if(database) { level |= ALPM_SIG_DATABASE; @@ -347,7 +345,6 @@ static int process_siglevel(alpm_list_t *values, alpm_siglevel_t *storage, if(package) { level |= ALPM_SIG_PACKAGE; level &= ~ALPM_SIG_PACKAGE_OPTIONAL; - level |= ALPM_SIG_PACKAGE_SET; } if(database) { level |= ALPM_SIG_DATABASE; @@ -357,7 +354,6 @@ static int process_siglevel(alpm_list_t *values, alpm_siglevel_t *storage, if(package) { level &= ~ALPM_SIG_PACKAGE_MARGINAL_OK; level &= ~ALPM_SIG_PACKAGE_UNKNOWN_OK; - level |= ALPM_SIG_PACKAGE_TRUST_SET; } if(database) { level &= ~ALPM_SIG_DATABASE_MARGINAL_OK; @@ -367,7 +363,6 @@ static int process_siglevel(alpm_list_t *values, alpm_siglevel_t *storage, if(package) { level |= ALPM_SIG_PACKAGE_MARGINAL_OK; level |= ALPM_SIG_PACKAGE_UNKNOWN_OK; - level |= ALPM_SIG_PACKAGE_TRUST_SET; } if(database) { level |= ALPM_SIG_DATABASE_MARGINAL_OK; @@ -397,28 +392,6 @@ static int process_siglevel(alpm_list_t *values, alpm_siglevel_t *storage, return ret; } -/** - * Merge the package entires of two signature verification levels. - * @param base initial siglevel - * @param over overridden siglevel, derived value is stored here - */ -static void merge_siglevel(alpm_siglevel_t *base, alpm_siglevel_t *over) -{ - alpm_siglevel_t level = *over; - if(!(level & ALPM_SIG_USE_DEFAULT)) { - if(!(level & ALPM_SIG_PACKAGE_SET)) { - level |= *base & ALPM_SIG_PACKAGE; - level |= *base & ALPM_SIG_PACKAGE_OPTIONAL; - } - if(!(level & ALPM_SIG_PACKAGE_TRUST_SET)) { - level |= *base & ALPM_SIG_PACKAGE_MARGINAL_OK; - level |= *base & ALPM_SIG_PACKAGE_UNKNOWN_OK; - } - } - - *over = level; -} - static int process_cleanmethods(alpm_list_t *values, const char *file, int linenum) { @@ -572,6 +545,9 @@ static int _parse_options(const char *key, char *value, FREELIST(values); } else if(strcmp(key, "LocalFileSigLevel") == 0) { alpm_list_t *values = NULL; + if(config->localfilesiglevel == ALPM_SIG_USE_DEFAULT) { + config->localfilesiglevel = config->siglevel; + } setrepeatingoption(value, "LocalFileSigLevel", &values); if(process_siglevel(values, &config->localfilesiglevel, file, linenum)) { FREELIST(values); @@ -580,6 +556,9 @@ static int _parse_options(const char *key, char *value, FREELIST(values); } else if(strcmp(key, "RemoteFileSigLevel") == 0) { alpm_list_t *values = NULL; + if(config->remotefilesiglevel == ALPM_SIG_USE_DEFAULT) { + config->remotefilesiglevel = config->siglevel; + } setrepeatingoption(value, "RemoteFileSigLevel", &values); if(process_siglevel(values, &config->remotefilesiglevel, file, linenum)) { FREELIST(values); @@ -707,9 +686,6 @@ static int setup_libalpm(void) } alpm_option_set_default_siglevel(handle, config->siglevel); - - merge_siglevel(&config->siglevel, &config->localfilesiglevel); - merge_siglevel(&config->siglevel, &config->remotefilesiglevel); alpm_option_set_local_file_siglevel(handle, config->localfilesiglevel); alpm_option_set_remote_file_siglevel(handle, config->remotefilesiglevel); -- 1.8.4.1

Allan McRae

31 Oct 31 Oct

6:13 a.m.

New subject: [pacman-dev] [PATCH 2/2] copy file siglevels from SigLevel when first seen

On 28/10/13 23:58, Andrew Gregory wrote:

...

Both repo-specific siglevels and file siglevels used the default siglevel as their base. Previously, repo siglevels inherited when the repo was parsed, but file siglevels inherited after config parsing was complete. Having both options inherit from the default when they are first parsed is more intuitive and reduces parser complexity.

Signed-off-by: Andrew Gregory ---

This may change how some existing config files are parsed, but I doubt anybody is purposefully using the old behavior.

Can you provide details of the old behaviour that is "broken" by this?

...

lib/libalpm/alpm.h | 3 --- lib/libalpm/be_sync.c | 2 +- src/pacman/conf.c | 36 ++++++------------------------------ 3 files changed, 7 insertions(+), 34 deletions(-)

diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h index b049007..034b155 100644 --- a/lib/libalpm/alpm.h +++ b/lib/libalpm/alpm.h @@ -115,9 +115,6 @@ typedef enum _alpm_siglevel_t { ALPM_SIG_DATABASE_MARGINAL_OK = (1 << 12), ALPM_SIG_DATABASE_UNKNOWN_OK = (1 << 13),

- ALPM_SIG_PACKAGE_SET = (1 << 27), - ALPM_SIG_PACKAGE_TRUST_SET = (1 << 28), - ALPM_SIG_USE_DEFAULT = (1 << 31) } alpm_siglevel_t;

diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c index 123d953..a4d6aff 100644 --- a/lib/libalpm/be_sync.c +++ b/lib/libalpm/be_sync.c @@ -694,7 +694,7 @@ alpm_db_t *_alpm_db_register_sync(alpm_handle_t *handle, const char *treename, _alpm_log(handle, ALPM_LOG_DEBUG, "registering sync database '%s'\n", treename);

#ifndef HAVE_LIBGPGME - if((level &= ~ALPM_SIG_PACKAGE_SET) != 0 && level != ALPM_SIG_USE_DEFAULT) { + if(level != 0 && level != ALPM_SIG_USE_DEFAULT) { RET_ERR(handle, ALPM_ERR_WRONG_ARGS, NULL); } #endif diff --git a/src/pacman/conf.c b/src/pacman/conf.c index 231fe2a..387ade4 100644 --- a/src/pacman/conf.c +++ b/src/pacman/conf.c @@ -328,7 +328,6 @@ static int process_siglevel(alpm_list_t *values, alpm_siglevel_t *storage, if(strcmp(value, "Never") == 0) { if(package) { level &= ~ALPM_SIG_PACKAGE; - level |= ALPM_SIG_PACKAGE_SET; } if(database) { level &= ~ALPM_SIG_DATABASE; @@ -337,7 +336,6 @@ static int process_siglevel(alpm_list_t *values, alpm_siglevel_t *storage, if(package) { level |= ALPM_SIG_PACKAGE; level |= ALPM_SIG_PACKAGE_OPTIONAL; - level |= ALPM_SIG_PACKAGE_SET; } if(database) { level |= ALPM_SIG_DATABASE; @@ -347,7 +345,6 @@ static int process_siglevel(alpm_list_t *values, alpm_siglevel_t *storage, if(package) { level |= ALPM_SIG_PACKAGE; level &= ~ALPM_SIG_PACKAGE_OPTIONAL; - level |= ALPM_SIG_PACKAGE_SET; } if(database) { level |= ALPM_SIG_DATABASE; @@ -357,7 +354,6 @@ static int process_siglevel(alpm_list_t *values, alpm_siglevel_t *storage, if(package) { level &= ~ALPM_SIG_PACKAGE_MARGINAL_OK; level &= ~ALPM_SIG_PACKAGE_UNKNOWN_OK; - level |= ALPM_SIG_PACKAGE_TRUST_SET; } if(database) { level &= ~ALPM_SIG_DATABASE_MARGINAL_OK; @@ -367,7 +363,6 @@ static int process_siglevel(alpm_list_t *values, alpm_siglevel_t *storage, if(package) { level |= ALPM_SIG_PACKAGE_MARGINAL_OK; level |= ALPM_SIG_PACKAGE_UNKNOWN_OK; - level |= ALPM_SIG_PACKAGE_TRUST_SET; } if(database) { level |= ALPM_SIG_DATABASE_MARGINAL_OK; @@ -397,28 +392,6 @@ static int process_siglevel(alpm_list_t *values, alpm_siglevel_t *storage, return ret; }

-/** - * Merge the package entires of two signature verification levels. - * @param base initial siglevel - * @param over overridden siglevel, derived value is stored here - */ -static void merge_siglevel(alpm_siglevel_t *base, alpm_siglevel_t *over) -{ - alpm_siglevel_t level = *over; - if(!(level & ALPM_SIG_USE_DEFAULT)) { - if(!(level & ALPM_SIG_PACKAGE_SET)) { - level |= *base & ALPM_SIG_PACKAGE; - level |= *base & ALPM_SIG_PACKAGE_OPTIONAL; - } - if(!(level & ALPM_SIG_PACKAGE_TRUST_SET)) { - level |= *base & ALPM_SIG_PACKAGE_MARGINAL_OK; - level |= *base & ALPM_SIG_PACKAGE_UNKNOWN_OK; - } - } - - *over = level; -} - static int process_cleanmethods(alpm_list_t *values, const char *file, int linenum) { @@ -572,6 +545,9 @@ static int _parse_options(const char *key, char *value, FREELIST(values); } else if(strcmp(key, "LocalFileSigLevel") == 0) { alpm_list_t *values = NULL; + if(config->localfilesiglevel == ALPM_SIG_USE_DEFAULT) { + config->localfilesiglevel = config->siglevel; + } setrepeatingoption(value, "LocalFileSigLevel", &values); if(process_siglevel(values, &config->localfilesiglevel, file, linenum)) { FREELIST(values); @@ -580,6 +556,9 @@ static int _parse_options(const char *key, char *value, FREELIST(values); } else if(strcmp(key, "RemoteFileSigLevel") == 0) { alpm_list_t *values = NULL; + if(config->remotefilesiglevel == ALPM_SIG_USE_DEFAULT) { + config->remotefilesiglevel = config->siglevel; + } setrepeatingoption(value, "RemoteFileSigLevel", &values); if(process_siglevel(values, &config->remotefilesiglevel, file, linenum)) { FREELIST(values); @@ -707,9 +686,6 @@ static int setup_libalpm(void) }

alpm_option_set_default_siglevel(handle, config->siglevel); - - merge_siglevel(&config->siglevel, &config->localfilesiglevel); - merge_siglevel(&config->siglevel, &config->remotefilesiglevel); alpm_option_set_local_file_siglevel(handle, config->localfilesiglevel); alpm_option_set_remote_file_siglevel(handle, config->remotefilesiglevel);

Allan McRae

6:18 a.m.

New subject: [pacman-dev] [PATCH 1/2] support ALPM_SIG_USE_DEFAULT for file siglevels

On 28/10/13 23:58, Andrew Gregory wrote:

...

This brings file siglevels in line with how db siglevels are handled.

Signed-off-by: Andrew Gregory ---

As an aside, why is alpm storing these values at all? They're only used by the frontend.

Continuing your aside... aren't these stored in the backend so it can use them for signature verification? Or am I not understanding your question. A

Andrew Gregory

11:35 a.m.

New subject: [pacman-dev] [PATCH 1/2] support ALPM_SIG_USE_DEFAULT for file siglevels

On 10/31/13 at 04:18pm, Allan McRae wrote:

...

On 28/10/13 23:58, Andrew Gregory wrote:

...
This brings file siglevels in line with how db siglevels are handled.

Signed-off-by: Andrew Gregory ---

As an aside, why is alpm storing these values at all? They're only used by the frontend.

Continuing your aside... aren't these stored in the backend so it can use them for signature verification? Or am I not understanding your question.

A

alpm never uses the localfilesiglevel or remotefilesiglevel options directly. It only stores them for pacman, which picks the appropriate one and passes it to alpm_pkg_load in upgrade.c. It seems like it would make more sense to just store them in the frontend. apg

Allan McRae

11:38 a.m.

New subject: [pacman-dev] [PATCH 1/2] support ALPM_SIG_USE_DEFAULT for file siglevels

On 31/10/13 21:35, Andrew Gregory wrote:

...

On 10/31/13 at 04:18pm, Allan McRae wrote:

...
On 28/10/13 23:58, Andrew Gregory wrote:

...
This brings file siglevels in line with how db siglevels are handled.

Signed-off-by: Andrew Gregory ---

As an aside, why is alpm storing these values at all? They're only used by the frontend.

Continuing your aside... aren't these stored in the backend so it can use them for signature verification? Or am I not understanding your question.

A

alpm never uses the localfilesiglevel or remotefilesiglevel options directly. It only stores them for pacman, which picks the appropriate one and passes it to alpm_pkg_load in upgrade.c. It seems like it would make more sense to just store them in the frontend.

Hrm... I think the frontend should not even need to worry about that. We could adjust alpm_pkgfrom_t to distiguish between remote and local files and then just have libalpm select the appropriate value in alpm_pkg_load rather than the frontend having to provide it. A

Allan McRae

11:43 a.m.

New subject: [pacman-dev] [PATCH 2/2] copy file siglevels from SigLevel when first seen

On 31/10/13 21:45, Andrew Gregory wrote:

...

On 10/31/13 at 04:13pm, Allan McRae wrote:

...
On 28/10/13 23:58, Andrew Gregory wrote:

...
Both repo-specific siglevels and file siglevels used the default siglevel as their base. Previously, repo siglevels inherited when the repo was parsed, but file siglevels inherited after config parsing was complete. Having both options inherit from the default when they are first parsed is more intuitive and reduces parser complexity.

Signed-off-by: Andrew Gregory ---

This may change how some existing config files are parsed, but I doubt anybody is purposefully using the old behavior.

Can you provide details of the old behaviour that is "broken" by this?

Sure, here's an example:

SigLevel = Required TrustedOnly RemoteFileSigLevel = Optional SigLevel = TrustAll

Old behavior: RemoteFileSigLevel's final value is Optional TrustAll. New behavior: RemoteFileSigLevel's final value is Optional TrustedOnly.

I'm happy to break that. Perhaps we should consider SigLevel being specified twice in a pacman.conf as an error. It does not seem like a good idea to allow that as people would just see the first one when scanning their pacman.conf file. A

Andrew Gregory

11:45 a.m.

New subject: [pacman-dev] [PATCH 2/2] copy file siglevels from SigLevel when first seen

On 10/31/13 at 04:13pm, Allan McRae wrote:

...

On 28/10/13 23:58, Andrew Gregory wrote:

...
Both repo-specific siglevels and file siglevels used the default siglevel as their base. Previously, repo siglevels inherited when the repo was parsed, but file siglevels inherited after config parsing was complete. Having both options inherit from the default when they are first parsed is more intuitive and reduces parser complexity.

Signed-off-by: Andrew Gregory ---

This may change how some existing config files are parsed, but I doubt anybody is purposefully using the old behavior.

Can you provide details of the old behaviour that is "broken" by this?

Sure, here's an example: SigLevel = Required TrustedOnly RemoteFileSigLevel = Optional SigLevel = TrustAll Old behavior: RemoteFileSigLevel's final value is Optional TrustAll. New behavior: RemoteFileSigLevel's final value is Optional TrustedOnly. apg

Allan McRae

1 Nov 1 Nov

4:25 a.m.

New subject: [pacman-dev] [PATCH 2/2] copy file siglevels from SigLevel when first seen

On 31/10/13 21:43, Allan McRae wrote:

...

On 31/10/13 21:45, Andrew Gregory wrote:

...
On 10/31/13 at 04:13pm, Allan McRae wrote:

...
On 28/10/13 23:58, Andrew Gregory wrote:

...
Both repo-specific siglevels and file siglevels used the default siglevel as their base. Previously, repo siglevels inherited when the repo was parsed, but file siglevels inherited after config parsing was complete. Having both options inherit from the default when they are first parsed is more intuitive and reduces parser complexity.

Signed-off-by: Andrew Gregory ---

This may change how some existing config files are parsed, but I doubt anybody is purposefully using the old behavior.

Can you provide details of the old behaviour that is "broken" by this?

Sure, here's an example:

SigLevel = Required TrustedOnly RemoteFileSigLevel = Optional SigLevel = TrustAll

Old behavior: RemoteFileSigLevel's final value is Optional TrustAll. New behavior: RemoteFileSigLevel's final value is Optional TrustedOnly.

I'm happy to break that.

Perhaps we should consider SigLevel being specified twice in a pacman.conf as an error. It does not seem like a good idea to allow that as people would just see the first one when scanning their pacman.conf file.

And here I remembered why I did that all this weirdness: LocalFileSigLevel = TrustedOnly SigLevel = Required DatabaseOptional TrustedOnly Old behaviour: LocalFileSigLevel = Required New behaviour: LocalFileSigLevel = Optional or: SigLevel = Required DatabaseOptional TrustedOnly LocalFileSigLevel = TrustedOnly Old behaviour: LocalFileSigLevel = Required New behaviour: LocalFileSigLevel = Required So now order of values in the pacman.conf file matters. The documentation says LocalFileSigLevel uses SigLevel as the default. The question is how is that interpreted. The old way is that SigLevel was the base that LocalFileSigLevel built upon. The new way is that LocalFileSigLevel takes value of SigLevel only if it is unspecified in the pacman.conf. I really do not like the order options are provided in the pacman.conf mattering. We could simplify this by saying that LocalFileSigLevel defaults to SigLevel when it is unset (which is probably how documentation in pacman.conf.5 is interpreted anyway...) Allan

3841

Age (days ago)

3845

Last active (days ago)

List overview

Download

8 comments

2 participants

participants (2)

Allan McRae
Andrew Gregory

[pacman-dev] [PATCH 1/2] support ALPM_SIG_USE_DEFAULT for file siglevels

tags

participants (2)