Repository navigation
GUACAMOLE-2301: Add serial console protocol - #695
Conversation
0e0a581 to
3d70f03
Compare
necouchman
left a comment
There was a problem hiding this comment.
@ciroiriarte I've found a few things that I think could use changing - mainly cosmetic/style/documentation. Other than that it looks pretty good to me.
I appreciate you taking the time to review this. Will look into it. |
|
Pushed 8c12b4b addressing the review: style/docs fixes, the libtelnet define cleanup, configure.ac now reuses the telnet check, transport-specific argument parsing, |
necouchman
left a comment
There was a problem hiding this comment.
A handful of style changes, and I think it's close.
That said, it's failing build, now, I think because of some recent changes to how process and thread titles are handled (see #723). I think you'll need to rework with those changes to fix that.
…w TCP, RFC2217) Adds a new "serial" terminal protocol plugin for browser access to serial consoles (switch/router/PDU administration). Three transports select via the serial-type/network-protocol parameters: a local tty device configured with termios, raw TCP to a ser2net endpoint, and RFC2217 (telnet COM-PORT-OPTION, compiled only when libtelnet is available via ENABLE_SERIAL_RFC2217). Supports configurable send-break (break-duration) through a client-agnostic serial-control pipe, paste pacing (paste-delay), and a device-path allowlist, reusing the shared terminal for recording/typescript/clipboard/color-scheme, mirroring the telnet/ipmi plugin structure.
…, and diagnostics support Implements the reviewed serial console feature gaps on top of the initial serial protocol plugin: - Auto-reconnect on drop (new "auto-reconnect", default true): the terminal and scrollback are kept alive across reconnects; a single stream object persists for the session and its fd/telnet context is swapped under write_lock by guac_serial_stream_reopen(), so the input thread never touches a stale fd. - Line-ending translation (new "line-ending": cr|lf|crlf, default cr) applied to typed Enter and pasted text via a small cross-buffer state machine. - Distinct contention/error logging: EBUSY/EACCES/EPERM and TIOCEXCL-busy on local devices, ECONNREFUSED/ECONNRESET on network transports. - Local echo (new "local-echo", default false). - Structured connect/disconnect reason logging (EOF, EIO, reset, hangup). - Hangup-on-close (new "hangup-on-close" -> HUPCL) and DTR/RTS control-line toggles via the serial-control pipe (dtr-on/off, rts-on/off), using TIOCMBIS/TIOCMBIC locally and RFC2217 SET-CONTROL (DTR 8/9, RTS 11/12). - Connect status banner with an "8N1"-style summary and press-Enter hint. - Interruptible paste-delay pacing so a long paste cannot stall teardown. The device allowlist is retained in settings and re-validated (re-realpath) on every local (re)open.
Add a reverse/listen mode for network serial connections: instead of dialing out to a gateway, guacd binds a local TCP port and accepts a single inbound connection from the device (e.g. a QEMU/KVM or VMware virtual serial port, or a console server configured to dial out). Network fd acquisition is factored into guac_serial_net_open_fd(), used by both the raw and RFC2217 backends, which either dials out or, when reverse-connect is set, binds/listens/accepts. The accepted socket feeds the existing raw/RFC2217 framing unchanged, and the auto-reconnect path re-listens for the next inbound connection on drop. New parameters: reverse-connect, bind-address (default 127.0.0.1), listen-timeout (default 60s). In reverse mode hostname is optional.
Add network-protocol=telnet: base RFC854 Telnet framing (IAC escaping, BINARY/SGA/ECHO negotiation) via libtelnet, without the RFC2217 COM-PORT option. Serial line settings are informational, as with raw. Intended for endpoints that speak plain Telnet rather than RFC2217 -- QEMU/KVM telnet: serial ports, VMware ESXi telnet:// virtual serial ports, and telnet-mode console servers. Reuses the existing libtelnet event handler and send/recv/free, and acquires its socket through guac_serial_net_open_fd(), so it works in both forward and reverse mode. A Send Break is delivered as the Telnet BRK command. Gated on libtelnet (ENABLE_SERIAL_RFC2217) with no-op stubs when libtelnet is absent.
Style and documentation: - Put each switch case on its own lines and expand one-line conditionals. - Complete function documentation, document every parity and flow-control enum member, and note the default of each connection argument. - Correct docs that omitted the Telnet transport or claimed that open failures abort the client; document the dtr/rts control-pipe commands. - Convert the open() and connect() errno chains to switch statements. libtelnet / build: - Drop the TELNET_BREAK fallback (libtelnet defines it) and rename the COM-PORT-OPTION number to GUAC_SERIAL_TELOPT_COMPORT, since libtelnet does not define option 44. - Define ENABLE_SERIAL_RFC2217 from the existing ENABLE_TELNET conditional rather than a separate one, report build_telnet in the configure summary, and align the summary columns. Settings: - Parse transport-specific arguments only for the transport in use. - Remove the per-connection allowed-devices parameter: whoever can set "device" can also set the allowlist, so it provides no protection. A guacd-level allowlist can be proposed separately. - Remove the unused GUAC_SERIAL_DEFAULT_PORT.
GUACAMOLE-2283 removed guacamole/proctitle.h from libguac. Threads are now created with guac_thread_create(), which names them, and the endpoint shown in guacd's process title is reported through guac_client_set_info(). Local devices and reverse-mode listeners are reported as the resource name, and outbound network connections as hostname and port. guac_thread tracks whether it was started, so the separate client_thread_valid flag is no longer needed. Also fixes early line breaks in the open() documentation of local.h, rfc2217.h and tcp.h.
8c12b4b to
5e0e378
Compare
|
Rebased onto current main and pushed 5e0e378, which adapts serial to the GUACAMOLE-2283 changes from #723:
The three early line breaks are fixed in the same commit. The plugin builds with |
Great. I'll take one more pass through it, hopefully tomorrow, and see if there's anything else to be fixed. |
necouchman
left a comment
There was a problem hiding this comment.
Just a few more things that might need to be updated. After these items I think it'll be ready to go.
- Include config.h automatically via AM_CPPFLAGS, as the other protocol plugins do, and drop the explicit includes. - Define the Pause, Break and Ctrl+0 keysyms that send a serial break as named constants. - Replace the RFC2217 COM-PORT-OPTION command #defines with an enum, and name the SET-PARITY and SET-CONTROL values instead of using bare numbers. - Document why control pipe tokens are limited to 64 bytes, and discard oversized tokens entirely. Previously the buffer was reset but the rest of the token kept accumulating, so the tail of an oversized token could be dispatched as a command.
|
Pushed 1b7d730, which addresses the four items from the last review. It builds with |
Summary
Adds a new
serialconsole protocol to guacd, implementing GUACAMOLE-2301. It brings serial-line consoles — network switches/routers, PDUs, out-of-band management ports, and VM virtual serial ports — into the browser using the existingguac_terminal, following the structure of the SSH and telnet protocols.Capabilities
/dev/ttyUSB0), guarded by anallowed-devicesallowlist.network-protocol:raw— transparent TCP byte piperfc2217— Telnet COM-Port-Option; negotiates baud/parity/etc. and can deliver a Send Breaktelnet— base RFC 854 Telnet framing (e.g. QEMUtelnet:, VMware ESXitelnet://)reverse-connect=true, guacd binds a local port and accepts an inbound connection from the device instead of dialing out (QEMU/VMware virtual serial ports, dial-out console servers). Parameters:reverse-connect,bind-address(default127.0.0.1),listen-timeout.RFC2217/telnet support is gated on libtelnet at build time and degrades gracefully to
rawwhen it is unavailable.Testing
Validated end-to-end through the web client against a real Juniper switch console and a QEMU/KVM virtual serial console — local,
raw,rfc2217,telnet, and reverse mode — including bidirectional input and reconnection. Full QA evidence (screenshots, typescripts, guacd logs) is attached to GUACAMOLE-2301.Coordinated PRs
This is one of three coordinated PRs: