-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
ws: always take packages from the first host #17970
ws: always take packages from the first host #17970
Conversation
When constructing requests to the packages webserver in the bridge, in response to incoming GET requests, always route the request to the bridge on the login host. This prevents the additional hosts that we login to from sending us evil JS.
This essentially breaks the "add remote host" functionality of the shell, unless machines are exactly identical (which is a very untypical case, I'd say). We don't actually test that a lot, but there's at least two "proper" integration test failures which demonstrate that, plus a more or less accidental one. "cockpit pages are loaded from the logged in system" has been a pretty fundamental design concept from the start. With this change this would still be true for the "primary" session (which delivers the shell), so the bastion host and flatpak scenarios should still be okay. That I consider a lot more important than the "add remote host" shell feature, which has been somewhat of a thorn in our eye for a long time (e.g. we didn't even support it in RHEL 7, as we had too many concerns about not isolating hosts from each other). With the portable bridge direction, I've been a proponent myself to losen that "load from target system" principle, but this requires a lot of work still: At least some kind of dynamic feature detection from manifests (or perhaps installation of required APIs on demand); and we'd have to bundle all the pages which we want to support in the cockpit/ws container or flatplak, so it will never be able to fully support all available cockpit pages. That is okay, as this is supposed to be a foot into the door, and then the Apps page etc. can provide installation of more cockpit-* packages on demand. In that sense, we could also just ship cockpit-{system,packagekit} for a minimal UI, and leave installation of everything else to the admin then. All that is fungible of course, and subject to debate. So from my POV, what I am willing to do is: Drop the "remote host" functionality from the shell. If we keep it and also want this PR's change, then we need to present this in a different and more cautious way, as we don't even know whether the local and remote pages are compatible with each other. But the honest approach would be to disable it. FTR, what I am not willing to do is to give up the "cockpit pages are loaded from the logged in system" principle completely. Dropping that would nullify a lot of Cockpit's strength and utility. |
My opinion, for the record: I think we're inescapably moving toward a place where the pages available for use with cockpit are more a feature of the client side (or bastion host/container) and less a feature of the installed OS. We're discussing — after all — using cockpit to admin machines that don't have any cockpit packages on them. I think we can come up with all kinds of interesting ways to offer sufficient extensibility from the side of the client. BUT, for the sake of this PR: this is not about packages coming from the client. In this case, the packages are still coming from the server — but the first server. I don't think that doing this is in any way dishonest or reduced functionality — in some ways it might even be preferable, since custom pages (for example) need only be installed one one host, and then can be used on all the others. I certainly disagree that we're somehow ending up with a dysfunctional vestige of what we used to have here. It's just different — in some ways better, but definitely different. And I certainly disagree with ripping the feature out entirely as being the "honest" way to proceed. |
Strongly disagree. I have my server (Debian stable) as a remote host. Of course for dogfooding, but also because I find some features genuinely interesting, e.g. rummaging in PCP's logs (the new metrics page was the reason why I installed PCP on my server). This is actually quite a common setup -- workstations and servers are necessarily very different, and servers also have different purposes, packages, and OSes. I built cockpit-ws with this change, connected to my server, and in the first two minutes see these issues:
All these are fixable somehow -- sometimes it's relatively easy, like adding some runtime detection to manifest parsing. Sometimes it's super hard, like keeping compatibility with changing APIs for a lot longer, and testing a lot more old OS releases (Ubuntu 20.04, Debian oldstable, RHEL 7, etc.). But we have to do something -- we can't possibly sell this broken remote session in good faith. FTR, it has never been my intention to only ever use pages from the server. I'd like to get some usable Cockpit with a few crucial pages (Overview, some metrics, Terminal), and most importantly the ability to discover and install more cockpit-* packages as needed (like podman, storaged, machines, 389-ds, file-sharing, and more). Supporting all of these in a static flatpak or container is neither doable nor desirable. So, if your intention of this PR is something like "it'll be at the end of a long dependency list and a year or three of work", then sure. If you actually intended to land it soon, then I frankly don't see how to salvage shell remote hosts, and it'd be honest to just kill it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(See above, need to handle the review request notification)
See #19071 for the new way forward here... |
When constructing requests to the packages webserver in the bridge, in response to incoming GET requests, always route the request to the bridge on the login host.
This prevents the additional hosts that we login to from sending us evil JS.