Repository navigation
feat(sources): hardened the s3, postgres and mysql sources - #16
Merged
Merged
Conversation
The s3 source captured only Content-Type besides user metadata and tags, so a restore dropped Content-Disposition, Cache-Control, Content-Encoding and Content-Language. Outline stores attachments of types a browser would render (HTML, SVG) with "Content-Disposition: attachment"; after a restore those files were rendered on Outline's origin instead of being downloaded. * metadata.json now records the four headers per object (null when the object has none) * restore passes them back to put_object * snapshots taken before this change lack the keys and restore as before, with Content-Type only
A single refused get_object_tagging call failed the whole s3 mirror:
credentials without s3:GetObjectTagging, or a store without object
tagging (NotImplemented), left the job with no copy of the bucket at
all although every object was readable.
* AccessDenied, NotImplemented, MethodNotAllowed, NotSupported,
UnsupportedOperation, XNotImplemented or a bare HTTP 403/405/501
now keep the object and record its tags as null in metadata.json
({} still means "no tags")
* the component carries one aggregated warning with the count and
the error codes, so the job ends in warning with an alert instead
of silently losing tags
* those objects restore without tags; any other tagging error (a
server error, a timeout) still fails the component
Restore built the Tagging parameter by joining key=value pairs without encoding. Tagging is a URL query string, so S3 and MinIO decode "+" as a space: a tag "lang=c++" came back as "lang=c ", and a space or "=" inside a key or value garbled the tag. The tags are now encoded with urlencode/quote, which round-trips every character S3 allows in a tag.
Self-hosted MinIO or Ceph endpoints often present a certificate from an internal CA. The s3 source and destination had no way to trust it other than the process-wide AWS_CA_BUNDLE variable, and no way to connect at all to an endpoint whose certificate cannot be verified. * verify_tls (default true) and ca_bundle (default null) on the s3 source and the s3 destination * ca_bundle points boto3 at a PEM file of CAs to trust; a missing file is a clear error instead of a TLS failure on the first call * verify_tls false switches verification off and logs a warning on every client build that names the endpoint and the risk * with neither set, boto3 receives verify=None as before, so its own default and AWS_CA_BUNDLE keep working unchanged * both configs now share S3ConnectionConfig and one client builder (net/s3.py) instead of two copies of the same boto3 setup Tests run real handshakes against a local HTTPS server with a throw-away CA: rejected by default, accepted with ca_bundle, accepted with verify_tls false.
A custom-format restore ran pg_restore --clean --if-exists in one
transaction. Its clean phase drops each partition's primary key on its
own, which PostgreSQL refuses ("cannot drop inherited constraint"),
so a restore into a database holding partitioned tables rolled back
completely. CS-IAM had to ship its own zitadel-postgres source for
Zitadel's partitioned cache tables; the engine now does it for every
consumer.
* before a custom restore, one catalog query lists the target's
top-level partitioned tables (NUL-separated, so any name parses)
* only when the dump recreates some of them (matched against
pg_restore --list), the restore runs as one psql
--single-transaction: DROP TABLE ... CASCADE for exactly those
tables, then the dump's own clean-and-create script, generated into
a file first so a half-written script is never executed
* any error rolls back everything, the drops included; psql's
"drop cascades to" notices are logged, and an error message shows
the ERROR lines rather than the notices before them
* without partitioned tables in the target, and for plain dumps, the
restore is exactly the pg_restore / psql call it was before
The postgres source dumped and restored with --no-acl. --clean drops and recreates every object, so after a restore an application that connects as a restricted runtime role lost its GRANTs and failed with "permission denied"; CS-IAMStack had to re-grant them in a post_restore hook. * keep_acl (default false) leaves out --no-acl on pg_dump (custom and plain) and on the restore, including the partition-safe path, so GRANT/REVOKE and ALTER DEFAULT PRIVILEGES survive the round trip * components dumped with it carry "acl": true in their manifest metadata, so show tells which snapshots hold privileges * the default is unchanged: --no-acl both ways, as before Roles are cluster objects and stay outside the dump; the docs spell out that every granted role must exist on the target.
The postgres table listed "database" as the default component name. The source names its component after the database (name, else database), and restore matches the component by that name, so the documented default sent operators looking for a database.dump that is never written. Also documented exclude_table_data, which the source accepts but the table did not list.
Alpine's mariadb package deletes every client authentication plugin from mariadb-client because mariadb-connector-c installs the same files into /usr/lib/mariadb/plugin, yet mariadb-client does not depend on it. The image therefore had none of them: a login to a MySQL user with the default caching_sha2_password failed with "Plugin caching_sha2_password could not be loaded", which rules out every default MySQL 8.4+ server (mysql_native_password is off in 8.4 and removed in 9.0) and MariaDB users on ed25519 or PARSEC. The image now installs mariadb-connector-c next to mariadb-client, in the runtime and the test stage. It adds the caching_sha2_password, sha256_password, client_ed25519, parsec and dialog plugins; a mysql_native_password user keeps working as before.
scripts/e2e.sh now covers what the unit tests can only fake, against real servers and through the engine image's own clients: * postgres: a partitioned table and a restricted runtime role with grants; restore into an empty database, in place over the live partitioned table, and with keep_acl (grants kept, "acl" marked in the manifest); the default still drops the grants * mariadb/mysql: 4-byte UTF-8 text and a trigger survive the round trip; mysql logs in as a fresh caching_sha2_password user after FLUSH PRIVILEGES, so every login takes the full exchange instead of the healthcheck-warmed auth cache, and the compose service no longer forces mysql_native_password (gone in MySQL 9) * s3: content headers, user metadata and URL-special tags come back (scripts/e2e_s3.py checks them); a user without s3:GetObjectTagging still backs up every object, ends in warning and restores them untagged * E2E_POSTGRES_IMAGE / E2E_MARIADB_IMAGE / E2E_MYSQL_IMAGE swap the server images for one run without touching the Dependabot-tracked compose file; the run prints server and client versions * failed checks print the command output, a failed run the service logs Credentials of new users are generated per run; existing dev values are reused from the script's own variables.
The e2e matrix (scripts/e2e.sh) existed but no workflow ran it, so a restore path broken against a real server - partitioned tables, a MySQL default login - reached a release unnoticed. * new job "E2E Round Trip" in docker-release.yml, three legs: compose (the Dependabot-tracked images of docker-compose.e2e.yml), lts (mariadb 11.4, mysql 8.4) and latest (newest mariadb/mysql) * the release job now needs it, next to pytest and the validations * pull requests touching scripts/ or this workflow run it too * least privilege (contents: read), 30-minute timeout
karlspace
changed the title
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
CodeQL flagged the throw-away HTTPS server of the TLS verification tests (py/insecure-protocol): ssl.PROTOCOL_TLS_SERVER still allows TLS 1.0/1.1. The context now sets minimum_version to TLS 1.2; the tests are unchanged otherwise.
mariadb-dump takes every server version from 10.3 up for MariaDB and, with --routines, asks for MariaDB packages (SHOW PACKAGE STATUS). MySQL's calendar versions (26.x) are above that, so the e2e run against mysql:26.7 failed the whole dump with "Couldn't execute 'SHOW PACKAGE STATUS ...' (1064)" - no MySQL 26 backup was possible. * before each dump the source reads SELECT VERSION(); a failed probe keeps the single pass, which then reports the real error * on MySQL 10+ (no "-MariaDB" in the version) the main pass runs with --skip-routines, and a second pass (--routines --no-create-info --no-data --force) adds functions and procedures; only the package query error is accepted there (exit code 2), anything else fails the component * both passes land in the same .sql.gz and restore as one; the component metadata says "routines": "separate pass (MySQL 26+)" * MariaDB and MySQL 8/9 keep the single pass unchanged The docs also note that a restore needs a user allowed to recreate objects for their DEFINER.
The first CI run of the e2e matrix showed two problems in the script itself: * minio/mc no longer exists on Docker Hub, so every "archive in MinIO" check and the creation of the user without s3:GetObjectTagging failed; mc now runs from the CS-MinIO init image, which ships it, and a failed check prints the listing * the MariaDB trigger was seeded by root but restored by the application user, which may not recreate objects for another DEFINER; the extras are now created by the application user, as an application would The MariaDB and MySQL round trips also check a stored procedure now, which exercises the MySQL 26+ routines pass, and a DB restore must report "restore complete".
README, sources, deployment and restore still claimed "MariaDB 11/12, MySQL 8/9" without any test behind it, and the restore guide said the database paths were covered by unit tests only. They now name what CI actually round-trips through the engine image (MariaDB 11.4, 11.8 and 13; MySQL 8.0, 8.4 and 26.7; PostgreSQL 18 with partitioned tables and grants), and the README explains how to run scripts/e2e.sh locally.
This was referenced Oct 9, 2026
On MySQL 26+ the routines come from a second mariadb-dump pass run with --force. That pass was accepted unless a stderr line other than the package query said "error" or "couldn't". A routine whose body the backup user may not read (MySQL hides it from a user holding only EXECUTE or ALTER ROUTINE) is reported as "<user> has insufficient privileges to SHOW CREATE PROCEDURE ..." - neither word - and --force leaves it out. The backup then ended in success without the routine, while the single pass on every other server fails the component for the same condition. The check now fails closed: the pass is accepted only if every stderr line is the MariaDB package query or one of the client's notices (the TLS notice for a password from MYSQL_PWD, the deprecated mysqldump name). Any other report fails the component with the dump's stderr. The new regression tests for the insufficient-privileges and the transaction report failed against the old check. docs/sources.md states the rule.
The second mariadb-dump pass on MySQL 26+ ran without --single-transaction. mariadb-dump then keeps its default lock_tables and issues LOCK TABLES ... READ LOCAL on every table of each dumped database, even with --no-data, and holds the locks until the routines are written. On a busy InnoDB database that blocks writers for the length of the pass, and a least-privilege backup user without the LOCK TABLES grant - fine for the main pass - failed the whole component with error 1044. The pass now runs with --single-transaction like the main pass: no table locks, and the routines are read from one consistent snapshot. A unit test asserts the flag; docs/sources.md describes the second pass with its real flags and says it needs no LOCK TABLES privilege.
The MySQL round trip ran only as a user with ALL ON *.*, so it could not show that a dump needs no LOCK TABLES privilege, or that a routine the backup user may not read fails the backup - the two defects just fixed in the MySQL 26+ routines pass. Two more users now back up the same database on every leg: * bk_min (SELECT, SHOW VIEW, TRIGGER, EVENT on the database and SHOW_ROUTINE, no LOCK TABLES): the backup must succeed, and restoring it must bring the procedure back. * bk_exec (the same with EXECUTE instead of SHOW_ROUTINE): MySQL hides the procedure body, so the backup must end in error with "insufficient privileges" in its manifest. The error text goes to the log as evidence of what mariadb-dump reported. Passwords are generated at run time, as for the existing bk user.
The mysql section never said which grants a dedicated backup user needs, and the two routines fixes change the answer for MySQL 26+: no pass needs LOCK TABLES any more, and a routine whose body the user may not read now fails the dump instead of vanishing from it. The new "Backup user" paragraph lists the grants (SELECT, SHOW VIEW, TRIGGER, EVENT; SHOW_ROUTINE or global SELECT for routine bodies), explains the hidden-body case and points at the e2e users that check both sides.
sources, deployment and restore listed the exact MariaDB and MySQL versions CI round-trips (11.4, 11.8, 13; 8.0, 8.4, 26.7). Two of them only come from docker-compose.e2e.yml, which Dependabot moves: once the open bumps to MariaDB 13 and MySQL 26.7 are merged, CI no longer runs 11.8 or 8.0 and the docs would claim coverage that is gone. They now name what each leg runs - the versions in docker-compose.e2e.yml, the LTS lines MariaDB 11.4 and MySQL 8.4 (pinned in the workflow) and the newest releases - which stays true whatever Dependabot merges. Every e2e run logs the exact versions.
A restore over live partitioned tables first writes the dump's clean-and-create script to a file, so that psql never commits a truncated script. That file is the whole database as uncompressed SQL and lands in the restore's temporary directory - /tmp, which the recommended hardening mounts as a tmpfs, i.e. memory. The docs did not say so; an operator could only find out when a large restore ran out of space. The partitioned-tables paragraph now states the space it needs and that script generation and apply each have the 4-hour restore timeout.
With verify_tls false the docs said urllib3 "adds an InsecureRequestWarning", which reads like a single line. urllib3 emits it for every request: five head_bucket calls against a local HTTPS server gave five warnings. The s3 source makes two requests per object (get_object, get_object_tagging), so a mirror floods the log. The TLS section now says so. Suppressing the warning in the engine was left out on purpose: it is process-wide, and the engine already logs its own warning naming each unverified endpoint once per run.
karlspace
changed the title
Contributor
|
🎉 This PR is included in version 1.9.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Gaps in the engine's built-in sources, found in the consumers or in the first real-database CI run of this PR:
Content-Typewas captured.Content-Disposition,Cache-Control,Content-EncodingandContent-Languagewere lost on restore. CS-Outline stores attachments of types a browser would render (HTML, SVG) withContent-Disposition: attachment; after a restore they were rendered on Outline's origin instead of being downloaded.s3:GetObjectTagging, or a store without object tagging (NotImplemented), left the job with no copy of the bucket at all.Taggingis a query string, solang=c++came back aslang=c(reproduced with moto).AWS_CA_BUNDLE.pg_restore --cleandrops each partition's primary key on its own, and PostgreSQL refuses (cannot drop inherited constraint). CS-IAM ships thezitadel-postgresplugin only for this; CS-BillingStack runspostgresql-partman.--no-acl, so a restricted runtime role lost its GRANTs (CS-IAMStack re-grants them in apost_restorehook).mariadb-client, and the image did not installmariadb-connector-c, which provides them. Logins withcaching_sha2_passwordfailed, and that is the only default on MySQL 8.4+ and 9+.mariadb-dumptakes any server version ≥ 10.3 for MariaDB and runsSHOW PACKAGE STATUS, which fails on MySQL 26.7 with error 1064. Seen in run 1, latest leg.scripts/e2e.sh. Itsminio/mcimage no longer exists on Docker Hub (404), so every "archive in MinIO" check failed in run 1.docs/sources.mdgave"database"as the postgresnamedefault. The code uses the database name.--forceand accepted every stderr line that did not say "error" or "couldn't". A routine whose body the backup user may not read is reported only as<user> has insufficient privileges to SHOW CREATE PROCEDURE …, so the backup ended insuccesswithout it.--single-transaction, mariadb-dump runsLOCK TABLES … READ LOCALon each table of the database, even with--no-data. That blocks writers, and a backup user without theLOCK TABLESgrant (fine for the main pass) would fail the component with error 1044 (from the mariadb-dump source; not run).Change
fix(sources): preserved content headers of mirrored s3 objectsmetadata.json(null when absent) and are re-applied byput_object. Older snapshots restore as before.fix(sources): kept s3 objects whose tags cannot be readAccessDenied/NotImplemented/MethodNotAllowed/NotSupported/ … or a bare HTTP 403/405/501, the object is kept withtags: null. The component carries one aggregated warning, so the job ends inwarningwith an alert. These objects restore without tags. Any other tagging error still fails the component.fix(sources): url-encoded s3 object tags on restoreurlencode(..., quote_via=quote)feat(s3): added TLS verification settings for s3 endpointsverify_tls(defaulttrue) andca_bundle(defaultnull) on the s3 source and destination, with a sharedS3ConnectionConfigand client builder (net/s3.py). When neither is set, boto3 getsverify=Noneas before, soAWS_CA_BUNDLEstill applies.verify_tls: falselogs a warning naming the endpoint on every build. The security warning is in the docs.fix(sources): restored postgres dumps over partitioned tablespsql --single-transaction:DROP TABLE … CASCADEfor exactly those tables, then the dump's own clean-and-create script (generated into a file first). Any error rolls back everything. CASCADE notices are logged. Without partitioned tables, and for plain dumps, the restore runs the samepg_restore/psqlcommand as before.feat(sources): added an opt-in to keep postgres privilegeskeep_acl(defaultfalse) drops--no-aclfrom dump and restore. Such components carry"acl": truein their manifest metadata.docs(sources): corrected the postgres name defaultexclude_table_data.fix(sources): shipped the client authentication pluginsmariadb-connector-cin the runtime and test stages, which adds the caching_sha2_password, sha256_password, client_ed25519, parsec and dialog plugins.fix(sources): dumped mysql 26+ routines in a second passSELECT VERSION()probe runs first. On MySQL 10+ (no-MariaDBin the version), the main pass uses--skip-routinesand a second pass (--routines --no-data --no-create-info --force) adds functions and procedures. In that second pass the package-query error is the only one accepted. Both passes go into one.sql.gz. MariaDB and MySQL 8/9 keep the single pass.test: …(3)ci: ran the e2e round trips against real databases🧪 E2E Round Tripwith legscompose(Dependabot-tracked images),lts(mariadb 11.4, mysql 8.4) andlatest. The release job needs it. Least privilege, 30-minute timeout.docs: listed the database versions the e2e round trips coverfix(sources): failed the MySQL 26+ dump on unreadable routinesMYSQL_PWD, the deprecatedmysqldumpname). Anything else fails the component.fix(sources): kept the MySQL 26+ routines pass free of table locks--single-transactionin the second pass too: no table locks, and the routines come from one consistent snapshot.test: covered least-privilege MySQL backup users in the e2ebk_min(SELECT, SHOW VIEW, TRIGGER, EVENT+SHOW_ROUTINE, noLOCK TABLES) must back up and restore the procedure.bk_exec(EXECUTE, noSHOW_ROUTINE) must fail with "insufficient privileges".docs(sources): listed the privileges a MySQL backup user needsdocs: described the e2e coverage per CI leg instead of versionsdocs(sources): noted the temp space of a partitioned-table restore/tmp(atmpfsin the recommended hardening).docs(destinations): noted the per-request urllib3 TLS warningverify_tls: false, urllib3 warns on every request (about two per object for ans3source).How verified
🧪 Pytest). New tests:ca_bundle, accepted withverify_tls: falsekeep_aclon every path--single-transaction. The insufficient-privileges, transaction and lock-flag tests failed against 037d87d.keep_acl, a restricted role keepsSELECT/INSERTcaching_sha2_passworduser afterFLUSH PRIVILEGES, so every login takes the full exchangeLOCK TABLES(bk_min) backs up, and its snapshot restores the procedure, on MySQL 8.0.46, 8.4.11 and 26.7.0 (on 26.7 that includes the second pass)EXECUTE(bk_exec) ends inerroron all three. The leg logs show the real stderr; on 26.7.0 it is the deprecated-name notice, the TLS notice,mysqldump: bk_exec has insufficient privileges to SHOW CREATE PROCEDURE `demo_count`!and the package error. Fed to the classifier of 037d87d, these lines are accepted (the procedure would have been dropped silently); the new classifier rejects them.s3:GetObjectTaggingbacks up every object, ends inwarningwithget_object_tagging: AccessDenied, and restores untaggedSHOW PACKAGE STATUSfailure, the missingminio/mcimage and a trigger seeded with a root DEFINER (test setup only, docs now explain the DEFINER rule). The run also showed that the plugin fix works: MySQL 8.0 and 8.4caching_sha2_passwordround trips already passed there.-S warning, yamllint relaxed, anddocker compose -f docker-compose.e2e.yml -f <override> config(render only).Not verified
LOCK TABLESwas not run. It follows from mariadb-dump 11.8 (lock_tables=1by default, cleared only by--single-transactionor--lock-all-tables;LOCK TABLES … READ LOCALindump_all_tables_in_db). The fixed pass was run with such a user.keep_acland the partitioned-table restore are not verified as a non-superuser owner.Impact / upgrade steps for existing deployments
keep_acl: false,verify_tls: true,ca_bundle: null, and the single-pass dump for MariaDB and MySQL 8/9.tags: nullfor objects whose tags could not be read. Jobs whose credentials lacks3:GetObjectTaggingmove fromerror(no snapshot) towarning(snapshot without tags).SELECT VERSION()). The image grows bymariadb-connector-c(404 KiB installed on Alpine 3.24).LOCK TABLES, but must be able to read routine bodies (SHOW_ROUTINEor globalSELECT, or be the definer). Otherwise the component fails with "insufficient privileges", as the single pass on MySQL 8/9 always did.zitadel-postgresback totype: postgres.keep_acl: truefor the grants, but still needs its hook to create the roles and passwords on a new host.feat, matching the two new options) or rebase-merge. 2b45e27 and e470f49 add the e2e gate before the fixes that make it pass, so a rebase-merge puts those two red commits on main.latestleg, a new MariaDB or MySQL release the engine cannot back up blocks the next engine release. If that is unwanted, mark the legcontinue-on-error.Dependabot PRs (evaluated with this PR's e2e, not merged)
chore(deps): bump mariadb from 11 to 13 #7 mariadb 11 → 13: safe to merge. MariaDB 13.0.2 (the same image
mariadb:13resolves to) passed the full round trip with the engine's mariadb-dump 11.8.8 in thelatestleg. This includes utf8mb4 text, a trigger and a procedure. Merge after this PR, so the rebased Dependabot PR runs thecomposeleg on 13 as well.chore(deps): bump mysql from 8.0 to 26.7 #6 mysql 8.0 → 26.7: not safe before this PR. Engine 1.7.7 cannot back up MySQL 26.7 for three reasons:
caching_sha2_passwordpluginmariadb-dumpfails onSHOW PACKAGE STATUS(see run 1, latest leg)--default-authentication-plugin, which MySQL 8.4 removed according to its release notes (not run here)With this PR, MySQL 26.7.0 passes the full round trip (latest leg). It is safe to merge after this PR, once Dependabot has rebased it and its
composee2e leg is green.Follow-ups
zitadel-postgresplugin withtype: postgresonce this is released, and keep its round trip as proof.keep_acl: true. The role/password repair hook stays.option --ssl-verify-server-cert is disabled, because of an insecure passwordless login. It does not see theMYSQL_PWDpassword, so server certificates are not verified. Averify_tls/ca_bundlepair for the MySQL-family sources (like S3) is a separate change. A future client that verifies here could also refuse the plain-textcaching_sha2_passwordexchange over an unverified connection.>= 100300version checks misfire on MySQL's calendar versions.composeandltsas the gate and runlatestas a non-gating job with an alert.verify_tls: falseis used in earnest: suppress urllib3's per-requestInsecureRequestWarningafter the engine's own warning (process-wide, so left out here).Content-Encodingis replayed verbatim; consider droppingaws-chunkedshould a store ever report it."<schema> <name> "); a table named likeevents xnext toeventscould match the wrong entry (same limitation as the CS-IAM plugin).