Allow for additional reverse proxy vhost options

[MartinSpiessl]

the class foreman_proxy_content::reverse_proxy already has a parameter vhost_params. However, it is not exposed for usage, so while one can specify additional vhost parameters for HTTP via pulpcore::apache::http_vhost_options, there is no way to do the same for HTTPS. This commit changes that by adding a variable foreman_proxy_content::reverse_proxy_vhost_params that is then passed on to foreman_proxy_content::reverse_proxy such that a user can add additional vhost parameters for HTTP and HTTPS e.g. via custom-hiera.

[ehelms]

What kind of additional configuration are you looking to perform on the vhost?

[MartinSpiessl]

EDIT: tl;dr: for adding custom httpd rewrite rules

The use cases that triggered this is preventing directory traversal to the parent directory from e.g. the content view directories up to the organizations by adding a custom httpd RewriteRule. Some users of foreman might manage different of their customers as different organizations, and ideally the "list of customers" is not leaked by a simple director traversal. Still the full paths to other "customer's" lifecycles, repositories et.c might be determined by guessing, but it is better than nothing. A "proper" solution of this might also be worth investigating as part of a separate issue (if you want I will gladly open one), but the change here allows us to be flexible in addressing such custom requirements without adding a lot of edge cases to the foreman code itself, so I think this is really worth having. Custom vhost parameters also work out of the box for foreman (via custom-hiera.yaml), just not for the foreman proxy.

[MartinSpiessl]

@ehelms if you have any more questions let me know. Is there anything more I need to do here, e.g. search for a reviewer, or are you reviewing this?

[ehelms]

So, you'd want a setting like https://pulpproject.org/pulpcore/docs/admin/reference/settings/#hide_guarded_distributions that applies to everything and not just content guarded repositories?

@ekohl what are your thoughts on this open ended parameter?

[MartinSpiessl]

I did not know about these content guarded distributions, but yes, just guarding the distribution names would not help.

Unless distributions are a different concept in pulp, and what I call organization is indeed a "distribution". On foreman, I talk about paths like:
foreman.example.org/pulp/content/MyOrganization/MyLifeCycle/AlmaLinux_8/custom/AlmaLinux_8/AppStream/

the main use case currently for us is to have the listing in foreman.example.org/pulp/content/ not be shown to all users, such that not all organization names like MyOrganization are visible. Also it should be possible to hide the lifecycle environment names like MyLifeCycle in the directory of foreman.example.org/pulp/content/MyOrganization/. This can be achieved by a simple httpd rewrite rule, not allowing the directory listing to be shown.

Even if these content guarded distributions would work, I am not sure how this can be enabled in foreman, I could not find any documentation on this from the foreman side.

This MR addresses an implementation oversight where one parameter is not passed through correctly in one particular case. While I am curious about other solutions and whether this could also be provided via other means, I think this MR has its justification regardless whether there is a different solution. Otherwise, the implication would be that it should also not be possible to add custom vhost options on http, which is currently possible.

While searching documentation on the content guarded distributions, I found this:
pulp/pulpcore#3538
pulp/pulpcore#3768

Which sounds like this is yet another way to hide distributions.

[ekohl]

@ekohl what are your thoughts on this open ended parameter?

Generally I try to avoid them at the top level because they're really hard to support: you're exposing many internal details. I have no objection to adding them in a way you can only use Hiera because that by definition is a "you're on your own" situation.

Even if these content guarded distributions would work, I am not sure how this can be enabled in foreman, I could not find any documentation on this from the foreman side.

There is the pulpcore::hide_guarded_distributions option:

https://github.com/theforeman/puppet-pulpcore/blob/8288f3736a1543417eba6322e7013d0f230bd85e/manifests/init.pp#L269

We pass that from our other module where it defaults to true:

puppet-foreman_proxy_content/manifests/init.pp

Line 111 in 73ab73b

Boolean $pulpcore_hide_guarded_distributions = true,

puppet-foreman_proxy_content/manifests/init.pp

Line 225 in 73ab73b

hide_guarded_distributions => $pulpcore_hide_guarded_distributions,

Then I don't know how from Katello you can enable this.

Which sounds like this is yet another way to hide distributions.

I think a proper feature in Pulp would be the preferred way.

Looking at the code I think https://github.com/pulp/pulpcore/blob/9971e1beafb9e27c26d71568d1b64a7986bc5b33/pulpcore/content/handler.py#L87-L118 is the listing in question.

I get the impression that if you hide all your content with guarded distributions then distros will be empty and it won't show the organizations. In other words: it should only show organizations which have unguarded content available already.

[ehelms]

the class foreman_proxy_content::reverse_proxy already has a parameter vhost_params. However, it is not exposed for usage, so while one can specify additional vhost parameters for HTTP via pulpcore::apache::http_vhost_options, there is

There exists a parameter for HTTPs, pulpcore::apache::https_vhost_options that can be set in custom-hiera.yaml. Does that suffice?

As @ekohl indicates, our desire on design and support falls into:

• Not exposing internals at the top level given our experience attempting to support those long term
• The proper fix would be a feature in Pulp to control what is displayed @dralley @ggainey
• If there is not an internal parameter that does what you need via custom-hiera.yaml we can look at what might be possible to add to puppet-pulpcore

[MartinSpiessl]

There exists a parameter for HTTPs, pulpcore::apache::https_vhost_options that can be set in custom-hiera.yaml. Does that suffice?

We also saw this and tried it according to my notes, but unfortunately it did not work for https (pulpcore::apache::http_vhost_options works fine for http, this is what we use for http, but with the https variant we had no luck). I did not note the exact reason though, I can try again on Monday and report back, or maybe you can make sense of this.

[dralley]

It kind a sounds like you want to hide all directory listings from the content app, is that right?

[MartinSpiessl]

@dralley
I want to hide certain levels of the directory listing, which contain sensitive information, as outlined here: #504 (comment)

[ekohl]

We also saw this and tried it according to my notes, but unfortunately it did not work for https (pulpcore::apache::http_vhost_options works fine for http, this is what we use for http, but with the https variant we had no luck). I did not note the exact reason though, I can try again on Monday and report back, or maybe you can make sense of this.

The problem is that we define the vhost elsewhere:

puppet-foreman_proxy_content/manifests/init.pp

Lines 239 to 248 in dd1593c

	foreman_proxy_content::reverse_proxy { $apache_https_vhost:
	docroot => $pulpcore::apache_docroot,
	path_url_map => {
	$rhsm_path => "${proxy_foreman_url}${rhsm_path}",
	$insights_path => "${proxy_foreman_url}${insights_path}",
	},
	port => $rhsm_port,
	priority => '10',
	before => Class['pulpcore::apache'],
	}

That is a define, so you can't use Hiera to set foreman_proxy_content::reverse_proxy::vhost_params.

Now that we've dropped port 8443 we could turn it into a class which allows you to set it via Hiera.

[MartinSpiessl]

As long as I can set it via the hiera, I would be happy. Just to be sure I get it correctly, you are saying the own class would be preferred over a solution like I proposed in this MR?

[ekohl]

Yes. Mostly because we expose all parameters in init.pp in our installer and this isn't an option we want to advertise. Adding it to a private class so users can set it via Hiera is fine because users don't see it unless they really know what they're looking for.