Project

Profile

Help

Story #5974

As a user, I can restrict file:/// to one or more specific paths

Added by bmbouter 5 months ago. Updated 4 months ago.

Status:
CLOSED - CURRENTRELEASE
Priority:
Normal
Assignee:
Category:
-
Sprint/Milestone:
Start date:
Due date:
% Done:

100%

Platform Release:
Blocks Release:
Backwards Incompatible:
No
Groomed:
No
Sprint Candidate:
No
Tags:
QA Contact:
Complexity:
Smash Test:
Verified:
No
Verification Required:
No
Sprint:
Sprint 65

Description

At the system level, default to denying file import from file:/// unless a subpath of one or more declared approved paths. The list of approved paths will live in settings as it's a system-wide thing.

Example whitelisting importing from /mnt/ and /anotherdir/

ALLOWED_IMPORT_PATHS = ['/mnt/', '/anotherdir/']

Then any file:/// sync that starts with '/mnt/' or '/anotherdir/', e.g. file:///mnt/foo/ or file:///anotherdir/bar/PULP_MANIFEST would sync.

Each path listed in ALLOWED_IMPORT_PATHS must be absolute

Default

For safety reasons, this would be disabled by default and administrators need to opt-in to its use.

ALLOWED_IMPORT_PATHS = []

Validation checking at Remote save-time and runtime

The Remote serializer needs to validate at save time. Also that validation needs to be performed at runtime. It's this second check that will handle any existing Remote's with invalid paths already saved in the DB in a reasonable way.

Associated revisions

Revision b7db663a View on GitHub
Added by bmbouter 4 months ago

Adds ALLOWED_IMPORT_PATHS for file:// urls

Remote.url now validate to only allow file:// paths that are a subpath of a configured ALLOWED_IMPORT_PATHS setting. This setting comes with docs. URLs starting with other protocol handlers, e.g. http:// or https:// are unaffected.

https://pulp.plan.io/issues/5974 closes #5974

History

#1 Updated by bmbouter 5 months ago

  • Description updated (diff)

#2 Updated by bmbouter 5 months ago

  • Description updated (diff)

Question 1: symlinks

How/where should we prevent the file:/// import processes from following symlinks. If we follow symlinks users could perhaps break out of the constraint this feature creates. Should that be its own piece of work separate from this?

Question 2: disallowing ..

Also what about disallowing .. from file:/// urls at all through validation? Should we make that part of this story also?

#3 Updated by bmbouter 5 months ago

  • Sprint/Milestone set to 3.1.0

#4 Updated by gmbnomis 5 months ago

I think restricting the allowed paths by configuration is a good idea (if it is ok for the use cases that this feature should support). And disabling it by default is the safest option.

bmbouter wrote:

Question 1: symlinks

How/where should we prevent the file:/// import processes from following symlinks. If we follow symlinks users could perhaps break out of the constraint this feature creates. Should that be its own piece of work separate from this?

I would like to have it part of this piece of work, as the protection offered without is basically none. Please see https://pulp.plan.io/issues/3841#note-11 and the problem mentioned in https://pulp.plan.io/issues/3841#note-12 for an implementation that could help here (using realpath to resolve symlinks). Normalizing via realpath() would probably need to happen for both the file URI to check as well as the configured allowed paths.

Question 2: disallowing ..

Also what about disallowing .. from file:/// urls at all through validation? Should we make that part of this story also?

We could do, but using realpath() will normalize .. away anyway.

#5 Updated by bmbouter 4 months ago

  • Status changed from NEW to ASSIGNED
  • Assignee set to bmbouter
  • Sprint set to Sprint 65

Adding 3.1 story to sprint since I have capacity to complete it.

#6 Updated by bmbouter 4 months ago

  • Status changed from ASSIGNED to POST

#7 Updated by bmbouter 4 months ago

  • Status changed from POST to MODIFIED
  • % Done changed from 0 to 100

#8 Updated by bmbouter 4 months ago

  • Status changed from MODIFIED to CLOSED - CURRENTRELEASE

Please register to edit this issue

Also available in: Atom PDF