Re: [gentoo-portage-dev] [PATCH] repos.conf: add bool sync-openpgp-key-refresh option (bug 661518)

[Zac Medico <zmedico@gentoo.org>]

[-- Attachment #1.1: Type: text/plain, Size: 5687 bytes --]

On 6/12/20 6:48 PM, Brian Dolbec wrote:
> On Fri, 12 Jun 2020 16:51:51 -0700
> Zac Medico <zmedico@gentoo.org> wrote:
> 
>> Add a sync-openpgp-key-refresh option that makes it possible to
>> disable key refresh, which may be useful in cases when it is not
>> possible to refresh keys.
>>
>> Key refresh is enabled by default, and if it is disabled then
>> the SyncBase._refresh_keys method will output an ewarn message
>> like this when the --quiet option is not enabled:
>>
>>  * Key refresh is disabled via a repos.conf sync-openpgp-key-refresh
>>  * setting, and this is a security vulnerability because it prevents
>>  * detection of revoked keys!
>>
>> Bug: https://bugs.gentoo.org/661518
>> Signed-off-by: Zac Medico <zmedico@gentoo.org>
>> ---
>>  lib/portage/repository/config.py | 10 +++++++++-
>>  lib/portage/sync/syncbase.py     |  9 ++++++++-
>>  man/portage.5                    |  9 ++++++++-
>>  3 files changed, 25 insertions(+), 3 deletions(-)
>>
>> diff --git a/lib/portage/repository/config.py
>> b/lib/portage/repository/config.py index 50ab18026..6155c130a 100644
>> --- a/lib/portage/repository/config.py
>> +++ b/lib/portage/repository/config.py
>> @@ -1,4 +1,4 @@
>> -# Copyright 2010-2019 Gentoo Authors
>> +# Copyright 2010-2020 Gentoo Authors
>>  # Distributed under the terms of the GNU General Public License v2
>>  
>>  from __future__ import unicode_literals
>> @@ -113,6 +113,7 @@ class RepoConfig(object):
>>  		'sync_hooks_only_on_change',
>>  		'sync_openpgp_keyserver',
>>  		'sync_openpgp_key_path',
>> +		'sync_openpgp_key_refresh',
>>  		'sync_openpgp_key_refresh_retry_count',
>>  		'sync_openpgp_key_refresh_retry_delay_exp_base',
>>  		'sync_openpgp_key_refresh_retry_delay_max',
>> @@ -233,6 +234,9 @@ class RepoConfig(object):
>>  		self.sync_openpgp_key_path = repo_opts.get(
>>  			'sync-openpgp-key-path', None)
>>  
>> +		self.sync_openpgp_key_refresh = repo_opts.get(
>> +			'sync-openpgp-key-refresh', 'true').lower()
>> in ('true', 'yes') +
>>  		for k in ('sync_openpgp_key_refresh_retry_count',
>>  			'sync_openpgp_key_refresh_retry_delay_exp_base',
>>  			'sync_openpgp_key_refresh_retry_delay_max',
>> @@ -497,6 +501,8 @@ class RepoConfig(object):
>>  			repo_msg.append(indent + "location: " +
>> self.location) if not self.strict_misc_digests:
>>  			repo_msg.append(indent +
>> "strict-misc-digests: false")
>> +		if not self.sync_openpgp_key_refresh:
>> +			repo_msg.append(indent +
>> "sync-openpgp-key-refresh: no") if self.sync_type:
>>  			repo_msg.append(indent + "sync-type: " +
>> self.sync_type) if self.sync_umask:
>> @@ -609,6 +615,7 @@ class RepoConfigLoader(object):
>>  							'sync_hooks_only_on_change',
>>  							'sync_openpgp_keyserver',
>>  							'sync_openpgp_key_path',
>> +
>> 'sync_openpgp_key_refresh', 'sync_openpgp_key_refresh_retry_count',
>>  							'sync_openpgp_key_refresh_retry_delay_exp_base',
>>  							'sync_openpgp_key_refresh_retry_delay_max',
>> @@ -1047,6 +1054,7 @@ class RepoConfigLoader(object):
>>  		bool_keys = (
>>  			"strict_misc_digests",
>>  			"sync_allow_hardlinks",
>> +			"sync_openpgp_key_refresh",
>>  			"sync_rcu",
>>  		)
>>  		str_or_int_keys = (
>> diff --git a/lib/portage/sync/syncbase.py
>> b/lib/portage/sync/syncbase.py index 46644d68e..74818a420 100644
>> --- a/lib/portage/sync/syncbase.py
>> +++ b/lib/portage/sync/syncbase.py
>> @@ -1,4 +1,4 @@
>> -# Copyright 2014-2018 Gentoo Foundation
>> +# Copyright 2014-2020 Gentoo Authors
>>  # Distributed under the terms of the GNU General Public License v2
>>  
>>  '''
>> @@ -252,6 +252,13 @@ class SyncBase(object):
>>  		@type openpgp_env: gemato.openpgp.OpenPGPEnvironment
>>  		"""
>>  		out = portage.output.EOutput(quiet=('--quiet' in
>> self.options['emerge_config'].opts)) +
>> +		if not self.repo.sync_openpgp_key_refresh:
>> +			out.ewarn('Key refresh is disabled via a
>> repos.conf sync-openpgp-key-refresh')
>> +			out.ewarn('setting, and this is a security
>> vulnerability because it prevents')
>> +			out.ewarn('detection of revoked keys!')
>> +			return
>> +
>>  		out.ebegin('Refreshing keys via WKD')
>>  		if openpgp_env.refresh_keys_wkd():
>>  			out.eend(0)
>> diff --git a/man/portage.5 b/man/portage.5
>> index 36c871123..136ebaafe 100644
>> --- a/man/portage.5
>> +++ b/man/portage.5
>> @@ -1,4 +1,4 @@
>> -.TH "PORTAGE" "5" "Apr 2019" "Portage VERSION" "Portage"
>> +.TH "PORTAGE" "5" "Jun 2020" "Portage VERSION" "Portage"
>>  .SH NAME
>>  portage \- the heart of Gentoo
>>  .SH "DESCRIPTION"
>> @@ -1124,6 +1124,13 @@ Path to the OpenPGP key(ring) used to verify
>> received repository. Used only for protocols supporting cryptographic
>> verification, provided that the respective verification option is
>> enabled. If unset, the user's keyring is used.
>> +.TP
>> +.B sync\-openpgp\-key\-refresh = yes
>> +Enable OpenPGP key(ring) refresh. This option is enabled by default.
>> +
>> +\fBWarning\fR: It is a security vulnerability to disable this option
>> +because this will prevent detection of revoked keys!
>> +
>>  .TP
>>  .B sync\-openpgp\-key\-refresh\-retry\-count = 40
>>  Maximum number of times to retry key refresh if it fails. Between
>> each
> 
> 
> Is this something we can override with emaint sync in order to refresh
> the keys on demand?   This would be the same as emaint sync ability to
> ignore the sync = no for manaual syncing on demand only.
> 
> Possibly add an option to emaint sync that causes a refresh of the key.

These are questions for Rick since I never plan to use this feature myself.
-- 
Thanks,
Zac


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 981 bytes --]