Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add the ability to hide certificate guarded repositories from the listing present at /pulp/content #3061

Closed
ehelms opened this issue Aug 11, 2022 · 10 comments · Fixed by #3179

Comments

@ehelms
Copy link
Contributor

ehelms commented Aug 11, 2022

Is your feature request related to a problem? Please describe.
For repositories that are protected by client certificates, users desire a way for these to not appear on the listing page that is present when visiting /pulp/content. This behavior is how Pulp 2 worked and users have expressed a desire for it to return.

My understanding is that user's view these as protected, and more sensitive than repositories not guarded by client certificates and would prefer they do not appear as browsable in any way to users who do not have the proper client certificates to present to the server.

Additional context

There is a Satellite BZ that provides additional context and the originating request: https://bugzilla.redhat.com/show_bug.cgi?id=2088559

@mdellweg
Copy link
Member

Can this be handled if that page was rendered as a (jinja2-) template?

@mdellweg
Copy link
Member

Should users with the proper access (through the content guard) be able to see the paths again?

@dralley
Copy link
Contributor

dralley commented Aug 17, 2022

This:

For repositories that are protected by client certificates, users desire a way for these to not appear on the listing page that is present when visiting /pulp/content

Would be quite simple to do, a one line change in fact [0]

But this:

do not appear as browsable in any way to users who do not have the proper client certificates to present to the server.

Would be a bit more complictated. @ehelms Can you clarify which behavior you're looking for?

[0]

Author: Daniel Alley <dalley@redhat.com>
Date:   Wed Aug 17 09:38:03 2022 -0400

    Hide contentguard-protected repositories from the directory listing
    
    closes #3061

diff --git a/CHANGES/3061.bugfix b/CHANGES/3061.bugfix
new file mode 100644
index 000000000..aedf06027
--- /dev/null
+++ b/CHANGES/3061.bugfix
@@ -0,0 +1 @@
+Repositories that are protected by a content guard should be hidden from the directory listing in the content app.
diff --git a/pulpcore/content/handler.py b/pulpcore/content/handler.py
index 8c864f819..ee858d7cf 100644
--- a/pulpcore/content/handler.py
+++ b/pulpcore/content/handler.py
@@ -162,7 +162,7 @@ class Handler:
 
         def get_base_paths_blocking():
             distro_model = self.distribution_model or Distribution
-            raise DistroListings(path="", distros=distro_model.objects.all())
+            raise DistroListings(path="", distros=distro_model.objects.filter(content_guard__isnull=True))
 
         if request.method.lower() == "head":
             raise HTTPOk(headers={"Content-Type": "text/html"})

@ehelms
Copy link
Contributor Author

ehelms commented Aug 24, 2022

The way it seems to work today is that all repositories within Pulp show up in a list when browsing to /pulp/content/, e.g. The repositories which are content guarded shouldn't appear in that list. That would be the simple blanket fix to the problem I think.

Users have expressed that if they present valid client certificates when visiting /pulp/content/ they would see those content guarded repositories listed. I think this is a nice to have not a must have for almost all users.

Accessing an individual repository at it's fully qualified path is already protected and to my knowledge needs no additional work, e.g. if visit a content guarded repository I see:

403: [('PEM routines', 'get_name', 'no start line')]

(One could argue that should be a 404 rather than a 403.)

@dralley
Copy link
Contributor

dralley commented Aug 26, 2022

The way it seems to work today is that all repositories within Pulp show up in a list when browsing to /pulp/content/, e.g. The repositories which are content guarded shouldn't appear in that list. That would be the simple blanket fix to the problem I think.

Users have expressed that if they present valid client certificates when visiting /pulp/content/ they would see those content guarded repositories listed. I think this is a nice to have not a must have for almost all users.

I'm pretty certain we can do this in an hour or two in that case. Plus some margin for edge cases I haven't thought of and writing some automated tests.

Accessing an individual repository at it's fully qualified path is already protected and to my knowledge needs no additional work, e.g. if visit a content guarded repository I see:

Great news, glad there's no issue there

(One could argue that should be a 404 rather than a 403.)

For obscuration purposes I presume? (If you were spraying requests you could theoretically uncover which protected directories exist based on getting not-found or access-denied).

@ehelms
Copy link
Contributor Author

ehelms commented Aug 26, 2022

For obscuration purposes I presume? (If you were spraying requests you could theoretically uncover which protected directories exist based on getting not-found or access-denied).

Correct.

@mdellweg
Copy link
Member

mdellweg commented Sep 7, 2022

For obscuration purposes I presume? (If you were spraying requests you could theoretically uncover which protected directories exist based on getting not-found or access-denied).

Correct.

I agree that this may be a valid concern, but i think that would be a separate issue. Should we have one filed?

@mdellweg mdellweg self-assigned this Sep 7, 2022
@dralley
Copy link
Contributor

dralley commented Sep 7, 2022

I believe so, yes.

mdellweg added a commit to mdellweg/pulpcore that referenced this issue Sep 8, 2022
mdellweg added a commit to mdellweg/pulpcore that referenced this issue Sep 8, 2022
@mdellweg mdellweg linked a pull request Sep 8, 2022 that will close this issue
mdellweg added a commit to mdellweg/pulpcore that referenced this issue Sep 9, 2022
@ipanova
Copy link
Member

ipanova commented Sep 9, 2022

For obscuration purposes I presume? (If you were spraying requests you could theoretically uncover which protected directories exist based on getting not-found or access-denied).

Correct.

I agree that this may be a valid concern, but i think that would be a separate issue. Should we have one filed?

I think we should distinguish between 404 and 403, in case user presents invalid client cert he should receive 403, imo.

@mdellweg
Copy link
Member

mdellweg commented Sep 9, 2022

For obscuration purposes I presume? (If you were spraying requests you could theoretically uncover which protected directories exist based on getting not-found or access-denied).

Correct.

I agree that this may be a valid concern, but i think that would be a separate issue. Should we have one filed?

I think we should distinguish between 404 and 403, in case user presents invalid client cert he should receive 403, imo.

Isn't that a 401?

mdellweg added a commit to mdellweg/pulpcore that referenced this issue Sep 13, 2022
@dralley dralley added the BZ label Oct 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants