Project

Profile

Help

Issue #3474

closed

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

Added by mihai.ibanescu@gmail.com about 6 years ago. Updated about 5 years ago.

Status:
CLOSED - CURRENTRELEASE
Priority:
Normal
Assignee:
-
Sprint/Milestone:
-
Start date:
Due date:
Estimated time:
Severity:
2. Medium
Version:
Platform Release:
2.16.0
OS:
Triaged:
Yes
Groomed:
No
Sprint Candidate:
No
Tags:
Pulp 2
Sprint:
Quarter:

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 overridesCLOSED - CURRENTRELEASEActions
Actions #1

Updated by dalley about 6 years ago

  • Triaged changed from No to Yes
Actions #2

Updated by dkliban@redhat.com about 6 years ago

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

Actions #3

Updated by mihai.ibanescu@gmail.com about 6 years 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.

Actions #4

Updated by mihai.ibanescu@gmail.com about 6 years ago

  • Status changed from NEW to POST
Actions #5

Updated by mihai.ibanescu@gmail.com about 6 years ago

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

Added by Mihai Ibanescu about 6 years ago

Revision f351ff72 | View on GitHub

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

Actions #6

Updated by Anonymous about 6 years ago

  • Status changed from POST to MODIFIED
Actions #7

Updated by bmbouter about 6 years ago

  • Platform Release set to 2.16.0

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

Added by Mihai Ibanescu about 6 years ago

Revision 7b6f0f6f | View on GitHub

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)

Actions #8

Updated by Anonymous about 6 years ago

Actions #9

Updated by bmbouter about 6 years ago

  • Status changed from MODIFIED to 5
Actions #11

Updated by bmbouter about 6 years ago

  • Status changed from 5 to CLOSED - CURRENTRELEASE
Actions #12

Updated by bmbouter about 5 years ago

  • Tags Pulp 2 added

Also available in: Atom PDF