pybridge: Improve beiboot error reporting for fatal login failures

[martinpitt]

After all SSH authentication attemps fail, e.g. entering the wrong
password three times, ferny throws an InteractionError. Merely exiting
cockpit-beiboot gets interpreted as "Internal error" which is unfriendly.

Ferny recognizes the most common errors, like DNS resolution and and
authentication failure. Use that API to convert them to proper cockpit
protocol error codes, so that they get presented and translated properly.

---

Two commits split out from PR #19401

[martinpitt]

@allisonkarlitskaya gentle review ping?

[allisonkarlitskaya]

Where are you going with this? This is (so far) only about Cockpit Client, right, and we normally see the error messages via looking at stderr there...

If you want to do this, I think it would also make sense to disable (and possibly remove) the code in -ws for showing stderr of the spawned process?

[martinpitt]

@allisonkarlitskaya it's preparing us for using cockpit-beiboot for the normal login page as well. As I said, it was taken out of #19401 as preparation, but it seems like a good general fix.

[allisonkarlitskaya]

@allisonkarlitskaya it's preparing us for using cockpit-beiboot for the normal login page as well. As I said, it was taken out of #19401 as preparation, but it seems like a good general fix.

My question about the stderr handling in Cockpit Client remains. If we're no longer writing error messages to stderr, then probably we should remove that special case, right?

[martinpitt]

@allisonkarlitskaya ack, thanks for pointing out. I'll do some git-archaeology to find that and see if we can remove it (but not today, I have some higher-prio stuff to work on).

[martinpitt]

Ah, this is about X-For-CockpitClient. We can't eliminate that wholesale, as it also affects the layout of the login page. But the stderr special cases in commit 05978ba and commit 9ad3f39 can be dropped indeed. Done.

[allisonkarlitskaya]

I'm a bit worried about other unexpected failures that we might have wanted to sort out by inspecting stderr. This happens to me a lot and I'm happy to be able to see the error in the browser window directly. I feel like we get into a lot more interesting situations with Cockpit Client, particularly with beibooting over ssh(1)... think random stuff like #20164, which would have been totally opaque without the stderr reporting, which made it immediately obvious.

I wonder if we could do some sort of a hybrid model here: if we get a proper "init/problem" message from the other side, we show that as a proper error message. In case the program exits unexpectedly with no error message, we show the stderr as an "Internal error", maybe behind a expander.

[martinpitt]

@allisonkarlitskaya I'm confused now. Wasn't that "hybrid model" exactly the previous state of the PR, when we handled the explicitly known errors, but any exception would directly bubble up to the (c-client) login page? Then you asked me to drop the special case. If you have something else in mind, then I need some more details.

[allisonkarlitskaya]

@martinpitt I think the thing that changed in the meantime is that #20164 happened and I remembered how much I like seeing the stderr messages in case of things going totally off the rails.

(Sorry.)

[martinpitt]

Ack, backing out the revert (commit 8454d65) then.

[allisonkarlitskaya]

This looks good, subject to a single question.

[allisonkarlitskaya]

I assume that's enough to get Cockpit Client not to show the stderr output then?

Like, what happens if we leak some stderr and then send a proper init/problem? Which one will get shown? Can we test it?

[martinpitt]

With this diff

--- src/cockpit/beiboot.py
+++ src/cockpit/beiboot.py
@@ -308,6 +308,8 @@ async def run(args) -> None:
         bridge.write_control(message)
         bridge.ssh_peer.thaw_endpoint()
     except ferny.InteractionError as exc:
+        import sys
+        sys.stderr.write("Uh, schlimme Dinge!\n")
         error = ferny.ssh_errors.get_exception_for_ssh_stderr(str(exc))
         logger.debug("ferny.InteractionError: %s, interpreted as: %r", exc, error)
         if isinstance(error, ferny.SshAuthenticationError):

... it will still show the "proper" error code and no stderr:

But with

--- src/cockpit/beiboot.py
+++ src/cockpit/beiboot.py
@@ -308,6 +308,7 @@ async def run(args) -> None:
         bridge.write_control(message)
         bridge.ssh_peer.thaw_endpoint()
     except ferny.InteractionError as exc:
+        assert False, "Uh, schlimmere Dinge!"
         error = ferny.ssh_errors.get_exception_for_ssh_stderr(str(exc))
         logger.debug("ferny.InteractionError: %s, interpreted as: %r", exc, error)
         if isinstance(error, ferny.SshAuthenticationError):

you get the full glory:

That feels right to me?

[allisonkarlitskaya]

Yup.

[allisonkarlitskaya]

TODO: fixed with and pre and expander.