Fixes #37761 - Allow rewrites needed for cockpit integration #1178

adamruzicka · 2024-08-26T14:17:34Z

A successor to #1177 which github won't allow me to reopen.

ehelms

Tested and solved it for me. @evgeni @ekohl any comments to add?

ekohl · 2024-09-16T21:55:19Z

spec/classes/plugin/remote_execution_cockpit_spec.rb

@@ -71,7 +71,7 @@ class {'foreman':
            .without_content
            .with_ssl_content(%r{^<Location /webcon>$})
            .with_ssl_content(%r{^  RewriteRule /webcon/\(\.\*\)           ws://127\.0\.0\.1:19090/webcon/\$1 \[P\]$})


This is still needed? And for websockets we don't suffer from the same problem? I ask because I get the impression this line is redundant

My understanding is (without having explicitly tested that) that w/o this we do not get a protocol upgrade on EL8 (and Ubuntu Focal).

On EL9 (and Debian), we could use ProxyPass … upgrade=websocket, but not on EL8

evgeni

I am afraid this is not making the webconsole work for me on CS9 :(

httpd-2.4.57-8.el9.x86_64

This is the version without the fix for CVE-2024-38474 and thus "obviosly" works.

old config

<Location /webcon>
  ProxyPreserveHost On

  RewriteEngine On
  RewriteCond %{HTTP:Upgrade} =websocket [NC]
  RewriteRule /webcon/(.*)           ws://127.0.0.1:19090/webcon/$1 [P]
  RewriteCond %{HTTP:Upgrade} !=websocket [NC]
  RewriteRule /webcon/(.*)           http://127.0.0.1:19090/webcon/$1 [P]
</Location>

WORKS

new config

<Location /webcon>
  ProxyPreserveHost On

  RewriteEngine On
  RewriteCond %{HTTP:Upgrade} =websocket [NC]
  RewriteRule /webcon/(.*)           ws://127.0.0.1:19090/webcon/$1 [P]
  ProxyPass http://127.0.0.1:19090/webcon
</Location>

WORKS

config without explicit `ws://` proxy

<Location /webcon>
  ProxyPreserveHost On
  ProxyPass http://127.0.0.1:19090/webcon
</Location>

FAILS with

Connection failed
There was an unexpected error while connecting to the machine.

config without explicit `ws://` proxy but with `upgrade=websocket`

<Location /webcon>
  ProxyPreserveHost On
  ProxyPass http://127.0.0.1:19090/webcon upgrade=websocket
</Location>

I assumed (based on https://httpd.apache.org/docs/2.4/mod/mod_proxy.html#wsupgrade) this would work, but alas, it does not, same "unexpected error". 🤔

httpd-2.4.62-1.el9.x86_64

That's the version with the CVE fix

old config

<Location /webcon>
  ProxyPreserveHost On

  RewriteEngine On
  RewriteCond %{HTTP:Upgrade} =websocket [NC]
  RewriteRule /webcon/(.*)           ws://127.0.0.1:19090/webcon/$1 [P]
  RewriteCond %{HTTP:Upgrade} !=websocket [NC]
  RewriteRule /webcon/(.*)           http://127.0.0.1:19090/webcon/$1 [P]
</Location>

FAILS with "Forbidden" - this was expected

new config

<Location /webcon>
  ProxyPreserveHost On

  RewriteEngine On
  RewriteCond %{HTTP:Upgrade} =websocket [NC]
  RewriteRule /webcon/(.*)           ws://127.0.0.1:19090/webcon/$1 [P]
  ProxyPass http://127.0.0.1:19090/webcon
</Location>

FAILS with

Connection failed
There was an unexpected error while connecting to the machine.

config from #1177

<Location /webcon>
  ProxyPreserveHost On

  RewriteEngine On
  RewriteCond %{HTTP:Upgrade} =websocket [NC]
  RewriteRule /webcon/(.*)           ws://127.0.0.1:19090/webcon/$1 [P,UnsafeAllow3F]
  RewriteCond %{HTTP:Upgrade} !=websocket [NC]
  RewriteRule /webcon/(.*)           http://127.0.0.1:19090/webcon/$1 [P,UnsafeAllow3F]
</Location>

FAILS with

Connection failed
There was an unexpected error while connecting to the machine.

In all "unexpected error" cases, I see my Firefox (in the network tab) trying to access wss://centos9-stream-katello-nightly.tanso.example.com/webcon/cockpit+=centos9-stream.tanso.example.com/socket and quitting with "404 / WEBSOCKET CONNECTION REFUSED"

httpd agrees:

192.168.122.1 - - [17/Sep/2024:08:00:36 +0000] "GET /webcon/cockpit+=centos9-stream.tanso.example.com/socket HTTP/1.1" 404 1564 "-" "Mozilla/5.0 (X11; Linux x86_64; rv:130.0) Gecko/20100101 Firefox/130.0"

It's like the Upgrade: websocket header (which is sent!) is completely ignored.

evgeni · 2024-09-17T08:34:31Z

On EL8, with httpd-2.4.37-65.module_el8.10.0+3874+c2064c23.2.x86_64, both fixes (here and #1177) work, as long as I keep

  RewriteCond %{HTTP:Upgrade} =websocket [NC]
  RewriteRule /webcon/(.*)           ws://127.0.0.1:19090/webcon/$1 [P]

in the config.

upgrade=websocket doesn't work as a param, as expected.

evgeni · 2024-09-17T11:59:03Z

We probably should still merge this, to at least fix the issue on EL8… But it's still ugly.

adamruzicka · 2024-09-17T13:22:50Z

to at least fix the issue on EL8

and EL <=9.4

evgeni · 2024-09-17T13:25:06Z

to at least fix the issue on EL8

and EL <=9.4

But it's not broken with the old code on EL9.4 either, right?

ekohl · 2024-09-17T14:02:12Z

It's like the Upgrade: websocket header (which is sent!) is completely ignored.

We're loading mod_proxy_wstunnel. https://httpd.apache.org/docs/2.4/mod/mod_proxy_wstunnel.html has some examples that look relevant for RewriteRules:

ProxyPass / http://example.com:9080/
RewriteEngine on
RewriteCond %{HTTP:Upgrade} websocket [NC]
RewriteCond %{HTTP:Connection} upgrade [NC]
RewriteRule ^/?(.*) "ws://example.com:9080/$1" [P,L]

Would those help?

evgeni · 2024-09-17T14:08:10Z

Would those help?

I tested these all after my initial "doesn't work" reply, and the changes do not have any visible effect.

evgeni · 2024-09-17T14:57:12Z

the plot thickens!

It seems the WS request ends up routed to Rails, not foreman-cockpit, as I see the following in production.log:

2024-09-17T14:56:17 [I|app|53724f64] Started GET "/webcon/cockpit+=centos9-stream.tanso.example.com/socket" for 192.168.122.1 at 2024-09-17 14:56:17 +0000
2024-09-17T14:56:17 [F|app|53724f64]   
 53724f64 | ActionController::RoutingError (No route matches [GET] "/webcon/cockpit+=centos9-stream.tanso.example.com/socket"):

And that explains the 404!

And indeed, when I remove the following from the apache config:

  RewriteCond %{HTTP:Upgrade} =websocket [NC]
  RewriteRule /(.*) unix:///run/foreman.sock|ws://foreman/$1 [P,L]

it works.

WTF

ekohl · 2024-09-17T15:32:26Z

Could it be the order of items? That Apache evaluates the rules from top to bottom.

evgeni

The EL9 problem seems unrelated to the change at hand, so let's run with it.

evgeni · 2024-09-17T16:03:43Z

When I use

<VirtualHost *:443>
…
  ProxyPass http://foreman upgrade=websocket
<Location /webcon>
  ProxyPass http://cockpit/webcon upgrade=websocket
</Location>
</VirtualHost>

Things work. But that'll break EL8 and Focal 😢

ekohl · 2024-09-17T16:41:32Z

Things work. But that'll break EL8 and Focal 😢

I want to drop both of those for 3.13. Any thoughts on OS version specific content if we want to backport this?

evgeni · 2024-09-17T16:47:31Z

You mean using proxypass with upgrade on OSes that support it, and without (and instead rewrite) on those which don't?

ekohl · 2024-09-17T17:03:18Z

Exactly

evgeni · 2024-09-18T08:07:43Z

Okay, let's try this!

#1185

evgeni · 2024-09-18T17:49:40Z

#1185 was merged instead

adamruzicka force-pushed the cockpit-3f branch from f40d5a7 to d641409 Compare August 26, 2024 14:18

This was referenced Aug 26, 2024

Fixes #37761 - Pass cockpit access token as fragment theforeman/foreman_remote_execution#918

Open

Fixes #37761 - Allow rewrites needed for cockpit integration #1177

Closed

adamruzicka force-pushed the cockpit-3f branch 2 times, most recently from 390a202 to 8dee101 Compare August 27, 2024 08:50

Fixes #37761 - Allow rewrites needed for cockpit integration

242bf3c

adamruzicka force-pushed the cockpit-3f branch from 8dee101 to 242bf3c Compare August 27, 2024 09:49

ehelms approved these changes Sep 16, 2024

View reviewed changes

ekohl reviewed Sep 16, 2024

View reviewed changes

evgeni requested changes Sep 17, 2024

View reviewed changes

pr-processor bot added the Waiting on contributor label Sep 17, 2024

evgeni approved these changes Sep 17, 2024

View reviewed changes

evgeni closed this Sep 18, 2024

evgeni added the skip-changelog label Sep 18, 2024

adamruzicka deleted the cockpit-3f branch September 19, 2024 07:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes #37761 - Allow rewrites needed for cockpit integration #1178

Fixes #37761 - Allow rewrites needed for cockpit integration #1178

adamruzicka commented Aug 26, 2024

ehelms left a comment

ekohl Sep 16, 2024

evgeni Sep 17, 2024 •

edited

Loading

evgeni left a comment •

edited

Loading

evgeni commented Sep 17, 2024

evgeni commented Sep 17, 2024

adamruzicka commented Sep 17, 2024

evgeni commented Sep 17, 2024

ekohl commented Sep 17, 2024

evgeni commented Sep 17, 2024

evgeni commented Sep 17, 2024 •

edited

Loading

ekohl commented Sep 17, 2024

evgeni left a comment

evgeni commented Sep 17, 2024

ekohl commented Sep 17, 2024

evgeni commented Sep 17, 2024

ekohl commented Sep 17, 2024

evgeni commented Sep 18, 2024

evgeni commented Sep 18, 2024

Fixes #37761 - Allow rewrites needed for cockpit integration #1178

Fixes #37761 - Allow rewrites needed for cockpit integration #1178

Conversation

adamruzicka commented Aug 26, 2024

ehelms left a comment

Choose a reason for hiding this comment

ekohl Sep 16, 2024

Choose a reason for hiding this comment

evgeni Sep 17, 2024 • edited Loading

Choose a reason for hiding this comment

evgeni left a comment • edited Loading

Choose a reason for hiding this comment

httpd-2.4.57-8.el9.x86_64

old config

new config

config without explicit ws:// proxy

config without explicit ws:// proxy but with upgrade=websocket

httpd-2.4.62-1.el9.x86_64

old config

new config

config from #1177

evgeni commented Sep 17, 2024

evgeni commented Sep 17, 2024

adamruzicka commented Sep 17, 2024

evgeni commented Sep 17, 2024

ekohl commented Sep 17, 2024

evgeni commented Sep 17, 2024

evgeni commented Sep 17, 2024 • edited Loading

the plot thickens!

ekohl commented Sep 17, 2024

evgeni left a comment

Choose a reason for hiding this comment

evgeni commented Sep 17, 2024

ekohl commented Sep 17, 2024

evgeni commented Sep 17, 2024

ekohl commented Sep 17, 2024

evgeni commented Sep 18, 2024

evgeni commented Sep 18, 2024

evgeni Sep 17, 2024 •

edited

Loading

evgeni left a comment •

edited

Loading

config without explicit `ws://` proxy

config without explicit `ws://` proxy but with `upgrade=websocket`

evgeni commented Sep 17, 2024 •

edited

Loading