From 4ffc434b1ba621ed5767f06f534bf3861e568055 Mon Sep 17 00:00:00 2001 From: Reactorcoremeltdown Date: Mon, 2 Nov 2026 15:38:32 +0100 Subject: [PATCH] Keep server data in an encrypted database, and harden every part Storage. The server keeps its data in SQLite (a file in the data directory, the default) or PostgreSQL (database, KEEPORG_DATABASE_URL or -database). Every row, the KDBX versions included, is sealed with AES-256-GCM under a key derived from the master key and named by an HMAC. The master key is required to start and a different one is refused. A data directory from 0.10 or earlier is imported on first start. Several servers may share one PostgreSQL database. Go goes to 1.26 for the pure-Go drivers (modernc.org/sqlite, jackc/pgx). Hardening, after an audit of the server, both clients and the deployment: - Single sign-on approves a device only after an explicit Approve in the browser that started it, bound by a cookie and PKCE; identities are never linked by username, by verified email only when allowed, never to admins. - Sign-in attempts are limited per address and per account, passwords are checked outside the store lock with uniform timing, one-time codes work once, and starting two-factor needs the password. - Merge vaults bound KDF parameters, payload size and XML shape, check pushes before storing them, and run at most two merges at once. A version is written in the same transaction as its metadata. - Tokens in URLs only on the event stream, which ends on revocation; CSP, frame-ancestors, no-store and HSTS; sign-in CSRF; 404 for databases without access; new trusted_proxies. - Clients open only the server's approval page, keep case-variant and retired file names apart, never overwrite a save made during a pull, require https except locally, send the stream token in a header, bound their reads and escape notifications and terminal output. - The .deb creates the master key, systemd is sandboxed further, nginx sets modern TLS and HSTS, and CI deploys from main only. Server 0.12.0, desktop 0.6.0, Android 0.4.0 (versionCode 4). Co-Authored-By: Claude Opus 5.5 --- .dockerignore | 3 +- .woodpecker/build.yaml | 3 + .woodpecker/deploy.yaml | 3 + Dockerfile | 29 +- README.md | 25 +- .../cc/tiredsysadmin/android/keeporg/Api.kt | 33 +- .../android/keeporg/KeepOrgProvider.kt | 21 +- .../android/keeporg/LoginActivity.kt | 15 + .../tiredsysadmin/android/keeporg/Mirror.kt | 91 +++- .../cc/tiredsysadmin/android/keeporg/Prefs.kt | 9 + .../cc.tiredsysadmin.android.keeporg.yml | 12 +- cmd/keeporg/agent.go | 3 + cmd/keeporg/agent_test.go | 8 + cmd/keeporg/autostart_darwin.go | 12 +- cmd/keeporg/autostart_xdg.go | 29 +- cmd/keeporg/autostart_xdg_test.go | 13 + cmd/keeporg/commands.go | 27 +- cmd/keeporg/desktop.go | 36 +- cmd/keeporgd/main.go | 57 ++- config/nginx/keeporg.conf.tmpl | 11 +- config/variables/server.values.yaml | 4 + docs/HANDOVER.md | 364 +++++++++++---- docs/PROTOCOL.md | 16 +- go.mod | 23 +- go.sum | 80 +++- internal/blob/blob.go | 180 +++++--- internal/blob/blob_test.go | 70 ++- internal/client/admin.go | 8 +- internal/client/api.go | 20 +- internal/client/config.go | 34 ++ internal/client/config_test.go | 42 ++ internal/client/mirror.go | 102 ++++- internal/client/mirror_names_test.go | 90 ++++ internal/client/sse.go | 9 + internal/kdbx/crypto.go | 3 + internal/kdbx/kdbx.go | 76 +++- internal/kdbx/limits.go | 60 +++ internal/kdbx/limits_test.go | 51 +++ internal/kdbx/xmltree.go | 10 + internal/secret/keyring.go | 53 ++- internal/secret/keyring_test.go | 21 + internal/server/account.go | 66 ++- internal/server/account_test.go | 74 ++- internal/server/admin.go | 140 +++--- internal/server/auth.go | 131 +++--- internal/server/authn.go | 353 +++++++++++++++ internal/server/authn_test.go | 82 ++++ internal/server/dataplane_test.go | 110 +++++ internal/server/delete_test.go | 40 +- internal/server/integration_test.go | 55 ++- internal/server/merge.go | 34 +- internal/server/oidc.go | 261 +++++++++-- internal/server/oidc_test.go | 174 +++++-- internal/server/pages.go | 21 +- internal/server/password_test.go | 11 +- internal/server/server.go | 111 ++++- internal/server/storage.go | 101 +++++ internal/server/storage_test.go | 99 ++++ internal/server/sync.go | 40 +- internal/server/vault.go | 15 +- internal/server/web.go | 67 ++- internal/server/web_test.go | 64 ++- internal/server/webaccount.go | 76 +++- internal/server/webadmin.go | 12 +- internal/sqldb/sqldb.go | 280 ++++++++++++ internal/sqldb/sqldbtest/sqldbtest.go | 51 +++ internal/store/acl.go | 4 +- internal/store/model.go | 35 +- internal/store/store.go | 424 +++++++++++++++--- internal/store/store_test.go | 162 ++++++- internal/totp/totp.go | 16 +- packaging/deb/client/changelog | 13 + packaging/deb/server/changelog | 33 ++ packaging/docker/compose.yaml | 11 +- packaging/systemd/keeporgd.service | 16 +- scripts/build-deb.sh | 16 +- versions.mk | 14 +- 77 files changed, 4192 insertions(+), 776 deletions(-) create mode 100644 cmd/keeporg/autostart_xdg_test.go create mode 100644 internal/client/mirror_names_test.go create mode 100644 internal/kdbx/limits.go create mode 100644 internal/kdbx/limits_test.go create mode 100644 internal/server/authn.go create mode 100644 internal/server/authn_test.go create mode 100644 internal/server/dataplane_test.go create mode 100644 internal/server/storage.go create mode 100644 internal/server/storage_test.go create mode 100644 internal/sqldb/sqldb.go create mode 100644 internal/sqldb/sqldbtest/sqldbtest.go diff --git a/.dockerignore b/.dockerignore index e64bec4..e0465d5 100644 --- a/.dockerignore +++ b/.dockerignore @@ -1,6 +1,7 @@ -# The image is built from go.mod, cmd/keeporgd and internal only. Everything +# The image is built from go.mod, go.sum, cmd/keeporgd and internal only. Everything # else just makes the build context bigger. * !go.mod +!go.sum !cmd/keeporgd/ !internal/ diff --git a/.woodpecker/build.yaml b/.woodpecker/build.yaml index 3587994..53f6c8d 100644 --- a/.woodpecker/build.yaml +++ b/.woodpecker/build.yaml @@ -9,6 +9,9 @@ when: - 'Dockerfile' - '**/*.go' event: [push, cron, manual] + # These steps run as root on the build and production hosts: only the + # main branch may reach them. + branch: main labels: role: production skip_clone: true diff --git a/.woodpecker/deploy.yaml b/.woodpecker/deploy.yaml index ace9ce9..3c54b98 100644 --- a/.woodpecker/deploy.yaml +++ b/.woodpecker/deploy.yaml @@ -9,6 +9,9 @@ when: - 'Dockerfile' - '**/*.go' event: [push, cron, manual] + # These steps run as root on the build and production hosts: only the + # main branch may reach them. + branch: main labels: role: homeserver depends_on: diff --git a/Dockerfile b/Dockerfile index 3c7fc40..9b09d76 100644 --- a/Dockerfile +++ b/Dockerfile @@ -2,9 +2,10 @@ # KeepOrg synchronization server. # -# The server is a static binary with no third-party dependencies and no cgo, -# so the runtime image carries the binary, a certificate bundle for talking -# to an OIDC provider, and nothing else. There is no shell in it: the binary +# The server is a static binary with no cgo — its only third-party code is +# the two pure-Go database drivers — so the runtime image carries the +# binary, a certificate bundle for talking to an OIDC provider or a +# PostgreSQL server over TLS, and nothing else. There is no shell in it: the binary # probes its own health. # # podman build --format docker -t keeporgd: --build-arg VERSION= . @@ -15,14 +16,15 @@ # # or `make docker`, which takes the version from versions.mk. -FROM golang:1.24-alpine AS build +FROM golang:1.26-alpine AS build ARG VERSION=dev WORKDIR /src # Only what the server is built from, so a change to the clients, the # Android app or the docs does not invalidate this layer. -COPY go.mod ./ +COPY go.mod go.sum ./ +RUN go mod download COPY cmd/keeporgd ./cmd/keeporgd COPY internal ./internal @@ -46,14 +48,15 @@ LABEL org.opencontainers.image.title="KeepOrg server" \ org.opencontainers.image.licenses="AGPL-3.0-or-later" \ cc.tiredsysadmin.application="cc.tiredsysadmin.linux.keeporgd" -# Needed to reach an OIDC provider over TLS. Nothing else from the builder +# Needed to reach an OIDC provider, or PostgreSQL, over TLS. Nothing else from the builder # comes along. COPY --from=build /etc/ssl/certs/ca-certificates.crt /etc/ssl/certs/ca-certificates.crt -# The data directory holds state.json and the KDBX blobs. The configuration -# directory holds server.json and, unless the key is supplied through -# KEEPORG_MASTER_KEY, the master key that unwraps the vault keys. They are -# separate on purpose: whoever holds both can read every merge vault. +# The data directory holds the SQLite database, unless KEEPORG_DATABASE_URL +# points at PostgreSQL. Every row in it is sealed under the master key. The +# configuration directory holds server.json and, unless the key is supplied +# otherwise, that master key. They are separate on purpose: whoever holds +# both can read everything, and either alone is useless. # # Give the master key somewhere durable, one of these three ways: # @@ -67,9 +70,9 @@ COPY --from=build /etc/ssl/certs/ca-certificates.crt /etc/ssl/certs/ca-certifica # a secret that failed to mount is reported instead of being replaced. # # With none of the three, the server mints a key into the container's own -# layer on first start and loses it on the next `run`. It says so loudly at -# startup, and merging is all that stops working: clients still open their -# vaults. +# layer on first start and loses it on the next `run`, after which it +# refuses to start: the database does not open with a new key. Nothing is +# overwritten; mount the original key and it starts. COPY --from=build --chown=65532:65532 /out/var/lib/keeporg /var/lib/keeporg COPY --from=build --chown=65532:65532 /out/etc/keeporg /etc/keeporg VOLUME ["/var/lib/keeporg", "/etc/keeporg"] diff --git a/README.md b/README.md index bb58fc7..ba6adf6 100644 --- a/README.md +++ b/README.md @@ -25,11 +25,12 @@ keep their key off the server entirely, exactly as before. - **Android client**: the same, published to every file picker as a storage provider. -No FUSE, and the server has no third-party dependencies and no cgo. The -KDBX 4 codec, including ChaCha20, BLAKE2b and Argon2, is part of the -repository and is tested against both the relevant RFCs and a real -KeePassXC. The desktop client's one dependency is `fyne.io/systray`, for the -tray icon. +No FUSE and no cgo. The server keeps everything in SQLite or PostgreSQL, +every row sealed under a master key the database never sees, and its only +third-party code is the two pure-Go database drivers. The KDBX 4 codec, +including ChaCha20, BLAKE2b and Argon2, is part of the repository and is +tested against both the relevant RFCs and a real KeePassXC. The desktop +client's one dependency is `fyne.io/systray`, for the tray icon. ## Quick start @@ -97,7 +98,7 @@ Or open `http://127.0.0.1:8765/web` and create the vault from the browser. ```sh make deb # Debian packages for the client and the server -make docker # server container image, scratch-based, about 9 MB +make docker # server container image, scratch-based, about 25 MB make dmg # macOS disk image with an /Applications symlink (on macOS) make windows # Windows zip: keeporg-tray.exe, keeporg.exe and a Task Scheduler job make android # unsigned release APK (needs the Android SDK) @@ -117,9 +118,11 @@ podman run -d -p 8765:8765 \ keeporgd:latest -addr :8765 -data /var/lib/keeporg -url https://keeporg.example.com ``` -Keep that master key: it unwraps the vault keys, and without it the server -can no longer merge. `KEEPORG_MASTER_KEY` takes the base64 key directly if -your secret store injects variables instead of files. See +Keep that master key: everything in the database is encrypted under it, +and the server does not start without it. `KEEPORG_MASTER_KEY` takes the +base64 key directly if your secret store injects variables instead of +files. The data goes to a SQLite file on the volume; to use PostgreSQL +instead, add `-e KEEPORG_DATABASE_URL=postgres://keeporg:…@db/keeporg`. See [packaging/docker/compose.yaml](packaging/docker/compose.yaml). Server environment variables: @@ -127,8 +130,10 @@ Server environment variables: | Variable | Meaning | | --- | --- | | `KEEPORG_MASTER_KEY` | the master key, base64; overrides the other two sources | -| `KEEPORG_MASTER_KEY_FILE` | path to a file holding the master key (base64 or 32 raw bytes); never created, and if it is missing the server runs but cannot create or merge vaults | +| `KEEPORG_MASTER_KEY_FILE` | path to a file holding the master key (base64 or 32 raw bytes); never created, and the server does not start without it | | `KEEPORG_EXTERNAL_URL` | the public base URL (`external_url`); `-url` overrides it | +| `KEEPORG_DATABASE_URL` | `postgres://…` or `sqlite:` (`database`); default a SQLite file in the data directory; `-database` overrides it | +| `KEEPORG_TRUSTED_PROXIES` | reverse proxies whose `X-Forwarded-For` is believed, for per-client rate limits (`trusted_proxies`) | | `KEEPORG_ADMIN_PASSWORD` | the first `admin` password, read only on an empty server and by `-reset-admin` | Everything else is configured in `server.json` or by flag. The details, diff --git a/android/app/src/main/java/cc/tiredsysadmin/android/keeporg/Api.kt b/android/app/src/main/java/cc/tiredsysadmin/android/keeporg/Api.kt index c707aca..d009c74 100644 --- a/android/app/src/main/java/cc/tiredsysadmin/android/keeporg/Api.kt +++ b/android/app/src/main/java/cc/tiredsysadmin/android/keeporg/Api.kt @@ -132,7 +132,7 @@ class Api(baseUrl: String, var token: String?) { val status = c.responseCode if (status == 204) return Pair(ByteArray(0), 0L) if (status != 200) throw errorOf(c, status) - val data = c.inputStream.use { it.readAllBytesCompat() } + val data = c.inputStream.use { it.readAllBytesCompat(MAX_PAYLOAD) } val version = c.getHeaderField("ETag")?.trim('"')?.toLongOrNull() ?: 0L return Pair(data, version) } finally { @@ -150,7 +150,7 @@ class Api(baseUrl: String, var token: String?) { c.outputStream.use { it.write(data) } val status = c.responseCode if (status != 200) throw errorOf(c, status) - val o = JSONObject(c.inputStream.use { it.readAllBytesCompat() }.decodeToString()) + val o = JSONObject(c.inputStream.use { it.readAllBytesCompat(MAX_JSON) }.decodeToString()) return PushResult( o.getLong("version"), o.optString("sha256"), @@ -169,10 +169,12 @@ class Api(baseUrl: String, var token: String?) { * stream costs a delay, never data. */ fun events(onEvent: (JSONObject) -> Unit) { - val url = URL("$base/api/v1/events?access_token=" + enc(token ?: "")) - val c = url.openConnection() as HttpURLConnection + // The token goes in the header, as everywhere else: in the URL it + // would land in proxy logs. + val c = URL("$base/api/v1/events").openConnection() as HttpURLConnection c.requestMethod = "GET" c.setRequestProperty("Accept", "text/event-stream") + token?.takeIf { it.isNotEmpty() }?.let { c.setRequestProperty("Authorization", "Bearer $it") } c.connectTimeout = 15_000 // The server sends a keepalive comment every 25 seconds. c.readTimeout = 70_000 @@ -182,6 +184,11 @@ class Api(baseUrl: String, var token: String?) { var data = StringBuilder() while (true) { val line = r.readLine() ?: return + // Events are a few hundred bytes; a stream that keeps + // growing one is not a KeepOrg server. + if (line.length > MAX_JSON || data.length > MAX_JSON) { + throw IOException("event stream: an event exceeds 1 MiB") + } when { line.isEmpty() -> { if (data.isNotEmpty()) { @@ -231,7 +238,7 @@ class Api(baseUrl: String, var token: String?) { val status = c.responseCode if (status < 200 || status > 299) throw errorOf(c, status) if (status == 202) throw ApiException(202, "pending", "pending") - val text = c.inputStream.use { it.readAllBytesCompat() }.decodeToString() + val text = c.inputStream.use { it.readAllBytesCompat(MAX_JSON) }.decodeToString() return if (text.isBlank()) JSONObject() else JSONObject(text) } finally { c.disconnect() @@ -240,7 +247,7 @@ class Api(baseUrl: String, var token: String?) { private fun errorOf(c: HttpURLConnection, status: Int): ApiException { val text = try { - (c.errorStream ?: c.inputStream)?.use { it.readAllBytesCompat() }?.decodeToString() ?: "" + (c.errorStream ?: c.inputStream)?.use { it.readAllBytesCompat(MAX_JSON) }?.decodeToString() ?: "" } catch (_: IOException) { "" } @@ -257,13 +264,23 @@ class Api(baseUrl: String, var token: String?) { private fun enc(s: String): String = URLEncoder.encode(s, "UTF-8") } -/** readAllBytes() needs API 33; this works on every supported release. */ -internal fun InputStream.readAllBytesCompat(): ByteArray { +/** One database download, at most; the server's own limit is far below. */ +internal const val MAX_PAYLOAD = 256 * 1024 * 1024 + +/** One JSON answer or event, at most. */ +internal const val MAX_JSON = 1024 * 1024 + +/** + * readAllBytes() needs API 33; this works on every supported release, and + * refuses to hold more than [limit] bytes of whatever the server sends. + */ +internal fun InputStream.readAllBytesCompat(limit: Int): ByteArray { val out = ByteArrayOutputStream() val buf = ByteArray(16 * 1024) while (true) { val n = read(buf) if (n < 0) break + if (out.size() + n > limit) throw IOException("the server sent more than ${limit shr 20} MiB") out.write(buf, 0, n) } return out.toByteArray() diff --git a/android/app/src/main/java/cc/tiredsysadmin/android/keeporg/KeepOrgProvider.kt b/android/app/src/main/java/cc/tiredsysadmin/android/keeporg/KeepOrgProvider.kt index 0eb89cf..640b3f1 100644 --- a/android/app/src/main/java/cc/tiredsysadmin/android/keeporg/KeepOrgProvider.kt +++ b/android/app/src/main/java/cc/tiredsysadmin/android/keeporg/KeepOrgProvider.kt @@ -97,9 +97,13 @@ class KeepOrgProvider : DocumentsProvider() { val file = fileOf(documentId) val access = ParcelFileDescriptor.parseMode(mode) if (!mode.contains("w")) return ParcelFileDescriptor.open(file, access) - // The application is about to save: reconcile once it has closed the - // file, which is the moment the new bytes are complete on disk. + // The application is about to save. Until it closes the file the + // mirror leaves it alone — a pass would read half a save, or rename + // a pulled copy over it — and the close triggers the next pass, + // when the new bytes are complete on disk. + writers.merge(file.name, 1, Int::plus) return ParcelFileDescriptor.open(file, access, closeHandler) { + writers.computeIfPresent(file.name) { _, n -> if (n > 1) n - 1 else null } Syncer.request(ctx(), "saved $documentId") } } @@ -154,10 +158,14 @@ class KeepOrgProvider : DocumentsProvider() { } private fun fileOf(documentId: String): File { - if (documentId.contains('/') || documentId.contains("..")) { + if (documentId.contains('/') || documentId.contains('\u0000')) { throw FileNotFoundException("bad document id") } - val f = File(mirrorDir(), documentId) + val dir = mirrorDir().canonicalFile + val f = File(dir, documentId) + // The canonical parent must be the mirror itself: no "..", no link + // out of it. + if (f.canonicalFile.parentFile != dir) throw FileNotFoundException("bad document id") if (!f.isFile) throw FileNotFoundException(documentId) return f } @@ -166,6 +174,11 @@ class KeepOrgProvider : DocumentsProvider() { name.contains(".conflict-") || name.contains(".server-v") companion object { + /** Files a KeePass application holds open for writing, by name. */ + private val writers = java.util.concurrent.ConcurrentHashMap() + + fun isOpenForWrite(name: String): Boolean = writers.containsKey(name) + const val AUTHORITY = "cc.tiredsysadmin.android.keeporg.documents" const val ROOT_ID = "keeporg" const val ROOT_DOC = "root" diff --git a/android/app/src/main/java/cc/tiredsysadmin/android/keeporg/LoginActivity.kt b/android/app/src/main/java/cc/tiredsysadmin/android/keeporg/LoginActivity.kt index 7f75ad9..84d6280 100644 --- a/android/app/src/main/java/cc/tiredsysadmin/android/keeporg/LoginActivity.kt +++ b/android/app/src/main/java/cc/tiredsysadmin/android/keeporg/LoginActivity.kt @@ -123,6 +123,12 @@ class LoginActivity : Activity() { System.currentTimeMillis() + start.expiresIn * 1000L, ) collectPendingSignIn(now = false) + // Only the server's own approval page: a compromised server + // must not choose what this phone opens. + if (!isServerPage(api.serverUrl, start.verificationUrl)) { + failed(IOException("the server asked to open a page that is not its own")) + return@fold + } runCatching { startActivity(Intent(Intent.ACTION_VIEW, Uri.parse(start.verificationUrl))) } @@ -215,6 +221,15 @@ class LoginActivity : Activity() { } companion object { + /** Whether [page] is [server]'s device approval page. */ + fun isServerPage(server: String, page: String): Boolean { + val s = runCatching { java.net.URI(server) }.getOrNull() ?: return false + val p = runCatching { java.net.URI(page) }.getOrNull() ?: return false + return p.scheme == s.scheme && p.host.equals(s.host, ignoreCase = true) && + p.port == s.port && p.userInfo == null && + p.path == (s.path ?: "").trimEnd('/') + "/device" + } + /** Where the approval page links back to; see the manifest. */ const val RETURN_URL = "keeporg://device-approved" } diff --git a/android/app/src/main/java/cc/tiredsysadmin/android/keeporg/Mirror.kt b/android/app/src/main/java/cc/tiredsysadmin/android/keeporg/Mirror.kt index 6a47847..7893293 100644 --- a/android/app/src/main/java/cc/tiredsysadmin/android/keeporg/Mirror.kt +++ b/android/app/src/main/java/cc/tiredsysadmin/android/keeporg/Mirror.kt @@ -43,11 +43,10 @@ class Mirror(private val context: Context, private val api: Api, private val pre } } val seen = dbs.map { it.id }.toSet() - for (gone in prefs.trackedIds - seen) { - // The file itself stays: deleting someone's password database is - // not this app's decision. - prefs.forgetState(gone) - } + // A database no longer assigned stops being synchronized. Its file + // stays — deleting someone's password database is not this app's + // decision — and its name stays retired. + prefs.retiredIds = (prefs.retiredIds + (prefs.trackedIds - seen)) - seen prefs.trackedIds = seen notifyPicker() return if (firstError == null) Result.success(Unit) else Result.failure(firstError) @@ -56,6 +55,13 @@ class Mirror(private val context: Context, private val api: Api, private val pre /** Decides and applies the one action this database needs now. */ private fun reconcile(db: Db) { val st = trackedState(db) + // A KeePass application is writing it right now: reading it would + // see half a save, and replacing it would lose the rest. The close + // of that descriptor triggers the next pass. + if (KeepOrgProvider.isOpenForWrite(st.file)) { + prefs.putState(db.id, st) + return + } val file = File(dir, st.file) val localExists = file.isFile val local = if (localExists) file.readBytes() else ByteArray(0) @@ -77,7 +83,7 @@ class Mirror(private val context: Context, private val api: Api, private val pre when (action) { Action.NOTHING -> if (prefs.status(db.id).isEmpty()) prefs.putStatus(db.id, "in sync") - Action.PULL -> pull(db, st, file) + Action.PULL -> pull(db, st, file, localSha) Action.PUSH, Action.PUSH_FOR_MERGE -> push(db, st, file, local, force = false) @@ -85,7 +91,7 @@ class Mirror(private val context: Context, private val api: Api, private val pre Action.SERVER_WINS, Action.RESTORE -> { val kept = keepConflictCopy(file, local) - pull(db, st, file) + pull(db, st, file, localSha) val why = if (action == Action.RESTORE) "read-only access: your save was kept as ${kept.name}" else @@ -104,9 +110,19 @@ class Mirror(private val context: Context, private val api: Api, private val pre prefs.putState(db.id, st) } - private fun pull(db: Db, st: Prefs.FileState, file: File) { + /** + * Replaces the local file with the server copy. [expectSha] is the digest + * of the file the decision was made on ("" for none): if the file is + * different now, it was saved in the meantime, and replacing it would + * lose that save without a trace. + */ + private fun pull(db: Db, st: Prefs.FileState, file: File, expectSha: String) { val (data, version) = api.pull(db.id) if (version == 0L || data.isEmpty()) return + val nowSha = if (file.isFile) digest(file.readBytes()) else "" + if (nowSha != expectSha) { + throw IOException("the file was saved during synchronization; trying again") + } writeAtomic(file, data) st.version = version st.sha256 = digest(data) @@ -120,12 +136,12 @@ class Mirror(private val context: Context, private val api: Api, private val pre st.sha256 = res.sha256 if (res.merged) { // The result is neither copy: take what the server holds. - pull(db, st, file) + pull(db, st, file, digest(data)) prefs.putStatus(db.id, "merged: ${res.mergeSummary}") return } if (res.sha256 != digest(data)) { - pull(db, st, file) + pull(db, st, file, digest(data)) return } prefs.putStatus(db.id, if (res.unchanged) "in sync" else "pushed version ${res.version}") @@ -138,12 +154,12 @@ class Mirror(private val context: Context, private val api: Api, private val pre return } val kept = keepConflictCopy(file, data) - pull(db, st, file) + pull(db, st, file, digest(data)) prefs.putStatus(db.id, "the server copy was newer: your save was kept as ${kept.name}") } e.status == 403 -> { val kept = keepConflictCopy(file, data) - pull(db, st, file) + pull(db, st, file, digest(data)) prefs.putStatus(db.id, "read-only access: your save was kept as ${kept.name}") } else -> throw e @@ -154,6 +170,8 @@ class Mirror(private val context: Context, private val api: Api, private val pre /** Returns the state for a database, following a rename made on the server. */ private fun trackedState(db: Db): Prefs.FileState { val st = prefs.state(db.id) + // Access came back: the file and what was last agreed still hold. + if (db.id in prefs.retiredIds) prefs.retiredIds = prefs.retiredIds - db.id if (st.file.isEmpty()) { st.name = db.name st.file = fileName(db) @@ -163,6 +181,13 @@ class Mirror(private val context: Context, private val api: Api, private val pre val want = fileName(db) val old = File(dir, st.file) if (old.isFile && want != st.file) { + // Never onto a file that is already there: renameTo would + // replace it, and it may be another vault. + if (File(dir, want).exists()) { + Log.w(TAG, "not renaming ${st.file} to $want, which already exists") + st.name = db.name + return st + } if (!old.renameTo(File(dir, want))) { Log.w(TAG, "could not rename ${st.file} to $want") return st @@ -174,10 +199,12 @@ class Mirror(private val context: Context, private val api: Api, private val pre return st } + /** Names compare without regard to case, as in the desktop client. */ private fun fileName(db: Db): String { val base = sanitize(db.name).ifEmpty { "database" } val name = "$base.kdbx" - val taken = prefs.trackedIds.any { it != db.id && prefs.state(it).file == name } + val taken = (prefs.trackedIds + prefs.retiredIds) + .any { it != db.id && prefs.state(it).file.equals(name, ignoreCase = true) } return if (taken) "$base-${db.id.take(6)}.kdbx" else name } @@ -248,10 +275,40 @@ class Mirror(private val context: Context, private val api: Api, private val pre } } - fun sanitize(name: String): String = buildString { - for (ch in name.trim()) { - append(if (ch.isLetterOrDigit() || ch in "-_. ") ch else '-') + /** + * Keeps a database name usable as a file name and out of the names + * this app makes for itself. The same rules as the desktop client's + * sanitize, character for character. + */ + fun sanitize(name: String): String { + var out = buildString { + for (ch in name.trim()) { + val ok = ch in 'a'..'z' || ch in 'A'..'Z' || ch in '0'..'9' || ch in "-_. " + append(if (ok) ch else '-') + } } - }.removeSuffix(".kdbx").trim(' ', '.', '-') + while (out.contains("..")) out = out.replace("..", ".") + out = out.removeSuffix(".kdbx").trim(' ', '.', '-') + for (reserved in listOf(".server-v", ".conflict-", ".keeporg")) { + out = undot(out, reserved) + } + val stem = out.substringBefore('.').uppercase(Locale.ROOT) + if (stem in WINDOWS_DEVICES) out += "-db" + return out + } + + private val WINDOWS_DEVICES = setOf("CON", "PRN", "AUX", "NUL") + + (1..9).flatMap { listOf("COM$it", "LPT$it") } + + /** Turns the dot of each case-insensitive [marker] into a dash. */ + private fun undot(s: String, marker: String): String { + val out = StringBuilder(s) + var i = s.indexOf(marker, ignoreCase = true) + while (i >= 0) { + out.setCharAt(i, '-') + i = s.indexOf(marker, i + marker.length, ignoreCase = true) + } + return out.toString() + } } } diff --git a/android/app/src/main/java/cc/tiredsysadmin/android/keeporg/Prefs.kt b/android/app/src/main/java/cc/tiredsysadmin/android/keeporg/Prefs.kt index a9b1f03..8eb9b9c 100644 --- a/android/app/src/main/java/cc/tiredsysadmin/android/keeporg/Prefs.kt +++ b/android/app/src/main/java/cc/tiredsysadmin/android/keeporg/Prefs.kt @@ -105,6 +105,15 @@ class Prefs(context: Context) { get() = sp.getStringSet("tracked", emptySet()) ?: emptySet() set(v) = sp.edit().putStringSet("tracked", v).apply() + /** + * Databases this device lost access to. Their state is kept, so their + * file names stay taken: a database created later under the same name + * must not adopt, and upload, a file that belonged to another. + */ + var retiredIds: Set + get() = sp.getStringSet("retired", emptySet()) ?: emptySet() + set(v) = sp.edit().putStringSet("retired", v).apply() + /** The last status line shown on the main screen, per database. */ fun putStatus(dbId: String, text: String) = sp.edit().putString("status_$dbId", text).apply() diff --git a/android/metadata/cc.tiredsysadmin.android.keeporg.yml b/android/metadata/cc.tiredsysadmin.android.keeporg.yml index ce82d71..9c1c116 100644 --- a/android/metadata/cc.tiredsysadmin.android.keeporg.yml +++ b/android/metadata/cc.tiredsysadmin.android.keeporg.yml @@ -57,7 +57,15 @@ Builds: - yes output: android/app/build/outputs/apk/release/app-release-unsigned.apk + - versionName: 0.4.0 + versionCode: 4 + commit: v0.4.0-android + subdir: android/app + gradle: + - yes + output: android/app/build/outputs/apk/release/app-release-unsigned.apk + AutoUpdateMode: None UpdateCheckMode: Tags -CurrentVersion: 0.3.0 -CurrentVersionCode: 3 +CurrentVersion: 0.4.0 +CurrentVersionCode: 4 diff --git a/cmd/keeporg/agent.go b/cmd/keeporg/agent.go index 261fb10..0749d6d 100644 --- a/cmd/keeporg/agent.go +++ b/cmd/keeporg/agent.go @@ -259,6 +259,9 @@ func (a *agent) signIn(ctx context.Context, open func(string)) error { if err != nil { return err } + if err := client.CheckVerificationURL(cfg.ServerURL, start.VerificationURL); err != nil { + return err + } a.setPending(start.UserCode) defer a.setPending("") open(start.VerificationURL) diff --git a/cmd/keeporg/agent_test.go b/cmd/keeporg/agent_test.go index e5c4c7a..412015b 100644 --- a/cmd/keeporg/agent_test.go +++ b/cmd/keeporg/agent_test.go @@ -159,3 +159,11 @@ func TestMirrorSettingsChanged(t *testing.T) { t.Error("a new mirror directory did not restart the mirror") } } + +func TestTermSafeStripsControlCharacters(t *testing.T) { + var b strings.Builder + termSafe{&b}.Write([]byte("Team\x1b]0;pwned\x07 Vault\t1\n\x9b2J")) + if got := b.String(); got != "Team]0;pwned Vault\t1\n2J" { + t.Fatalf("got %q", got) + } +} diff --git a/cmd/keeporg/autostart_darwin.go b/cmd/keeporg/autostart_darwin.go index d434413..06a6e72 100644 --- a/cmd/keeporg/autostart_darwin.go +++ b/cmd/keeporg/autostart_darwin.go @@ -3,6 +3,8 @@ package main import ( + "bytes" + "encoding/xml" "fmt" "os" "path/filepath" @@ -50,9 +52,17 @@ func setStartAtLogin(enable bool) error { Interactive -`, AppID, exe) +`, xmlEscape(AppID), xmlEscape(exe)) if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { return err } return os.WriteFile(path, []byte(plist), 0o644) } + +// xmlEscape makes a string safe inside a plist element; a path with "&" or +// "<" would otherwise produce a LaunchAgent that never loads. +func xmlEscape(s string) string { + var b bytes.Buffer + _ = xml.EscapeText(&b, []byte(s)) + return b.String() +} diff --git a/cmd/keeporg/autostart_xdg.go b/cmd/keeporg/autostart_xdg.go index 3c7af0d..0df8575 100644 --- a/cmd/keeporg/autostart_xdg.go +++ b/cmd/keeporg/autostart_xdg.go @@ -6,6 +6,7 @@ import ( "fmt" "os" "path/filepath" + "strings" ) // autostartPath is the XDG autostart entry every freedesktop session reads. @@ -39,13 +40,37 @@ func setStartAtLogin(enable bool) error { Type=Application Name=KeepOrg Comment=KeePass database synchronization -Exec="%s" tray +Exec=%s tray Icon=keeporg Terminal=false X-GNOME-Autostart-enabled=true -`, exe) +`, desktopExecArg(exe)) if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { return err } return os.WriteFile(path, []byte(entry), 0o644) } + +// desktopExecArg quotes a path for a desktop entry's Exec key: inside double +// quotes, ", `, $ and \ are escaped with a backslash, % is doubled as a field +// code would be, and then the whole value is escaped again as a string, which +// doubles every backslash. +func desktopExecArg(path string) string { + var b strings.Builder + b.WriteByte('"') + for _, r := range path { + switch r { + case '"', '`', '$': + b.WriteString(`\\`) // \ escaped once more as a string + b.WriteRune(r) + case '\\': + b.WriteString(`\\\\`) + case '%': + b.WriteString("%%") + default: + b.WriteRune(r) + } + } + b.WriteByte('"') + return b.String() +} diff --git a/cmd/keeporg/autostart_xdg_test.go b/cmd/keeporg/autostart_xdg_test.go new file mode 100644 index 0000000..5c02831 --- /dev/null +++ b/cmd/keeporg/autostart_xdg_test.go @@ -0,0 +1,13 @@ +//go:build linux || freebsd || openbsd || netbsd || dragonfly + +package main + +import "testing" + +func TestDesktopExecArgEscapes(t *testing.T) { + got := desktopExecArg(`/opt/a "b" $c\d 100%`) + want := `"/opt/a \\"b\\" \\$c\\\\d 100%%"` + if got != want { + t.Fatalf("got %s, want %s", got, want) + } +} diff --git a/cmd/keeporg/commands.go b/cmd/keeporg/commands.go index d4ee7f7..5912f6a 100644 --- a/cmd/keeporg/commands.go +++ b/cmd/keeporg/commands.go @@ -122,6 +122,9 @@ func browserLogin(ctx context.Context, api *client.API, label string) (*client.S if err != nil { return nil, err } + if err := client.CheckVerificationURL(api.BaseURL, start.VerificationURL); err != nil { + return nil, err + } fmt.Printf("Open this page and approve the code %s:\n\n %s\n\n", start.UserCode, start.VerificationURL) openBrowser(start.VerificationURL) @@ -240,7 +243,7 @@ func cmdStatus(ctx context.Context, args []string) error { if err != nil { return err } - w := tabwriter.NewWriter(os.Stdout, 0, 2, 2, ' ', 0) + w := tabwriter.NewWriter(stdout, 0, 2, 2, ' ', 0) fmt.Fprintln(w, "DATABASE\tACCESS\tSTRATEGY\tVERSION\tUPDATED BY\tUPDATED") for _, d := range dbs { when := "never" @@ -253,7 +256,7 @@ func cmdStatus(ctx context.Context, args []string) error { } func printStatus(st []client.Status) { - w := tabwriter.NewWriter(os.Stdout, 0, 2, 2, ' ', 0) + w := tabwriter.NewWriter(stdout, 0, 2, 2, ' ', 0) fmt.Fprintln(w, "DATABASE\tFILE\tVERSION\tSTATE\tNOTE") for _, s := range st { fmt.Fprintf(w, "%s\t%s\t%d\t%s\t%s\n", s.Name, s.File, s.Version, s.State, s.Message) @@ -302,7 +305,19 @@ func cmdTOTP(ctx context.Context, args []string) error { if err != nil { return err } - sec, uri, err := api.EnrollTOTP(ctx) + password, err := readPassword("Account password: ") + if err != nil { + return err + } + sec, uri, err := api.EnrollTOTP(ctx, password, "") + if client.Code(err) == "reauth_required" { + // A factor is on already; replacing it needs its current code. + code, cerr := prompt("Current one-time code: ") + if cerr != nil { + return cerr + } + sec, uri, err = api.EnrollTOTP(ctx, password, code) + } if err != nil { return err } @@ -353,7 +368,7 @@ func cmdGroups(ctx context.Context, args []string) error { if err != nil { return err } - w := tabwriter.NewWriter(os.Stdout, 0, 2, 2, ' ', 0) + w := tabwriter.NewWriter(stdout, 0, 2, 2, ' ', 0) fmt.Fprintln(w, "ID\tNAME\tPARENT\tMEMBERS\tYOU ADMINISTER") for _, g := range groups { fmt.Fprintf(w, "%s\t%s\t%s\t%d\t%v\n", g.ID, g.Name, g.ParentID, g.Members, g.Admin) @@ -592,7 +607,7 @@ func cmdInvite(ctx context.Context, args []string) error { if err != nil { return err } - w := tabwriter.NewWriter(os.Stdout, 0, 2, 2, ' ', 0) + w := tabwriter.NewWriter(stdout, 0, 2, 2, ' ', 0) fmt.Fprintln(w, "URL\tGROUP\tROLE\tUSES\tEXPIRES") for _, i := range invites { fmt.Fprintf(w, "%s\t%s\t%s\t%d/%d\t%s\n", i.URL, i.GroupID, i.Role, i.Uses, i.MaxUses, @@ -621,7 +636,7 @@ func cmdUsers(ctx context.Context, args []string) error { if err != nil { return err } - w := tabwriter.NewWriter(os.Stdout, 0, 2, 2, ' ', 0) + w := tabwriter.NewWriter(stdout, 0, 2, 2, ' ', 0) fmt.Fprintln(w, "ID\tUSERNAME\tEMAIL\tADMIN\t2FA") for _, u := range users { fmt.Fprintf(w, "%s\t%s\t%s\t%v\t%v\n", u.ID, u.Username, u.Email, u.ServerAdmin, u.TOTPEnabled) diff --git a/cmd/keeporg/desktop.go b/cmd/keeporg/desktop.go index a799f8e..e23d87e 100644 --- a/cmd/keeporg/desktop.go +++ b/cmd/keeporg/desktop.go @@ -2,8 +2,13 @@ package main import ( "fmt" + "html" + "io" + "os" "os/exec" "runtime" + "strings" + "unicode/utf8" ) // homepage is where About points. @@ -47,7 +52,10 @@ func notify(title, message string) { fmt.Sprintf("display notification %q with title %q", message, title)).Start() case "windows": default: - _ = exec.Command("notify-send", "--app-name=KeepOrg", "--icon=keeporg", title, message).Start() + // "--" so a database named "-u critical" is a title, not an option; + // the body is markup to notification daemons, so it is escaped. + _ = exec.Command("notify-send", "--app-name=KeepOrg", "--icon=keeporg", "--", + html.EscapeString(title), html.EscapeString(message)).Start() } } @@ -56,3 +64,29 @@ func aboutText(configPath string) string { return fmt.Sprintf("KeepOrg %s\n\nKeePass database synchronization.\n\nConfiguration: %s\n%s", Version, configPath, homepage) } + +// termSafe passes text through to a terminal without its control +// characters. Names, labels and URLs printed by the CLI come from the server, +// and escape sequences in them could rewrite what the terminal shows. +type termSafe struct{ w io.Writer } + +func (t termSafe) Write(p []byte) (int, error) { + clean := strings.Map(func(r rune) rune { + if r == '\n' || r == '\t' { + return r + } + // A lone byte 0x9b is a C1 control to some terminals and invalid + // UTF-8 to Go, which reads it as RuneError. + if r < 0x20 || (r >= 0x7f && r < 0xa0) || r == utf8.RuneError { + return -1 + } + return r + }, string(p)) + if _, err := io.WriteString(t.w, clean); err != nil { + return 0, err + } + return len(p), nil +} + +// stdout is where commands print: standard output, made safe for a terminal. +var stdout io.Writer = termSafe{os.Stdout} diff --git a/cmd/keeporgd/main.go b/cmd/keeporgd/main.go index f0d0f32..d0586ba 100644 --- a/cmd/keeporgd/main.go +++ b/cmd/keeporgd/main.go @@ -27,8 +27,17 @@ import ( "cc.tiredsysadmin.keeporg/internal/store" ) -// externalURLEnv sets external_url, between the configuration file and -url. -const externalURLEnv = "KEEPORG_EXTERNAL_URL" +// Settings a deployment usually knows only at deploy time can come from the +// environment as well. Each sits between the configuration file and its flag. +const ( + // externalURLEnv sets external_url, overridden by -url. + externalURLEnv = "KEEPORG_EXTERNAL_URL" + // databaseEnv sets database, overridden by -database. A PostgreSQL URL + // carries a password, which is why it belongs in a secret, not a file. + databaseEnv = "KEEPORG_DATABASE_URL" + // proxiesEnv sets trusted_proxies, comma-separated. + proxiesEnv = "KEEPORG_TRUSTED_PROXIES" +) // Version is stamped at build time from versions.mk. var Version = "dev" @@ -43,6 +52,8 @@ type fileConfig struct { MaxDBSizeMB int64 `json:"max_db_size_mb"` TLSCert string `json:"tls_cert"` TLSKey string `json:"tls_key"` + Database string `json:"database"` + TrustedProxies []string `json:"trusted_proxies"` MasterKeyFile string `json:"master_key_file"` VaultKDF vaultKDF `json:"vault_kdf"` OIDC server.OIDCConfig `json:"oidc"` @@ -60,7 +71,7 @@ func (f fileConfig) toConfig() server.Config { c := server.Config{ Addr: f.Addr, DataDir: f.DataDir, ExternalURL: f.ExternalURL, TLSCert: f.TLSCert, TLSKey: f.TLSKey, OIDC: f.OIDC, - MasterKeyFile: f.MasterKeyFile, + Database: f.Database, MasterKeyFile: f.MasterKeyFile, TrustedProxies: f.TrustedProxies, } if f.VaultKDF.MemoryMB > 0 { c.VaultKDF = kdbx.KDFParams{ @@ -87,7 +98,9 @@ func main() { addr = flag.String("addr", "", "listen address (overrides the config file)") dataDir = flag.String("data", "", "data directory (overrides the config file)") extURL = flag.String("url", "", "externally reachable base URL (overrides $"+externalURLEnv+" and the config file)") - masterKey = flag.String("master-key-file", "", + database = flag.String("database", "", "postgres://… or sqlite:; default sqlite:/keeporg.db "+ + "(overrides $"+databaseEnv+" and the config file)") + masterKey = flag.String("master-key-file", "", "file holding the master key that wraps the vault keys, created if "+ "absent; $"+secret.MasterKeyEnv+" and $"+secret.MasterKeyFileEnv+ " take precedence over it and are never created") @@ -131,6 +144,15 @@ func main() { if v := strings.TrimSpace(os.Getenv(externalURLEnv)); v != "" { cfg.ExternalURL = v } + if v := strings.TrimSpace(os.Getenv(databaseEnv)); v != "" { + cfg.Database = v + } + if v := strings.TrimSpace(os.Getenv(proxiesEnv)); v != "" { + cfg.TrustedProxies = strings.Split(v, ",") + } + if *database != "" { + cfg.Database = *database + } if *addr != "" { cfg.Addr = *addr } @@ -164,6 +186,7 @@ func main() { if err != nil { lg.Fatalf("start: %v", err) } + defer srv.Close() if err := bootstrap(srv, lg); err != nil { lg.Fatalf("bootstrap: %v", err) } @@ -181,8 +204,8 @@ func main() { go func() { eff := srv.Config() - lg.Printf("version %s listening on %s (external %s, data %s)", - Version, eff.Addr, eff.ExternalURL, eff.DataDir) + lg.Printf("version %s listening on %s (external %s, database %s)", + Version, eff.Addr, eff.ExternalURL, srv.DatabaseWhere()) var err error if eff.TLSCert != "" && eff.TLSKey != "" { err = hs.ListenAndServeTLS(eff.TLSCert, eff.TLSKey) @@ -247,23 +270,28 @@ func bootstrap(srv *server.Server, lg *log.Logger) error { } // resetAdministrator gives the server's administrator a new password, for -// when the one printed at first start was not kept. It writes the state -// document directly, so the server must be stopped: a running one holds the -// state in memory and would write over this on its next change. +// when the one printed at first start was not kept. It writes to the +// database like any server process would, so a running server picks the +// change up within seconds and need not be stopped. // // Any second factor on that account is cleared too. Someone who has lost // the password has often lost the authenticator with it, and a recovery // that leaves you locked out is not one. func resetAdministrator(cfg server.Config, lg *log.Logger) error { cfg.Defaults() - path := filepath.Join(cfg.DataDir, "state.json") - if _, err := os.Stat(path); err != nil { - return fmt.Errorf("no server state at %s: %w", path, err) + kr, err := secret.LoadKeyring(cfg.MasterKeyFile) + if err != nil { + return fmt.Errorf("master key: %w", err) } - st, err := store.Open(path) + st, db, err := server.OpenStore(cfg, kr) if err != nil { return err } + defer db.Close() + defer st.Close() + if st.Empty() { + return fmt.Errorf("the database %s holds no server yet; start keeporgd first", db.Where) + } password := os.Getenv("KEEPORG_ADMIN_PASSWORD") generated := false @@ -313,7 +341,6 @@ func resetAdministrator(cfg server.Config, lg *log.Logger) error { lg.Printf("reset %q to the password from KEEPORG_ADMIN_PASSWORD. Any second "+ "factor on that account was cleared and its sessions were revoked.", name) } - lg.Print("if keeporgd was running while this ran, restart it now so it reloads the change") return nil } @@ -356,6 +383,8 @@ func writeExampleConfig(path string) error { ExternalURL: "https://keeporg.example.com", SessionTTLHours: 24 * 90, MaxDBSizeMB: 32, + Database: "sqlite:/var/lib/keeporg/keeporg.db", + TrustedProxies: []string{"127.0.0.1", "::1"}, MasterKeyFile: "/etc/keeporg/master.key", VaultKDF: vaultKDF{MemoryMB: 32, Iterations: 2, Lanes: 2}, OIDC: server.OIDCConfig{ diff --git a/config/nginx/keeporg.conf.tmpl b/config/nginx/keeporg.conf.tmpl index a79f7bd..c293299 100644 --- a/config/nginx/keeporg.conf.tmpl +++ b/config/nginx/keeporg.conf.tmpl @@ -24,7 +24,16 @@ server { ssl_certificate {{ .nginx.variables.blog_ssl_certificate }}; ssl_certificate_key {{ .nginx.variables.blog_ssl_certificate_key }}; - client_max_body_size 8192M; + # Modern TLS only, whatever the host's defaults are. + ssl_protocols TLSv1.2 TLSv1.3; + ssl_ciphers ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305; + ssl_prefer_server_ciphers off; + ssl_session_tickets off; + add_header Strict-Transport-Security "max-age=63072000; includeSubDomains" always; + + # The server refuses databases over max_db_size_mb (32 by default); a + # little headroom over that, and no more, so oversized uploads stop here. + client_max_body_size 64M; server_name keeporg.tiredsysadmin.cc; diff --git a/config/variables/server.values.yaml b/config/variables/server.values.yaml index 6009e5a..9496a02 100644 --- a/config/variables/server.values.yaml +++ b/config/variables/server.values.yaml @@ -5,6 +5,10 @@ environment: value: "/etc/keeporg/config.txt" - name: "KEEPORG_EXTERNAL_URL" value: "https://keeporg.tiredsysadmin.cc" +# nginx reaches the pod from the cluster's private network; its +# X-Forwarded-For is what the sign-in rate limits count by. +- name: "KEEPORG_TRUSTED_PROXIES" + value: "10.0.0.0/8,172.16.0.0/12,192.168.0.0/16" image: name: "repo.rcmd.space/keeporg" version: "v0.1.0" diff --git a/docs/HANDOVER.md b/docs/HANDOVER.md index d4fbf6d..4cc6971 100644 --- a/docs/HANDOVER.md +++ b/docs/HANDOVER.md @@ -28,8 +28,11 @@ cmd/keeporgd/ server entry point: flags, config file, bootstrap cmd/keeporg/ desktop client: the tray application (agent.go is its logic, tray.go its menu), login, run, sync, status, administration, start-at-login per platform -internal/store/ domain model, JSON metadata store, ACL resolution -internal/blob/ versioned KDBX payload storage on disk +internal/store/ domain model, the in-memory state and its sealed rows, + ACL resolution +internal/blob/ versioned KDBX payloads, sealed, in the database +internal/sqldb/ SQLite or PostgreSQL: connection, dialect, schema, + generation lock, key check; sqldbtest for tests internal/kdbx/ KDBX 4 codec and the merge engine, with its own crypto internal/server/ HTTP surface: auth, OIDC, sync, admin, SSE hub, web, web administration, page templates @@ -52,21 +55,33 @@ Makefile the only entry point for building and publishing ### Server (`keeporgd`) -One process, one data directory, no database engine: +One process, one database, one master key: -- `state.json` holds users, groups, memberships, databases, grants, invites, - sessions and pending device logins. For a merge vault it also holds that - vault's password, encrypted under the server's master key. -- `blobs//.kdbx` holds the payloads, keeping the last - 20 versions of each database. -- The master key itself lives **outside** the data directory, so a copy of - the data directory alone yields ciphertext. +- **The database** is SQLite (`sqlite:`, by default + `/keeporg.db`) or PostgreSQL (`postgres://…`), chosen by the + `database` setting, `KEEPORG_DATABASE_URL` or `-database`. It holds + everything: users, groups, memberships, databases, grants, invites, + sessions, pending device logins, and the last 20 versions of every KDBX + payload. +- **Every row is sealed** with AES-256-GCM under a key derived from the + master key, and named by an HMAC rather than by anything readable. The + database, its backups and its administrators see three tables of opaque + rows; see [Encryption at rest](#encryption-at-rest). +- **The master key** lives outside the database and outside the data + directory, so neither a database dump nor a copy of the data directory + yields anything readable. The server does not start without it, and does + not start with a different one. -`internal/store.Store` is the only writer of `state.json`. Every mutation -runs through `Update(func(*State) error)`, which applies the change to a -private copy, writes it with `write` + `fsync` + `rename`, and only then -publishes it. A failed mutation changes nothing, and a crash leaves either -the old document or the new one. +`internal/store.Store` keeps the metadata in memory as one `State` and is +the only writer of it. Every mutation runs through `Update(func(*State) +error)`, which applies the change to a private copy, writes the rows that +changed in one transaction, and only then publishes the copy. A failed +mutation changes nothing, and a crash leaves the last committed state. + +`internal/sqldb` is the database underneath: connection, dialect, schema. +`internal/blob` keeps the payloads in the same database. Both engines run +through `database/sql` with pure-Go drivers (`modernc.org/sqlite`, +`jackc/pgx`), so the server still builds with `CGO_ENABLED=0`. Reads go through `Read(func(*State))` under a read lock. Access decisions are pure functions over `*State` in `internal/store/acl.go`, which is why they @@ -318,6 +333,7 @@ Three different things are encrypted here, and they are worth keeping apart: fails to open instead of unlocking the wrong database. It is never returned by the API, never logged, and never leaves the process. 3. **Account passwords.** Not stored at all; only PBKDF2-SHA256 verifiers. +4. **Everything else the server keeps**, in the database: see below. The master key is looked for in three places, in order: @@ -330,28 +346,56 @@ The master key is looked for in three places, in order: Only the configured path is ever created, which is what makes a plain package install work with no setup. The two environment variables name a key that something else owns, so a missing one is reported rather than replaced: -minting a key in its place would open none of the existing vaults. Without -any of the three the server still runs and still serves every key-less -strategy; only vault creation and merging are refused, with a line in the -log saying so. +minting a key in its place would open nothing that is already stored. The +server refuses to start without a key, and refuses one that is not the key +the database was written with (`sqldb.CheckKey` opens a sealed marker stored +at the first start). Carrying on would write rows nobody could read beside +rows this key cannot. + +### Encryption at rest + +The database holds three tables, and none of them says anything readable: + +| Table | Row | Name of the row | Content | +| --- | --- | --- | --- | +| `keeporg_records` | one user, group, database record, grant, membership, invite, session or device login | `HMAC(records key, kind ‖ natural key)` | the object as JSON, sealed | +| `keeporg_blobs` | one stored version of one KDBX file | `HMAC(blobs key, database id)` and the version number | the KDBX bytes, sealed | +| `keeporg_meta` | schema version, write generation, key check | plain names | numbers, and the sealed key-check marker | + +The records key and the blobs key are derived from the master key with +HMAC-SHA256 under distinct names (`secret.Keyring.Derive`), so a ciphertext +made for one purpose does not open under another. Each row is sealed with +AES-256-GCM, its own name as the authenticated data: a row copied over +another, or a version copied over another version, fails to open instead of +being served. A row's name is stable for the same object, which is what +lets a write touch only the rows that changed, and says nothing to anyone +without the key — session and invite tokens, which are bearer credentials, +appear only inside sealed rows. + +What the database reveals is the number of rows, their sizes, and how many +versions each payload has. Nothing is queryable by SQL: the server reads +every record at start and answers from memory, which is also why access +decisions stay pure functions over `*State`. + +KDBX is already encrypted by KeePass; sealing it again means the database +holds no readable KeePass headers either (KDF parameters, cipher choice), +and one backup of the database is the whole server. What this buys and what it does not: -- A stolen copy of `/var/lib/keeporg` is useless on its own. The vaults are - ciphertext and the wrapped keys do not open without the master key. -- A stolen copy of the data directory **and** the master key opens every - merge vault. Keep them on different media, or feed the key in through - `KEEPORG_MASTER_KEY` from a secret manager or a systemd credential so it - never touches the disk at all. -- Backups must cover both, separately. A backup of the data directory with no - master key cannot be used to merge after a restore; the vaults still open - for their users, and the keys can be stored again with - `keeporg db -id -store-password`. +- A database dump, a backup, a replica or a curious database administrator + gets ciphertext. So does a stolen copy of the data directory. +- The database **and** the master key together open everything, including + every merge vault. Keep them on different media, or feed the key in + through `KEEPORG_MASTER_KEY_FILE` from a secret manager, a Kubernetes + secret or a systemd credential so it never sits beside the data. +- Backups must cover both, separately, and a database backup without its + key restores nothing. Losing the key loses the server. ## Authentication -Three ways in, one kind of credential out: an opaque bearer token stored in -`state.json` with an expiry (90 days by default). +Three ways in, one kind of credential out: an opaque bearer token, stored +sealed in the database with an expiry (90 days by default). - **Password** (`POST /api/v1/auth/login`), with the built-in TOTP second factor when the account has enrolled one. The server answers @@ -369,6 +413,36 @@ only. The server is a confidential client: it exchanges the authorization code over TLS with the token endpoint and reads the identity from the userinfo endpoint, so there is no local JWT verification to get wrong. +### Proving who someone is + +Every password and one-time code the server checks — API login, the sign-in +form, the device page, a password change, a second-factor change, a vault +deletion — goes through `verify` in `internal/server/authn.go`: + +- **The hash is checked outside the store's lock.** It is deliberately + slow; under the write lock it stalled every other request, so a few + guesses a second were a denial of service. `commit` re-checks inside the + write that the account and its hash did not change in between. +- **Failures are limited** per client address (30 per 15 minutes) and per + account (10), and a client or account over budget gets `429` with + `Retry-After` before any hash is computed. Unauthenticated device and + single sign-on starts are limited per address too. The address is the + peer's, or, for a peer in `trusted_proxies` (`KEEPORG_TRUSTED_PROXIES`), + the right-most untrusted address in `X-Forwarded-For`; nobody else gets to + name their own address. Behind nginx, set it, or every client shares the + proxy's budget. +- **Unknown, disabled and password-less accounts take as long to refuse as + a wrong password** (a dummy hash is verified), so timing does not list the + usernames. +- **A one-time code is good once.** The step of the last accepted code is + stored on the account (`TOTPLastStep`) and anything at or before it is + refused, including the code that confirmed enrolment. +- **Re-authentication does not say which factor was wrong.** For a change + made from a live session (password, second factor, deletion), a wrong + password and a wrong code give the same answer: a stolen session must not + become a password oracle. Starting a second factor needs the password too, + since on a stolen session it would lock the owner out. + ### Setting up OIDC At the identity provider, register a confidential client with one redirect @@ -388,7 +462,8 @@ and restart: "client_secret": "...", "scopes": "openid profile email", "auto_create": false, - "default_group_id": "" + "default_group_id": "", + "link_verified_email": false } ``` @@ -404,20 +479,33 @@ Two entry points use it, and both end at the same callback: starts a sign-in that ends in a session cookie. - **A client**, from the device flow. `keeporg login -browser=sso` prints a short code; the device page at `/device?code=…` offers the same link, this - time as `/oidc/login?code=`, and approving it releases the - token the waiting client is polling for. + time as `/oidc/login?code=`. After the provider, the person is + shown which device is asking, as whom, and its code, and must press + **Approve**: only then is the token released to the waiting client. + Arriving at the callback proves that a browser with a provider session was + sent there, not that its owner meant to let a device in, and before 0.12 a + single link click was enough to hand an attacker's device a session. + +Every round trip is bound to the browser that started it (a `keeporg_oidc` +cookie holding the state, checked at the callback; the approval's nonce in +the same cookie, checked at `/oidc/approve`) and uses PKCE, so a forwarded +callback URL or a code intercepted on the way back is useless. How an identity becomes an account: 1. If some account is already linked to this issuer and `sub`, that one. -2. Otherwise an unlinked local account whose username equals - `preferred_username`, or whose email matches `email`, is linked to it. - This is how someone invited by link later moves to single sign-on. +2. Otherwise, only when `link_verified_email` is set and the provider says + `email_verified`, an unlinked local account with the same email — never a + server administrator's, never a disabled one. **Never by username**: + whoever could choose `preferred_username=admin` at the provider would + otherwise have become the server administrator. 3. Otherwise, with `auto_create` set, a new account is provisioned and put - in `default_group_id` when one is given. Without it, sign-in is refused - with "no account is linked to this identity; redeem an invite link - first", which is the setting to keep if membership should stay by - invitation. + in `default_group_id` when one is given. Without it, sign-in is refused. + +An existing account moves to single sign-on from its own **Account** page: +**Link single sign-on** runs the round trip with that session's CSRF token +and links whatever identity comes back to it. That replaces the automatic +username matching that used to do this. An account created this way has no password, so it cannot sign in with one and cannot set one through the password form. @@ -489,7 +577,7 @@ the `/api/v1/auth/sessions` and `/api/v1/auth/totp` endpoints: out, or all but the current one. Pages and responses refer to a session by `sessionID`, a digest of its token, so the list never carries a token. `LastSeenAt` is refreshed by `markSeen` at most every five minutes, because - each refresh rewrites the state document. + each refresh is a database write. - *Two-factor authentication* turns on with nothing more than the session, since it only adds a check. Replacing or removing an active factor asks for the password and a current code: before this, `totp/enroll` reset the @@ -527,16 +615,63 @@ database, encrypts it with the password from the form, stores it as version state update fails, the blob is removed again, so a half-created vault cannot be left behind. +## Security hardening (0.12) + +An audit of every part — server authentication, the data plane and its +cryptography, both clients, and the deployment — found the problems below, +and all of the critical, high and medium ones were fixed with a regression +test each. The authentication changes are described under +[Proving who someone is](#proving-who-someone-is) and +[Setting up OIDC](#setting-up-oidc). + +| Area | Was | Now | +| --- | --- | --- | +| Single sign-on | one click on a link approved an attacker's device | an explicit Approve in the browser that started it; state cookie; PKCE | +| Single sign-on | an identity named `admin` claimed the admin account | no linking by username; verified email only when allowed; never an administrator | +| Passwords | no attempt limit; each guess held the global write lock | per-address and per-account limits; hashing outside the lock | +| TOTP | a code could be replayed for its 90-second window | each code is accepted once | +| Merge vaults | a writer could push a header asking for 2⁶² AES rounds or 2 GiB of Argon2 memory and stop the server, or a gzip bomb of nested XML that overflowed the stack | KDF, payload and XML bounds in `internal/kdbx/limits.go`; merge-vault pushes are checked (`kdbx.Inspect`) before they are stored; at most two merges at a time | +| Several nodes | a stale node's push could overwrite a version another node had committed | the version is written in the same transaction as its metadata, after the generation lock | +| Tokens | `?access_token=` worked on every endpoint, and the clients put it in URLs that reached logs | only on the event stream, for old clients; current clients send the header | +| Event streams | a revoked device kept receiving events | the session is re-checked at every keepalive; disabled users get no events | +| Web pages | no CSP, framing allowed, cacheable, login CSRF | CSP, `frame-ancestors 'none'`, `no-store`, HSTS behind https; the sign-in form needs a pre-session token | +| Enumeration | admin endpoints said 403 for someone else's database, 400 for none | 404 for both; group administrators no longer see other people's email, sign-in methods or 2FA | +| Desktop client | the server chose what the desktop opened at sign-in (any URL, `file:`, a network share) | only the server's own `/device` page | +| Desktop client | a database whose name differed in case, or matched a file left by a lost database, adopted and uploaded another vault | names compare case-insensitively; files of lost databases stay retired; renames never replace a file; reserved suffixes and Windows device names are avoided | +| Clients | a save made while a pull ran could be overwritten without a trace; Android could read half a save | the file is re-hashed before it is replaced; Android leaves files open for writing alone | +| Clients | `http://` servers accepted silently; unbounded reads from the server; markup and options in notifications; control characters printed to the terminal | `https` required except for loopback (`allow_insecure_http`); bounded reads; `--` and escaping; control characters stripped | +| Deployment | the `.deb` service could not create its master key under `ProtectSystem=strict`, so a fresh install did not start | the package creates the key; more systemd sandboxing | +| Deployment | nginx: host TLS defaults, no HSTS, 8 GiB uploads; CI ran as root on any branch | TLS 1.2+ and modern ciphers, HSTS, 64 MiB; CI on `main` only | + +Left for later, deliberately: + +- **Rollback of database rows.** Someone with write access to the database + itself can replay an older sealed row (a revoked session, a user before a + demotion). A sealed digest over every row, checked at load, would catch + it; until then, write access to the database is as sensitive as the + master key. +- **The master key also keys the subkey derivation and the vault-key + wrapping directly.** Not exploitable, but cleaner as separate derived keys; + changing it means re-wrapping every stored vault key. +- **A cap on event streams per user**, and pinning the container's base + image by digest. +- **The Kubernetes NodePort** (32970) reaches the server without nginx, + Vouch or TLS. Make sure only nginx can reach it. +- **KDBX magic is not required of pushes to key-less databases.** The server + treats their bytes as opaque by design. + ## Decisions worth knowing -**No third-party Go dependencies in the server, no cgo outside the macOS -tray.** The server builds with `CGO_ENABLED=0` from the standard library -alone: SSE instead of a WebSocket library, `crypto/pbkdf2` (new in Go 1.24) +**No cgo, and as few dependencies as the job allows.** The server builds +with `CGO_ENABLED=0`: SSE instead of a WebSocket library, `crypto/pbkdf2` for account passwords, hand-written ChaCha20, BLAKE2b and Argon2 for the -KDBX format, 60 lines of HMAC for TOTP, `html/template` for the pages. There -is no dependency tree to audit in the program that holds the vault keys; -`go list -deps ./cmd/keeporgd` shows only the standard library and this -module, and the container builds without fetching anything. +KDBX format, 60 lines of HMAC for TOTP, `html/template` for the pages. Its +only third-party code is the two database drivers, both pure Go: +`modernc.org/sqlite`, SQLite translated to Go, and `jackc/pgx` for +PostgreSQL. The standard library has no SQL driver, and writing either +would be a project of its own. Keeping them pure Go keeps static binaries, +cross-compilation and the `scratch` container; a cgo driver such as +SQLCipher was rejected for exactly that. The desktop client takes exactly one, `fyne.io/systray` (with `godbus/dbus` and `golang.org/x/sys` under it), because a tray icon cannot @@ -550,13 +685,34 @@ darwin client with cgo when it is built on a Mac; built elsewhere it keeps every command but `tray` (`tray_nocgo.go`). The client only moves ciphertext, so the trade is in the program that never sees a key. -**A JSON document instead of SQLite.** The metadata is hundreds of records, -never thousands: users, groups, grants. One writer with atomic rename gives -durability and consistency without cgo or a migration story, and an operator -can read and back up the whole server state with `cat`. The cost is a single -node and a full rewrite per mutation. If this ever needs several servers or -tens of thousands of users, replace `internal/store` — the rest of the code -only touches it through `Read`, `Update` and the ACL functions. +**A database, held in memory, sealed row by row.** Until 0.11 the server +kept one JSON document and a directory of KDBX files. Enterprise use needs a +real database — managed backups, replication, a PostgreSQL the operations +team already runs — and data that is encrypted at rest under a key the +database never sees. Three choices follow from that: + +- *The state stays in memory.* The metadata is hundreds of records, never + thousands, and every request makes access decisions over it. Loading it + once and answering from memory kept `Read`, `Update` and every ACL function + unchanged, so the rest of the server did not move. The database is + durable storage, not a query engine. +- *Encryption is in the application, not the engine.* Sealing each row with + the master key works the same on SQLite and PostgreSQL and protects dumps + and backups, which disk encryption and PostgreSQL's own options do not. + The price is that nothing is queryable in SQL — which, for a server whose + rows are bearer tokens and password verifiers, is the point. +- *Writes are diffs.* `Update` compares each row's plaintext digest with the + last one stored and writes only what changed, so a session touch is one + row, not the whole state. + +Several processes may share one PostgreSQL database. A write locks the +`generation` row (`SELECT … FOR UPDATE`), and if another process has +committed since this one loaded, the state is reloaded and the mutation +applied again on top. A watcher reloads within two seconds of another +process's write, so a session revoked through one process is refused by all +of them. What is not shared yet is the event stream: a client connected to +one process hears about pushes made through that process only, and picks up +the rest at its next periodic rescan. **A mirror directory instead of FUSE.** The requirement is that a KeePass application sees a regular `.kdbx` file. A real file in a real directory is @@ -598,7 +754,7 @@ keeporgd: created the first administrator: username "admin" password "13e4…" ``` Read it with `journalctl -u keeporgd`, or `podman logs `. It is -never written anywhere else: the state file holds only a PBKDF2 verifier. +never written anywhere else: the database holds only a PBKDF2 verifier. Set `KEEPORG_ADMIN_PASSWORD` before the first start to choose it instead, in which case nothing is printed. The variable is read only when the server has no accounts at all, so leaving it set afterwards does nothing. @@ -615,10 +771,12 @@ sudo -u keeporg keeporgd -data /var/lib/keeporg -reset-admin ``` It prints a new one, clears any second factor on that account, and revokes -its sessions. The server must be stopped: a running one holds the state in -memory and would write over the change. Nothing else is touched, and no -vault is affected — account passwords and vault passwords are different -things. +its sessions. It needs the same `database` and master key as the server +(pass `-config /etc/keeporg/server.json`, or the same environment), and the +server may keep running: it writes to the database like any server process, +and the running one picks the change up within seconds. Nothing else is +touched, and no vault is affected — account passwords and vault passwords +are different things. `/etc/keeporg/server.json`: @@ -626,6 +784,7 @@ things. { "addr": ":8765", "data_dir": "/var/lib/keeporg", + "database": "sqlite:/var/lib/keeporg/keeporg.db", "external_url": "https://keeporg.example.com", "session_ttl_hours": 2160, "max_db_size_mb": 32, @@ -649,17 +808,19 @@ the server, or set `tls_cert` and `tls_key`. ### Environment variables -The server reads four variables of its own: the public URL, and secrets or -first-run material. Everything else — address, data directory, OIDC, +The server reads six variables of its own: where its database and its +public address are, and secrets or first-run material. Everything else — address, data directory, OIDC, limits — is set only in `server.json` or by flag, with flags winning; there is no environment equivalent for those. An empty or whitespace-only value counts as unset. | Variable | Read by | Meaning | | --- | --- | --- | -| `KEEPORG_MASTER_KEY` | every start | The master key itself, base64 of 32 bytes. Takes precedence over everything below. Never written anywhere. A value that does not decode has the same effect as a missing key file. | -| `KEEPORG_MASTER_KEY_FILE` | every start, when `KEEPORG_MASTER_KEY` is unset | Path to a file holding the key, as base64 text or exactly 32 raw bytes. The file must exist and is never created, since a new key would open none of the existing vaults. If it is missing or malformed the server logs `no master key, so this server cannot create or merge vaults` and runs without one: key-less strategies keep working, vault creation and merging are refused. Takes precedence over `master_key_file` / `-master-key-file`. | +| `KEEPORG_MASTER_KEY` | every start | The master key itself, base64 of 32 bytes. Takes precedence over everything below. Never written anywhere. A value that does not decode stops the server from starting. | +| `KEEPORG_MASTER_KEY_FILE` | every start, when `KEEPORG_MASTER_KEY` is unset | Path to a file holding the key, as base64 text or exactly 32 raw bytes. The file must exist and is never created, since a new key would open nothing already stored. If it is missing or malformed the server refuses to start. Takes precedence over `master_key_file` / `-master-key-file`. | +| `KEEPORG_DATABASE_URL` | every start, and `-reset-admin` | Where the data lives, as `database`: `postgres://user:password@host/db?sslmode=…` or `sqlite:`. Overrides the file; `-database` overrides it. A PostgreSQL URL carries a password, so this belongs in a secret. The password is masked in the log. | | `KEEPORG_EXTERNAL_URL` | every start | The base URL a browser reaches the server at, as `external_url`. Overrides the file; `-url` overrides it. Invite links, the device-approval page every client sign-in uses, and the OIDC redirect URI are built from it, and without any of the three it is `http://localhost`, which is wrong behind any proxy. | +| `KEEPORG_TRUSTED_PROXIES` | every start | Comma-separated addresses or CIDR ranges of reverse proxies whose `X-Forwarded-For` is believed, as `trusted_proxies`. Sign-in rate limits count per client address; behind a proxy without this, every client shares the proxy's budget. | | `KEEPORG_ADMIN_PASSWORD` | first start on an empty state, and `-reset-admin` | The password for the built-in `admin` account instead of a generated one, which is then not logged. Ignored once any account exists, so leaving it set is harmless but pointless; unset it after the first start. | With neither master-key variable set, the key comes from `master_key_file` @@ -688,11 +849,27 @@ nginx `server_name`; the image's command passes neither `-config` nor `KEEPORG_ADMIN_PASSWORD` is not set there, so the first start logs a generated password once. -Back up `/var/lib/keeporg` **and** `/etc/keeporg/master.key`, separately. -The data directory is the whole server: `state.json` plus the blobs. The -master key is what turns the wrapped vault keys in it back into vault -passwords; without it a restored server can still serve, but can no longer -merge. +Back up the database **and** `/etc/keeporg/master.key`, separately. The +database is the whole server: with SQLite that is `keeporg.db` (copy it with +`sqlite3 keeporg.db ".backup …"` while the server runs, or stop it and copy +the file with its `-wal`); with PostgreSQL, whatever backs that database up +already. Every row in it is sealed under the master key, so a backup without +the key restores nothing. + +### Moving from 0.10 or earlier + +A server up to 0.10 kept `state.json` and `blobs/` in the data directory. +On the first start of 0.11 with an empty database, it imports both — every +account, session, grant and stored version — so clients and their tokens +carry on without signing in again. The old files are then renamed to +`state.json.imported-