Project

Profile

Help

Issue #3474

gpg_cmd configuration option should not be accepted in repo config or overrides

Added by mihai.ibanescu@gmail.com over 1 year ago. Updated 2 months ago.

Status:
CLOSED - CURRENTRELEASE
Priority:
Normal
Assignee:
-
Category:
-
Sprint/Milestone:
-
Severity:
2. Medium
Version:
Platform Release:
2.16.0
Blocks Release:
OS:
Backwards Incompatible:
No
Triaged:
Yes
Groomed:
No
Sprint Candidate:
No
Tags:
Pulp 2
QA Contact:
Complexity:
Smash Test:
Verified:
No
Verification Required:
No
Sprint:

Description

As a user, I can specify a gpg_cmd option in the plugin's distributor config, which will override the plugin config.

This has security implications, since it allows a potentially malicious user to execute commands remotely on the pulp server as user apache.

The fix is not entirely straightforward. I think one would want a per-config-type allowed options.


Related issues

Copied to Debian Support - Issue #3498: gpg_cmd configuration option should not be accepted in repo config or overrides CLOSED - CURRENTRELEASE Actions

Associated revisions

Revision f351ff72 View on GitHub
Added by Mihai Ibanescu over 1 year ago

gpg_cmd is not allowed as plugin or override configuration

Since the command configured with gpg_cmd executes remotely as user apache,
a user should not be allowed to change it via a distributor config or
an override at publish time.

Fixes #3474
https://pulp.plan.io/issues/3474

Revision 7b6f0f6f View on GitHub
Added by Mihai Ibanescu over 1 year ago

gpg_cmd is not allowed as plugin or override configuration

Since the command configured with gpg_cmd executes remotely as user apache,
a user should not be allowed to change it via a distributor config or
an override at publish time.

Fixes #3474
https://pulp.plan.io/issues/3474

(cherry picked from commit f351ff7240f8bf9b184f40a5e6896dd7e485a3a7)

History

#1 Updated by dalley over 1 year ago

  • Triaged changed from No to Yes

#2 Updated by dkliban@redhat.com over 1 year ago

This affects 2.16.0 which will be in Beta next week. We should fix this issue before it goes GA.

#3 Updated by mihai.ibanescu@gmail.com over 1 year ago

Possible patch (to be tested):

diff --git a/plugins/pulp_rpm/plugins/distributors/yum/configuration.py b/plugins/pulp_rpm/plugins/distributors/yum/configuration.py
index 45a8b191..648b8058 100644
--- a/plugins/pulp_rpm/plugins/distributors/yum/configuration.py
+++ b/plugins/pulp_rpm/plugins/distributors/yum/configuration.py
@@ -27,6 +27,8 @@ OPTIONAL_CONFIG_KEYS = ('gpgkey', 'auth_ca', 'auth_cert', 'https_ca', 'checksum_
                         'remove_old_repodata', 'remove_old_repodata_threshold',
                         GPG_CMD, GPG_KEY_ID)

+LOCAL_CONFIG_KEYS = [GPG_CMD]
+
 ROOT_PUBLISH_DIR = '/var/lib/pulp/published/yum'
 MASTER_PUBLISH_DIR = os.path.join(ROOT_PUBLISH_DIR, 'master')
 HTTP_PUBLISH_DIR = os.path.join(ROOT_PUBLISH_DIR, 'http', 'repos')
@@ -71,9 +73,17 @@ def validate_config(repo, config, config_conduit):
     :return: tuple of (bool, str) stating that the configuration is valid or not and why
     :rtype:  tuple of (bool, str or None)
     """ 
+    error_messages = []
+    msg = _('Configuration key [%(k)s] is not allowed in %(config)s configuration')
+    remote_configs = [
+        (config.repo_plugin_config, "repository plugin"),
+        (config.override_config, "override")]
+    for key in LOCAL_CONFIG_KEYS:
+        for cfgdict, cfgname in remote_configs:
+            if cfgdict.get(key):
+                error_messages.append(msg % dict(k=key, config=cfgname))

     config = config.flatten()  # squish it into a dictionary so we can manipulate it
-    error_messages = []

     configured_keys = set(config)
     required_keys = set(REQUIRED_CONFIG_KEYS)

But let me know if you spot something before I get to do the testing.

#4 Updated by mihai.ibanescu@gmail.com over 1 year ago

  • Status changed from NEW to POST

#5 Updated by mihai.ibanescu@gmail.com over 1 year ago

  • Copied to Issue #3498: gpg_cmd configuration option should not be accepted in repo config or overrides added

#6 Updated by Anonymous over 1 year ago

  • Status changed from POST to MODIFIED

#7 Updated by bmbouter over 1 year ago

  • Platform Release set to 2.16.0

This one fix is to be included in 2.16.0 Beta 1.

#8 Updated by Anonymous over 1 year ago

#9 Updated by bmbouter over 1 year ago

  • Status changed from MODIFIED to ON_QA

#10 Updated by pthomas@redhat.com about 1 year ago

  • Smash Test set to 912

#11 Updated by bmbouter about 1 year ago

  • Status changed from ON_QA to CLOSED - CURRENTRELEASE

#12 Updated by bmbouter 2 months ago

  • Tags Pulp 2 added

Please register to edit this issue

Also available in: Atom PDF