You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@guacamole.apache.org by "4m1nr (via GitHub)" <gi...@apache.org> on 2026/07/14 07:47:31 UTC

[PR] GUACAMOLE-2064: Add interactive database terminal protocols [guacamole-server]

4m1nr opened a new pull request, #694:
URL: https://github.com/apache/guacamole-server/pull/694

   ## What this does
   
   Adds interactive, terminal-based access to five databases from within
   Guacamole — **MySQL/MariaDB, PostgreSQL, SQL Server, MongoDB, and Oracle** —
   implementing the feature requested in
   [GUACAMOLE-2064](https://issues.apache.org/jira/browse/GUACAMOLE-2064).
   
   Following the approach suggested on the issue, guacd does **not** spawn a
   command-line client (`mysql`, `psql`, …). Instead each protocol links the
   database's own C client library and drives `libguac-terminal` directly,
   presenting a prompt at which statements are typed, edited, and executed. As a
   result, every feature Guacamole already provides for terminal protocols —
   typescripts, graphical session recording, terminal resize, clipboard, and
   shared / read-only sessions — works here unchanged.
   
   ## How it is structured
   
   All database-agnostic logic lives in one new convenience library,
   **`libguac-dbshell`** (`src/dbshell`), modeled on `src/common-ssh`:
   
   - **Line editor** — a byte-stream editor with cursor movement
     (Left/Right/Home/End), in-memory command history (Up/Down), UTF-8-aware
     insertion/deletion, the usual control shortcuts (Ctrl-A/E/K/U/W/L), and a
     parser that consumes-and-drops unknown escape sequences so arrow keys etc.
     never leak bytes into the buffer.
   - **Statement splitter** — accumulates input and detects complete statements
     per dialect: MySQL, PostgreSQL, Transact-SQL, Oracle, and JSON. Handles
     single/double/backtick/bracket quoting, `--` / `#` / block comments,
     PostgreSQL dollar-quoting (and `$1` positional params), T-SQL `GO` batches,
     Oracle PL/SQL `/`-terminated blocks, and JSON brace/bracket balancing.
   - **Result table renderer** — buffers an initial window of rows to compute
     column widths, then streams the remainder; mysql-style borders, right-aligned
     numeric columns, `NULL` rendering, width capping with an ellipsis. **It
     strips control characters from every cell**, so a malicious or compromised
     database server cannot inject terminal escape sequences (the DB analogue of
     CVE-2023-43826).
   - **REPL, generic plugin scaffold, and common connection settings** shared by
     every protocol (hostname/port/credentials/database/timeout plus the full
     terminal, typescript, recording, and Wake-on-LAN parameter sets).
   
   Each protocol plugin (`src/protocols/{mysql,postgresql,mssql,mongodb,oracle}`)
   is a thin driver implementing a small vtable:
   
   | Protocol   | Library                 | Notable driver behavior |
   |------------|-------------------------|-------------------------|
   | mysql      | MariaDB Connector/C     | non-blocking API (so teardown never blocks on a dead server); `KILL QUERY` cancel via a second connection; `LOAD DATA LOCAL INFILE` disabled |
   | postgresql | libpq                   | asynchronous API; `PQcancel`; server notices rendered inline; dollar-quoting-aware |
   | mssql      | FreeTDS CT-Library      | `GO` batches; server messages incl. `PRINT`; `ct_cancel` attention on Ctrl-C |
   | mongodb    | MongoDB C Driver        | Extended-JSON command shell; automatic cursor `getMore`; `\use` / `\show` meta-commands |
   | oracle     | Oracle Call Interface   | EZConnect; PL/SQL blocks; `OCIBreak` cancel |
   
   ## Build / packaging
   
   - `configure` autodetects libmariadb, libpq, FreeTDS, and libmongoc and
     disables the corresponding protocol with a warning if absent (same pattern as
     the existing libwebsockets/Kubernetes check).
   - **Oracle is never built by default** — its client (Oracle Instant Client) is
     proprietary and non-redistributable. It is enabled explicitly with
     `--with-oracle=<instant-client-dir>`.
   - The guacd `Dockerfile` installs the four redistributable client libraries, so
     the official image gains MySQL/PostgreSQL/SQL Server/MongoDB support; Oracle
     is documented as a custom-image step.
   
   > **⚠️ Open question for maintainers — OpenSSL in the Docker image.** The four
   > database client packages on Alpine depend on `openssl-dev` (OpenSSL 3), which
   > **cannot be installed alongside `openssl1.1-compat-dev`**, the package the
   > image currently pins (the base image is held at Alpine 3.18 for that reason).
   > This PR therefore switches the image to `openssl-dev`; guacd and its existing
   > dependencies build and run against OpenSSL 3 in my testing (the full image
   > builds and all existing protocols — RDP/VNC/SSH/telnet/Kubernetes — are
   > present and load). I have **kept the 3.18 base pin** and only updated the
   > comment that explained it, rather than bumping the base image, to keep this
   > change minimal.
   >
   > **Context:** the `openssl1.1-compat-dev` pin comes from
   > [GUACAMOLE-1741](https://issues.apache.org/jira/browse/GUACAMOLE-1741), where
   > VeNCrypt VNC authentication broke against OpenSSL 3.0.8 due to a
   > **libvncserver** incompatibility (not a guacd one). The Alpine 3.18 pin was
   > then added in the still-open
   > [GUACAMOLE-1891](https://issues.apache.org/jira/browse/GUACAMOLE-1891) because
   > Alpine 3.19 dropped the compat package; Alpine 3.18 has since reached
   > end-of-life (9 May 2025). That libvncserver blocker was **fixed upstream in
   > [LibVNCServer 0.9.15](https://github.com/LibVNC/libvncserver/releases/tag/LibVNCServer-0.9.15)**
   > (22 Dec 2024): *"Fixed building with OpenSSL >= 3.0.0."* Since the Dockerfile
   > already auto-selects the latest LibVNCServer release, the image is building
   > 0.9.15+ today, so the original reason for the OpenSSL 1.1 pin appears to be
   > stale. Moving to `openssl-dev` here is therefore a step toward what
   > GUACAMOLE-1891 needs, and that ticket looks close to resolvable.
   >
   > **Why I did not also bump the base image off 3.18:** I tried building on
   > `alpine:latest` (currently 3.24) and it is *not* a self-contained change — it
   > cascades into unrelated packaging churn: `webkit2gtk-dev` has been renamed
   > (to `webkit2gtk-4.1-dev`), and CMake 4 on newer Alpine hard-rejects the older
   > `cmake_minimum_required(VERSION < 3.5)` still declared by some bundled
   > dependency sources (the libvncserver build fails to configure), with more
   > likely behind it. That is a genuine base-image migration and belongs in
   > GUACAMOLE-1891, not in this feature PR, so I kept the 3.18 pin (Alpine 3.18
   > still ships OpenSSL 3.1, so `openssl-dev` works there) and only switched the
   > OpenSSL package.
   >
   > **The one thing I could not fully verify:** that VeNCrypt VNC auth works
   > against libvncserver 0.9.15 + OpenSSL 3 (the exact scenario GUACAMOLE-1741 was
   > about). It is worth an explicit re-test before the pin is considered dead. If
   > you would rather the Docker packaging (and thus the OpenSSL switch and that
   > VeNCrypt re-test) be handled separately from this feature — e.g. under
   > GUACAMOLE-1891 — I'm happy to drop the Docker commit and file a follow-up; the
   > protocols build fine from source regardless, so this only affects the
   > convenience image. Please let me know which you'd prefer.
   
   ## Testing
   
   **Unit tests (CUnit, `make check`):** new suites for the line editor & input
   parser, command history, the statement splitter across all five dialects, and
   cell sanitization. Full suite: **160 tests across 7 suites, all passing**
   (was 123/6 before).
   
   **Manual / integration testing:** built and ran guacd against live database
   containers, opening real connections through the Guacamole protocol with a
   typescript recording enabled and asserting on the recorded output:
   
   - **MySQL** (`mariadb:11`), **PostgreSQL** (`postgres:16`), **SQL Server**
     (`mcr.microsoft.com/mssql/server:2022`), **MongoDB** (`mongo:7`), **Oracle**
     (`gvenzl/oracle-free`): `SELECT` rendering (bordered table, right-aligned
     numerics, "N rows in set" footer), banners, and clean disconnect.
   - PostgreSQL: multi-line statement with continuation prompt, statement-error
     recovery (session survives), and UTF-8 round-trip (`café`).
   - MySQL: wrong-password path renders `ERROR 1045` and aborts cleanly.
   - Oracle: multi-line `BEGIN … END; /` PL/SQL block followed by a `SELECT … FROM
     dual` (proves semicolons inside the block do not terminate early).
   - MongoDB: `\use`, `\show collections`, `insert`, and `find`.
   - `docker build .` produces a guacd image containing all four redistributable
     plugins; `make check` green with `--with-oracle` active.
   
   `-Werror -Wall` clean; every function, parameter, struct member, and constant
   is Doxygen-documented in the house style.
   
   ## Related PRs
   
   Part of a three-repo change under GUACAMOLE-2064:
   
   - Server (this PR): _<fill in after opening>_
   - Client (web UI / forms / translations): _<fill in guacamole-client PR URL>_
   - Manual (documentation): _<fill in guacamole-manual PR URL>_
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GUACAMOLE-2064: Add interactive database terminal protocols [guacamole-server]

Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman commented on PR #694:
URL: https://github.com/apache/guacamole-server/pull/694#issuecomment-4995865198

   > > **⚠️ Open question for maintainers — OpenSSL in the Docker image.** The four
   > > database client packages on Alpine depend on `openssl-dev` (OpenSSL 3), which
   > > **cannot be installed alongside `openssl1.1-compat-dev`**, the package the
   > > image currently pins (the base image is held at Alpine 3.18 for that reason).
   > > This PR therefore switches the image to `openssl-dev`; guacd and its existing
   > > dependencies build and run against OpenSSL 3 in my testing (the full image
   > > builds and all existing protocols — RDP/VNC/SSH/telnet/Kubernetes — are
   > > present and load). I have **kept the 3.18 base pin** and only updated the
   > > comment that explained it, rather than bumping the base image, to keep this
   > > change minimal.
   > > **Context:** the `openssl1.1-compat-dev` pin comes from
   > > [GUACAMOLE-1741](https://issues.apache.org/jira/browse/GUACAMOLE-1741), where
   > > VeNCrypt VNC authentication broke against OpenSSL 3.0.8 due to a
   > > **libvncserver** incompatibility (not a guacd one). The Alpine 3.18 pin was
   > > then added in the still-open
   > > [GUACAMOLE-1891](https://issues.apache.org/jira/browse/GUACAMOLE-1891) because
   > > Alpine 3.19 dropped the compat package; Alpine 3.18 has since reached
   > > end-of-life (9 May 2025). That libvncserver blocker was **fixed upstream in
   > > [LibVNCServer 0.9.15](https://github.com/LibVNC/libvncserver/releases/tag/LibVNCServer-0.9.15)**
   > > (22 Dec 2024): _"Fixed building with OpenSSL >= 3.0.0."_ Since the Dockerfile
   > > already auto-selects the latest LibVNCServer release, the image is building
   > > 0.9.15+ today, so the original reason for the OpenSSL 1.1 pin appears to be
   > > stale. Moving to `openssl-dev` here is therefore a step toward what
   > > [GUACAMOLE-1891](https://issues.apache.org/jira/browse/GUACAMOLE-1891) needs, and that ticket looks close to resolvable.
   
   Assuming that all of the protocols build and run, and don't have any feature degredation with OpenSSL 3, I have no issue moving in that direction.
   
   > > **Why I did not also bump the base image off 3.18:** I tried building on
   > > `alpine:latest` (currently 3.24) and it is _not_ a self-contained change — it
   > > cascades into unrelated packaging churn: `webkit2gtk-dev` has been renamed
   > > (to `webkit2gtk-4.1-dev`), and CMake 4 on newer Alpine hard-rejects the older
   > > `cmake_minimum_required(VERSION < 3.5)` still declared by some bundled
   > > dependency sources (the libvncserver build fails to configure), with more
   > > likely behind it. That is a genuine base-image migration and belongs in
   > > [GUACAMOLE-1891](https://issues.apache.org/jira/browse/GUACAMOLE-1891), not in this feature PR, so I kept the 3.18 pin (Alpine 3.18
   > > still ships OpenSSL 3.1, so `openssl-dev` works there) and only switched the
   > > OpenSSL package.
   > > **The one thing I could not fully verify:** that VeNCrypt VNC auth works
   > > against libvncserver 0.9.15 + OpenSSL 3 (the exact scenario [GUACAMOLE-1741](https://issues.apache.org/jira/browse/GUACAMOLE-1741) was
   > > about). It is worth an explicit re-test before the pin is considered dead. If
   > > you would rather the Docker packaging (and thus the OpenSSL switch and that
   > > VeNCrypt re-test) be handled separately from this feature — e.g. under
   > > [GUACAMOLE-1891](https://issues.apache.org/jira/browse/GUACAMOLE-1891) — I'm happy to drop the Docker commit and file a follow-up; the
   > > protocols build fine from source regardless, so this only affects the
   > > convenience image. Please let me know which you'd prefer.
   > 
   
   Seems like maybe as one of the updates to dependencies we should resolve this and get Guacamole building successfully against later Alpine Linux versions.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GUACAMOLE-2064: Add interactive database terminal protocols [guacamole-server]

Posted by "mike-jumper (via GitHub)" <gi...@apache.org>.
mike-jumper commented on PR #694:
URL: https://github.com/apache/guacamole-server/pull/694#issuecomment-4973724094

   Haven't read through this yet, but in principle I like the idea. It's broad and would be an interesting addition to the suite of supported protocols.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] GUACAMOLE-2064: Add interactive database terminal protocols [guacamole-server]

Posted by "4m1nr (via GitHub)" <gi...@apache.org>.
4m1nr commented on PR #694:
URL: https://github.com/apache/guacamole-server/pull/694#issuecomment-5640988809

   While this has been waiting for review I have kept testing it against live
   servers of all five engines, and found **four bugs**, plus one cosmetic one that
   turned up while fixing them. All are now fixed on this branch, one commit each:
   
   | # | Bug | Fix |
   |---|-----|-----|
   | 1 | **C1 control characters were not sanitised from result cells.** `guac_dbshell_table_sanitize()` only replaced C0 and DEL, so U+009B (the 8-bit form of CSI, `C2 9B` in UTF-8) passed straight through. `libguac-terminal` decodes thatcodepoint and treats it as CSI, so a cell containing `C2 9B 32 4A` really did clear the screen: the exact injection class the sanitiser exists to stop, via the 8-bit form instead of `ESC [`. The MongoDB shell had the same gap because libbson only escapes C0 in its JSON output. | 9c3f62d8: cells and column names now replace U+0080 to U+009F as well (both bytes become one space, so column widths are unaffected); MongoDB output escapes them as `\u009b`, matching what libbson already does for C0. Unit test added. |
   | 2 | **`timeout` was not honoured by mssql and oracle against a silent peer** (TCP handshake completes, server never sends a byte). libpq, MariaDB Connector/C and libmongoc all gave up after `timeout` seconds; the SQL Server and Oraclesessions hung indefinitely. For SQL Server there were two causes on my side: the client-message callback returned `CS_SUCCEED` for the timeout message, which CT-Library defines as "keep waiting"; and because no TDS version was requested, FreeTDS's `auto` mode probes Sybase TDS 5.0 after 7.4 fails, and FreeTDS 1.3's TDS 5.0 disconnect path loops forever when its LOGOUT gets no reply. For Oracle, EZConnect syntax simply has no way to express a connect timeout. | a670c385: the callback returns `CS_FAIL` for timeout messages (FreeTDS message 20003, `TDSETIME`), TDS 7.4 is requested by default (the server negotiates down during login, so this is what `auto` did for every SQL Server anyway, minus the Sybase probe), and the Oracle driver connects with a fu
 ll connect descriptor carrying `(CONNECT_TIMEOUT=n)`. Both now fail with a clear message within the configured timeout, and the "no effect for Oracle" caveat in the manual PR is gone. |
   | 3 | **MongoDB commands could not be cancelled with Ctrl+C.** The driver had no `cancel_handler`, so a long `$where` scan ran to completion. | 947d1bdf: Ctrl+C now runs `currentOp` filtered on the session's own `connectionId` (taken from the `hello` reply at connect time) and `killOp`s the result, over a second short-lived connection, the same approach as the mysql driver's `KILL QUERY`. MongoDB lets users kill their own operations without the `killop` privilege; I verified this with a plain `readWrite` user. The interrupted command fails with code 11601 and the prompt returns immediately. |
   | 4 | **F1 to F5 leaked a letter into the input line under `terminal-type=linux`** (the default). The console encodes them as `ESC [ [ A` through `ESC [ [ E`; the parser took the second `[` as the CSI final byte and inserted the trailing letter as text. | da66a3c2: the parser recognises `ESC [ [` and consumes the following byte. Unit test added. |
   | 5 | Cosmetic: every SQL Server session began with `ERROR: Server name not found in configuration files.` FreeTDS raises it unconditionally when the server is addressed with `CS_SERVERADDR` instead of a `freetds.conf` entry, which is what this driver does on purpose. | 543e1f98: that one message is dropped in the callback. |
   
   The matching manual PR has a small follow-up commit
   (apache/guacamole-manual#299, e9889fa) removing the two limitations that no
   longer exist and documenting the TDS 7.4 default. `make check` is green with the
   two new unit tests.
   
   I am not a heavy database user, so a DBA may well find things I would not think to
   try, but this is everything I have found so far.
   
   I know this is a large change to get through. If there is anything I can do to
   make the review easier, I would be glad to help.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org