diff --git a/docker/env/core.example.env b/docker/env/core.example.env index 9aafa27a..4047236e 100644 --- a/docker/env/core.example.env +++ b/docker/env/core.example.env @@ -128,7 +128,13 @@ OXYNOTE_CORE_DB_MAX_DOCUMENT_HISTORY_ENTRIES=100 OXYNOTE_CORE_DB_DOCUMENT_HISTORY_RETENTION=2160h # secrets -OXYNOTE_CORE_DB_DATA_SOURCE_CREDENTIALS_SIGNING_SECRET=devsigningsecret +# data source credentials encryption keys: comma-separated, newest first, +# each the standard base64 of exactly 32 random bytes (`openssl rand +# -base64 32`). The first key encrypts, every key decrypts; rotate by +# prepending a key and dropping the old one once core has restarted. +# The committed key below is for the dev stack only; any real deployment +# generates its own. +OXYNOTE_CORE_DB_DATA_SOURCE_CREDENTIALS_KEYS=wPpYGb4f7nmKRSeeVi1L1knqQa3f6pkt5CisctvQYEg= # observability # an empty DSN disables sentry. diff --git a/docker/prod/AGENTS.md b/docker/prod/AGENTS.md index 636cb680..cd027caf 100644 --- a/docker/prod/AGENTS.md +++ b/docker/prod/AGENTS.md @@ -68,7 +68,8 @@ mailpit, since signup needs a verified address. undeclared `OXYNOTE_*` variable is a boot error. - **Secrets precedence** (`launcher/src/secrets.ts`): explicit env (never persisted) → existing volume file → generate and persist 0600. The - data-source encryption key cannot be rotated; never weaken this path. + data-source override may carry a whole keyring; the generated file holds + one key. Never weaken this path. - **Sentry DSNs are build-time only** (esbuild `--define` from Dockerfile ARGs); `OXYNOTE_CRASH_REPORTING_DISABLED` is the only runtime switch. The launcher keeps its own DSN (`bakedLauncherSentryDsn`) to report a child diff --git a/docker/prod/README.md b/docker/prod/README.md index edcde320..8531a907 100644 --- a/docker/prod/README.md +++ b/docker/prod/README.md @@ -94,12 +94,23 @@ owner-only permissions — keep that volume. - Unless an object store is configured, every uploaded image lives there too, under `/oxynote/data/object-storage`. - **Losing the volume signs everyone out, permanently orphans every stored - data-source credential** — the encryption key cannot be rotated — **and - takes the uploaded images, and the embedded database, with it.** + data-source credential, and takes the uploaded images, and the embedded + database, with it.** - Two advanced overrides exist for migrating an existing deployment in: `OXYNOTE_AUTH_SECRET` (session/token signing) and - `OXYNOTE_DATA_SOURCE_ENCRYPTION_KEY` (exactly 16, 24, or 32 bytes). An - override always wins and is never written to disk. + `OXYNOTE_DATA_SOURCE_ENCRYPTION_KEYS` (comma-separated, newest first, + each the standard base64 of exactly 32 bytes). An override always wins + and is never written to disk. +- To rotate the data-source encryption key, read the current key from + `secrets/data-source-encryption-key` on the volume (or from your + existing override), set `OXYNOTE_DATA_SOURCE_ENCRYPTION_KEYS` to + `,` and restart: core re-encrypts every stored + credential under the new key at boot and logs how many it moved. On a + later restart set the override to `` alone. The override is + never written back, so from then on it stays set; to return to the + generated file instead, write `` into + `secrets/data-source-encryption-key` before dropping the override, or + core boots with only the retired key and reads no credential. ## Security model diff --git a/docker/prod/launcher/src/env.test.ts b/docker/prod/launcher/src/env.test.ts index 598ddc92..ad7dc01a 100644 --- a/docker/prod/launcher/src/env.test.ts +++ b/docker/prod/launcher/src/env.test.ts @@ -528,43 +528,67 @@ describe("loadConfig", () => { const config = loadConfig(completeEnv()) expect(config.authSecret).toBeUndefined() - expect(config.dataSourceEncryptionKey).toBeUndefined() + expect(config.dataSourceEncryptionKeys).toBeUndefined() }) it.for([ - { name: "16", input: "0123456789abcdef" }, - { name: "24", input: "0123456789abcdef01234567" }, { - name: "32", - input: "0123456789abcdef0123456789abcdef", + name: "one key", + input: "MDEyMzQ1Njc4OWFiY2RlZjAxMjM0NTY3ODlhYmNkZWY=", + }, + { + name: "two keys", + input: "MDEyMzQ1Njc4OWFiY2RlZjAxMjM0NTY3ODlhYmNkZWY=, ZmVkY2JhOTg3NjU0MzIxMGZlZGNiYTk4NzY1NDMyMTA=", }, ])( - "accepts a $name byte encryption key", + "accepts $name as the encryption keys", ({ input }, { expect }) => { const config = loadConfig( completeEnv({ - OXYNOTE_DATA_SOURCE_ENCRYPTION_KEY: + OXYNOTE_DATA_SOURCE_ENCRYPTION_KEYS: input, }), ) - expect(config.dataSourceEncryptionKey).toBe( + expect(config.dataSourceEncryptionKeys).toBe( input, ) }, ) - it("rejects an encryption key of any other length", ({ - expect, - }) => { - expect(() => - loadConfig( - completeEnv({ - OXYNOTE_DATA_SOURCE_ENCRYPTION_KEY: - "tooshort", - }), - ), - ).toThrow("OXYNOTE_DATA_SOURCE_ENCRYPTION_KEY") - }) + it.for([ + { + name: "a raw 32-byte string", + input: "0123456789abcdef0123456789abcdef", + }, + { + name: "base64 of 16 bytes", + input: "MDEyMzQ1Njc4OWFiY2RlZg==", + }, + { + name: "one bad key among good ones", + input: "MDEyMzQ1Njc4OWFiY2RlZjAxMjM0NTY3ODlhYmNkZWY=,tooshort", + }, + { + name: "non-canonical base64", + input: "MDEyMzQ1Njc4OWFiY2RlZjAxMjM0NTY3ODlhYmNkZWZ=", + }, + { + name: "the same key twice", + input: "MDEyMzQ1Njc4OWFiY2RlZjAxMjM0NTY3ODlhYmNkZWY=,MDEyMzQ1Njc4OWFiY2RlZjAxMjM0NTY3ODlhYmNkZWY=", + }, + ])( + "rejects $name as the encryption keys", + ({ input }, { expect }) => { + expect(() => + loadConfig( + completeEnv({ + OXYNOTE_DATA_SOURCE_ENCRYPTION_KEYS: + input, + }), + ), + ).toThrow("OXYNOTE_DATA_SOURCE_ENCRYPTION_KEYS") + }, + ) }) }) diff --git a/docker/prod/launcher/src/env.ts b/docker/prod/launcher/src/env.ts index 821dbb19..f5d04cf0 100644 --- a/docker/prod/launcher/src/env.ts +++ b/docker/prod/launcher/src/env.ts @@ -84,7 +84,7 @@ export interface Config { crashReportingDisabled: boolean // explicit secret overrides; unset means generate-and-persist. authSecret: string | undefined - dataSourceEncryptionKey: string | undefined + dataSourceEncryptionKeys: string | undefined } // every URL here is dialed or redirected to, so the scheme is pinned: bare @@ -266,19 +266,39 @@ const smtpDsn = z } }) -// the key encrypts stored data-source credentials with AES, which accepts -// exactly these lengths; core never validates it and would fail on the -// first credential save instead. -const encryptionKey = z +// the standard base64 encoding of exactly 32 bytes, as core's strict +// decoder reads it: 43 characters and one pad, and the last character +// carries two zero padding bits, which leaves it 16 of the alphabet. +const dataSourceKeyPattern = /^[A-Za-z0-9+/]{42}[AEIMQUYcgkosw048]=$/ + +// the keys encrypt stored data-source credentials: a comma-separated list, +// newest first, each the standard base64 of exactly 32 bytes and none +// listed twice. Core refuses to boot on anything else, so the check here +// names the public variable. +const encryptionKeys = z .string() .optional() .refine( (value) => value === undefined || - [16, 24, 32].includes( - new TextEncoder().encode(value).length, - ), - { error: "must be exactly 16, 24, or 32 bytes" }, + value + .split(",") + .every((key) => + dataSourceKeyPattern.test(key.trim()), + ), + { error: "every key must be base64 of exactly 32 bytes" }, + ) + .refine( + (value) => { + if (value === undefined) { + return true + } + + const keys = value.split(",").map((key) => key.trim()) + + return new Set(keys).size === keys.length + }, + { error: "lists the same key twice" }, ) // the value core parses with Go's time.ParseDuration. @@ -384,7 +404,7 @@ const baseSchema = z.object({ OXYNOTE_PRIVACY_POLICY_URL: httpUrl().optional(), OXYNOTE_CRASH_REPORTING_DISABLED: flag(), OXYNOTE_AUTH_SECRET: z.string().optional(), - OXYNOTE_DATA_SOURCE_ENCRYPTION_KEY: encryptionKey, + OXYNOTE_DATA_SOURCE_ENCRYPTION_KEYS: encryptionKeys, }) type ParsedValues = z.infer @@ -617,7 +637,7 @@ export function loadConfig(source: Record): Config { privacyPolicyUrl: values.OXYNOTE_PRIVACY_POLICY_URL ?? "", crashReportingDisabled: values.OXYNOTE_CRASH_REPORTING_DISABLED, authSecret: values.OXYNOTE_AUTH_SECRET, - dataSourceEncryptionKey: - values.OXYNOTE_DATA_SOURCE_ENCRYPTION_KEY, + dataSourceEncryptionKeys: + values.OXYNOTE_DATA_SOURCE_ENCRYPTION_KEYS, } } diff --git a/docker/prod/launcher/src/index.ts b/docker/prod/launcher/src/index.ts index b56a9b4b..f88a3323 100644 --- a/docker/prod/launcher/src/index.ts +++ b/docker/prod/launcher/src/index.ts @@ -191,7 +191,7 @@ async function main(): Promise { const { secrets, report } = ensureSecrets(join(dataDir, "secrets"), { authSecret: config.authSecret, - dataSourceEncryptionKey: config.dataSourceEncryptionKey, + dataSourceEncryptionKeys: config.dataSourceEncryptionKeys, }) for (const name of report.generated) { diff --git a/docker/prod/launcher/src/mapping.test.ts b/docker/prod/launcher/src/mapping.test.ts index a4e2b412..458a1da4 100644 --- a/docker/prod/launcher/src/mapping.test.ts +++ b/docker/prod/launcher/src/mapping.test.ts @@ -195,9 +195,8 @@ describe("buildChildEnvs", () => { .OXYNOTE_AUTH_REALTIME_BETTER_AUTH_SECRET, ).toBe("test-auth-secret") expect( - envs.core - .OXYNOTE_CORE_DB_DATA_SOURCE_CREDENTIALS_SIGNING_SECRET, - ).toBe("0123456789abcdef0123456789abcdef") + envs.core.OXYNOTE_CORE_DB_DATA_SOURCE_CREDENTIALS_KEYS, + ).toBe("MDEyMzQ1Njc4OWFiY2RlZjAxMjM0NTY3ODlhYmNkZWY=") expect( Object.values(envs.web).includes("test-auth-secret"), ).toBe(false) diff --git a/docker/prod/launcher/src/mapping.ts b/docker/prod/launcher/src/mapping.ts index 1f7cd0a2..aeedfb46 100644 --- a/docker/prod/launcher/src/mapping.ts +++ b/docker/prod/launcher/src/mapping.ts @@ -164,8 +164,8 @@ export function buildChildEnvs( config.maxDocumentHistoryEntries, OXYNOTE_CORE_DB_DOCUMENT_HISTORY_RETENTION: config.documentHistoryRetention, - OXYNOTE_CORE_DB_DATA_SOURCE_CREDENTIALS_SIGNING_SECRET: - secrets.dataSourceEncryptionKey, + OXYNOTE_CORE_DB_DATA_SOURCE_CREDENTIALS_KEYS: + secrets.dataSourceEncryptionKeys, OXYNOTE_CORE_EMAIL_SMTP_HOST: config.smtp?.host ?? "", OXYNOTE_CORE_EMAIL_SMTP_PORT: config.smtp?.port ?? "", OXYNOTE_CORE_EMAIL_SMTP_USERNAME: config.smtp?.username ?? "", diff --git a/docker/prod/launcher/src/secrets.test.ts b/docker/prod/launcher/src/secrets.test.ts index ed493070..17e2f194 100644 --- a/docker/prod/launcher/src/secrets.test.ts +++ b/docker/prod/launcher/src/secrets.test.ts @@ -16,9 +16,12 @@ function tempDir(): string { const noOverrides = { authSecret: undefined, - dataSourceEncryptionKey: undefined, + dataSourceEncryptionKeys: undefined, } +// a key in the shape ensureSecrets generates. +const validKey = "MDEyMzQ1Njc4OWFiY2RlZjAxMjM0NTY3ODlhYmNkZWY=" + describe("ensureSecrets", () => { it("generates every secret on first boot and persists it", ({ expect, @@ -27,9 +30,19 @@ describe("ensureSecrets", () => { const { secrets, report } = ensureSecrets(dir, noOverrides) - for (const value of Object.values(secrets)) { + // 32 random bytes as standard base64, the one key of a fresh + // keyring + expect(secrets.dataSourceEncryptionKeys).toMatch( + /^[A-Za-z0-9+/]{43}=$/, + ) + + for (const [name, value] of Object.entries(secrets)) { + if (name === "dataSourceEncryptionKeys") { + continue + } + // 24 random bytes as base64url — 32 ASCII characters, - // satisfying the AES-256 and 32-byte signing constraints + // satisfying the 32-byte signing constraints expect(value).toMatch(/^[A-Za-z0-9_-]{32}$/) } @@ -92,7 +105,7 @@ describe("ensureSecrets", () => { const { secrets, report } = ensureSecrets(dir, { authSecret: "operator-provided", - dataSourceEncryptionKey: undefined, + dataSourceEncryptionKeys: undefined, }) expect(secrets.authSecret).toBe("operator-provided") @@ -100,6 +113,35 @@ describe("ensureSecrets", () => { expect(readdirSync(dir)).not.toContain("auth-secret") }) + it("passes an overridden keyring through as it is", ({ expect }) => { + const dir = tempDir() + const keys = `${validKey},ZmVkY2JhOTg3NjU0MzIxMGZlZGNiYTk4NzY1NDMyMTA=` + + const { secrets, report } = ensureSecrets(dir, { + authSecret: undefined, + dataSourceEncryptionKeys: keys, + }) + + expect(secrets.dataSourceEncryptionKeys).toBe(keys) + expect(report.fromEnv).toEqual(["data-source-encryption-key"]) + expect(readdirSync(dir)).not.toContain( + "data-source-encryption-key", + ) + }) + + it("reuses a stored data-source key of the right shape", ({ + expect, + }) => { + const dir = tempDir() + + mkdirSync(dir, { recursive: true }) + writeFileSync(join(dir, "data-source-encryption-key"), validKey) + + const { secrets } = ensureSecrets(dir, noOverrides) + + expect(secrets.dataSourceEncryptionKeys).toBe(validKey) + }) + it("keeps an overridden secret out of the volume across boots", ({ expect, }) => { @@ -107,11 +149,11 @@ describe("ensureSecrets", () => { ensureSecrets(dir, { authSecret: "operator-provided", - dataSourceEncryptionKey: undefined, + dataSourceEncryptionKeys: undefined, }) const { secrets } = ensureSecrets(dir, { authSecret: "operator-provided", - dataSourceEncryptionKey: undefined, + dataSourceEncryptionKeys: undefined, }) expect(secrets.authSecret).toBe("operator-provided") diff --git a/docker/prod/launcher/src/secrets.ts b/docker/prod/launcher/src/secrets.ts index 9cfed35e..32690f6d 100644 --- a/docker/prod/launcher/src/secrets.ts +++ b/docker/prod/launcher/src/secrets.ts @@ -4,7 +4,7 @@ import { join } from "node:path" export interface Secrets { authSecret: string - dataSourceEncryptionKey: string + dataSourceEncryptionKeys: string githubInstallationSigningSecret: string slackInstallationSigningSecret: string databasePassword: string @@ -12,7 +12,7 @@ export interface Secrets { export interface SecretOverrides { authSecret: string | undefined - dataSourceEncryptionKey: string | undefined + dataSourceEncryptionKeys: string | undefined } export interface SecretReport { @@ -41,13 +41,23 @@ function readSecret(path: string): string | undefined { } } +// generateAscii draws 24 random bytes as base64url, which is exactly the 32 +// ASCII bytes core's installation signing secrets must be. +function generateAscii(): string { + return randomBytes(24).toString("base64url") +} + +// generateDataSourceKey draws the 32 random bytes of one AES-256 key, in +// the base64 form core's keyring reads. +function generateDataSourceKey(): string { + return randomBytes(32).toString("base64") +} + // ensureSecrets resolves every internal secret with the precedence the // established all-in-one images use: an explicit override wins and is never // written to disk, an existing volume file is reused, and only a secret // with neither is generated — from the CSPRNG — and persisted with -// owner-only permissions. Everything generated is 32 ASCII bytes, which -// satisfies both the AES-256 key length the data-source encryption needs -// and core's exactly-32-byte installation signing secrets. +// owner-only permissions. export function ensureSecrets( dir: string, overrides: SecretOverrides, @@ -58,7 +68,11 @@ export function ensureSecrets( fromEnv: [], } - function resolve(file: string, override?: string): string { + function resolve( + file: string, + generate: () => string, + override?: string, + ): string { if (override !== undefined) { report.fromEnv.push(file) @@ -74,8 +88,7 @@ export function ensureSecrets( return existing } - // base64url of 24 random bytes is exactly 32 ASCII bytes. - const value = randomBytes(24).toString("base64url") + const value = generate() try { mkdirSync(dir, { recursive: true, mode: 0o700 }) @@ -108,18 +121,25 @@ export function ensureSecrets( } const secrets: Secrets = { - authSecret: resolve("auth-secret", overrides.authSecret), - dataSourceEncryptionKey: resolve( + authSecret: resolve( + "auth-secret", + generateAscii, + overrides.authSecret, + ), + dataSourceEncryptionKeys: resolve( "data-source-encryption-key", - overrides.dataSourceEncryptionKey, + generateDataSourceKey, + overrides.dataSourceEncryptionKeys, ), githubInstallationSigningSecret: resolve( "github-installation-signing-secret", + generateAscii, ), slackInstallationSigningSecret: resolve( "slack-installation-signing-secret", + generateAscii, ), - databasePassword: resolve("database-password"), + databasePassword: resolve("database-password", generateAscii), } return { secrets, report } diff --git a/docker/prod/launcher/src/test-helpers.ts b/docker/prod/launcher/src/test-helpers.ts index 8efcd8be..a78b5932 100644 --- a/docker/prod/launcher/src/test-helpers.ts +++ b/docker/prod/launcher/src/test-helpers.ts @@ -66,7 +66,7 @@ export function testConfig(overrides: Partial = {}): Config { privacyPolicyUrl: "", crashReportingDisabled: false, authSecret: undefined, - dataSourceEncryptionKey: undefined, + dataSourceEncryptionKeys: undefined, ...overrides, } } @@ -74,7 +74,8 @@ export function testConfig(overrides: Partial = {}): Config { export function testSecrets(): Secrets { return { authSecret: "test-auth-secret", - dataSourceEncryptionKey: "0123456789abcdef0123456789abcdef", + dataSourceEncryptionKeys: + "MDEyMzQ1Njc4OWFiY2RlZjAxMjM0NTY3ODlhYmNkZWY=", githubInstallationSigningSecret: "github-installation-secret", slackInstallationSigningSecret: "slack-installation-secret", databasePassword: "test-database-password", diff --git a/e2e/docker-compose.dev.yaml b/e2e/docker-compose.dev.yaml index 62bfaab9..4393e1a5 100644 --- a/e2e/docker-compose.dev.yaml +++ b/e2e/docker-compose.dev.yaml @@ -118,7 +118,7 @@ services: - OXYNOTE_CORE_SERVER_PUBLIC_URL=http://localhost:18080/core - OXYNOTE_CORE_BASE_APP_URL=http://localhost:18080 - OXYNOTE_CORE_ORIGINS=http://localhost:18080,localhost:18080 - - OXYNOTE_CORE_DB_DATA_SOURCE_CREDENTIALS_SIGNING_SECRET=e2esigningsecret + - OXYNOTE_CORE_DB_DATA_SOURCE_CREDENTIALS_KEYS=49i0eQzCx6VZfY7ZSravUYR4CvvsDtkQlG4wPnizm+Y= - OXYNOTE_CORE_EMAIL_SMTP_HOST=mailpit - OXYNOTE_CORE_EMAIL_SMTP_PORT=1025 - OXYNOTE_CORE_EMAIL_SMTP_TLS=none diff --git a/server/core/cmd/core/main.go b/server/core/cmd/core/main.go index 3a7d62ea..1bdf561c 100644 --- a/server/core/cmd/core/main.go +++ b/server/core/cmd/core/main.go @@ -41,6 +41,7 @@ import ( "github.com/oxynote/oxynote/server/core/internal/storage" storageFS "github.com/oxynote/oxynote/server/core/internal/storage/fs" storageS3 "github.com/oxynote/oxynote/server/core/internal/storage/s3" + "github.com/oxynote/oxynote/server/core/pkg/cryptoutil" "github.com/oxynote/oxynote/server/core/pkg/ioutil" "github.com/oxynote/oxynote/server/core/pkg/logutil" "github.com/oxynote/oxynote/server/core/pkg/metricutil" @@ -117,13 +118,19 @@ func main() { //nolint:maintidx // main performs linear wiring of all components return } + dataSourceKeys, err := cryptoutil.ParseKeyring(buildinfo.Getenv("DB_DATA_SOURCE_CREDENTIALS_KEYS")) + if err != nil { + fail(log, closers, "invalid "+buildinfo.EnvName("DB_DATA_SOURCE_CREDENTIALS_KEYS"), err) + return + } + dbc, err := db.New(log, metrics, db.Options{ - DSN: buildinfo.Getenv("DB_DSN"), - MaxNotifications: _maxNotifications, - MaxSlackMessages: _maxSlackMessages, - MaxDocumentHistoryEntries: maxDocumentHistoryEntries, - DocumentHistoryRetention: documentHistoryRetention, - DataSourceCredentialsSigningSecret: buildinfo.Getenv("DB_DATA_SOURCE_CREDENTIALS_SIGNING_SECRET"), + DSN: buildinfo.Getenv("DB_DSN"), + MaxNotifications: _maxNotifications, + MaxSlackMessages: _maxSlackMessages, + MaxDocumentHistoryEntries: maxDocumentHistoryEntries, + DocumentHistoryRetention: documentHistoryRetention, + DataSourceCredentialsKeys: dataSourceKeys, }) if err != nil { fail(log, closers, "cannot create a database connection", err) @@ -132,6 +139,23 @@ func main() { //nolint:maintidx // main performs linear wiring of all components closers = append([]io.Closer{dbc}, closers...) + // a key prepended to the keyring is only safe to drop once nothing is + // sealed under the one it replaced, so every row moves before the + // process serves anything. + moved, unreadable, err := dbc.ReencryptDataSourceCredentials(context.Background()) + if err != nil { + fail(log, closers, "cannot re-encrypt data source credentials", err) + return + } + + if moved > 0 { + log.Info("moved data source credentials to the newest key", "count", moved) + } + + if unreadable > 0 { + log.Warn("data source credentials no configured key can read", "count", unreadable) + } + // an empty VALKEY_DSN is a deployment running without valkey at // all. The nil pool is what tells the assistant to keep its // conversations in this process instead. diff --git a/server/core/internal/datasource/datasource_test.go b/server/core/internal/datasource/datasource_test.go index efd5067c..92859ab1 100644 --- a/server/core/internal/datasource/datasource_test.go +++ b/server/core/internal/datasource/datasource_test.go @@ -1,11 +1,13 @@ package datasource import ( + "encoding/base64" "testing" "time" "github.com/guregu/null/v5" "github.com/oxynote/oxynote/server/core/internal/datasource/processor" + "github.com/oxynote/oxynote/server/core/pkg/cryptoutil" "github.com/oxynote/oxynote/server/core/pkg/testutil" "github.com/oxynote/oxynote/server/core/pkg/timeutil" "github.com/rs/xid" @@ -23,8 +25,11 @@ func TestMain(m *testing.M) { func undecryptableCredentials(t *testing.T) processor.Credentials { t.Helper() + keys, err := cryptoutil.ParseKeyring(base64.StdEncoding.EncodeToString([]byte("0123456789abcdef0123456789abcdef"))) + require.NoError(t, err) + creds := processor.NewCredentials([]byte("ciphertext")) - require.Error(t, creds.Decrypt("0123456789abcdef0123456789abcdef")) + require.Error(t, creds.Decrypt(keys, nil)) return creds } diff --git a/server/core/internal/datasource/processor/processor.go b/server/core/internal/datasource/processor/processor.go index 1ce827da..077376da 100644 --- a/server/core/internal/datasource/processor/processor.go +++ b/server/core/internal/datasource/processor/processor.go @@ -45,9 +45,9 @@ const ( // ConnectionStatusNotReadOnly indicates that the connection is not read-only. ConnectionStatusNotReadOnly ConnectionStatus = "not_read_only" - // ConnectionStatusInvalidSigningSecret indicates that the stored credentials - // cannot be decrypted with the configured signing secret. - ConnectionStatusInvalidSigningSecret ConnectionStatus = "invalid_signing_secret" + // ConnectionStatusInvalidEncryptionKey indicates that the stored + // credentials cannot be decrypted with the configured keys. + ConnectionStatusInvalidEncryptionKey ConnectionStatus = "invalid_encryption_key" ) // Error returns an error corresponding to the ConnectionStatus. @@ -61,11 +61,11 @@ func (cs ConnectionStatus) Error() error { return errutil.New(http.StatusBadRequest, "data_source.version_not_supported", "The data source version is not supported.") case ConnectionStatusNotReadOnly: return errutil.New(http.StatusBadRequest, "data_source.not_read_only", "The data source connection must be read-only.") - case ConnectionStatusInvalidSigningSecret: + case ConnectionStatusInvalidEncryptionKey: return errutil.New( http.StatusBadRequest, - "data_source.invalid_signing_secret", - "The data source credentials cannot be decrypted and must be entered again.", + "data_source.invalid_encryption_key", + "The data source credentials cannot be decrypted with the configured keys and must be entered again.", ) default: return nil @@ -89,8 +89,8 @@ type Credentials struct { data []byte // unreadable indicates that the stored credentials could not be - // decrypted, which is what a signing secret rotated since they were - // written leaves behind. Only Decrypt can decide it, so credentials + // decrypted, which is what a key dropped since they were written + // leaves behind. Only Decrypt can decide it, so credentials // carrying it are the ones that came out of a failed read and no // others. unreadable bool @@ -143,39 +143,42 @@ func (c *Credentials) Scan(src any) error { return nil } -// Encrypt encrypts the credentials using the provided signing key. +// Encrypt encrypts the credentials under the keyring's newest key, bound to +// the associated data, which is the owning row's id. // // Credentials that could not be read encrypt to nothing, so writing them back -// would replace the stored ciphertext — still readable once the right secret +// would replace the stored ciphertext — still readable once the right key // returns — with an encryption of the emptiness left in its place. They are // refused instead. -func (c *Credentials) Encrypt(signingKey string) ([]byte, error) { +func (c *Credentials) Encrypt(keys *cryptoutil.Keyring, aad []byte) ([]byte, error) { if !c.IsValid() { return nil, errors.New("cannot encrypt credentials that could not be decrypted") } - state, err := cryptoutil.EncryptText(string(c.data), []byte(signingKey)) + sealed, err := keys.Encrypt(c.data, aad) if err != nil { + // NOCOV: crypto/rand failures cannot be simulated in tests. return nil, fmt.Errorf("failed to encrypt credentials: %w", err) } - return []byte(state), nil + return sealed, nil } -// Decrypt decrypts the stored credentials using the provided signing key. +// Decrypt decrypts the stored credentials with whichever key of the keyring +// sealed them, given the same associated data Encrypt was handed. // // Credentials it cannot read are emptied and marked unreadable rather than // left holding ciphertext nobody can use, so every later reader is told the // same thing the error says here. -func (c *Credentials) Decrypt(signingKey string) error { - decrypted, err := cryptoutil.DecryptText(string(c.data), []byte(signingKey)) +func (c *Credentials) Decrypt(keys *cryptoutil.Keyring, aad []byte) error { + decrypted, err := keys.Decrypt(c.data, aad) if err != nil { *c = Credentials{unreadable: true} return fmt.Errorf("failed to decrypt credentials: %w", err) } - *c = Credentials{data: []byte(decrypted)} + *c = Credentials{data: decrypted} return nil } diff --git a/server/core/internal/datasource/processor/processor_test.go b/server/core/internal/datasource/processor/processor_test.go index 4e845b3b..972af8e4 100644 --- a/server/core/internal/datasource/processor/processor_test.go +++ b/server/core/internal/datasource/processor/processor_test.go @@ -3,6 +3,7 @@ package processor import ( "context" "database/sql" + "encoding/base64" "encoding/json" "fmt" "io" @@ -14,6 +15,7 @@ import ( mysqlDriver "github.com/go-sql-driver/mysql" "github.com/orlangure/gnomock" pgDocker "github.com/orlangure/gnomock/preset/postgres" + "github.com/oxynote/oxynote/server/core/pkg/cryptoutil" "github.com/oxynote/oxynote/server/core/pkg/errutil" "github.com/oxynote/oxynote/server/core/pkg/testutil" "github.com/stretchr/testify/assert" @@ -21,8 +23,18 @@ import ( "go.uber.org/goleak" ) -// _testSigningKey is a 32-byte AES key used by credentials crypto tests. -const _testSigningKey = "01234567890123456789012345678901" +// _testKeys is the keyring used by credentials crypto tests. +var _testKeys = func() *cryptoutil.Keyring { + keys, err := cryptoutil.ParseKeyring(base64.StdEncoding.EncodeToString([]byte("01234567890123456789012345678901"))) + if err != nil { + panic("cannot parse test keyring: " + err.Error()) + } + + return keys +}() + +// _testAAD is the associated data credentials crypto tests seal with. +var _testAAD = []byte("data-source-id") const ( // _testDB is the database name used in the throwaway containers. @@ -197,12 +209,12 @@ func Test_ConnectionStatus_Error(t *testing.T) { Status: ConnectionStatusNotReadOnly, Err: errutil.New(http.StatusBadRequest, "data_source.not_read_only", "The data source connection must be read-only."), }, - "Invalid signing secret status": { - Status: ConnectionStatusInvalidSigningSecret, + "Invalid encryption key status": { + Status: ConnectionStatusInvalidEncryptionKey, Err: errutil.New( http.StatusBadRequest, - "data_source.invalid_signing_secret", - "The data source credentials cannot be decrypted and must be entered again.", + "data_source.invalid_encryption_key", + "The data source credentials cannot be decrypted with the configured keys and must be entered again.", ), }, "Success status": { @@ -260,8 +272,8 @@ func Test_Credentials_IsValid(t *testing.T) { assert.True(t, empty.IsValid()) // only a decrypt that could not make sense of them says otherwise. - unreadable := NewCredentials([]byte("not-hex-encoded")) - require.Error(t, unreadable.Decrypt(_testSigningKey)) + unreadable := NewCredentials([]byte("not-sealed")) + require.Error(t, unreadable.Decrypt(_testKeys, _testAAD)) assert.False(t, unreadable.IsValid()) } @@ -319,25 +331,21 @@ func Test_Credentials_Encrypt(t *testing.T) { c := NewCredentials([]byte(`{"username":"user"}`)) - // error - _, err := c.Encrypt("short-key") - assert.Error(t, err) - // success - data, err := c.Encrypt(_testSigningKey) + data, err := c.Encrypt(_testKeys, _testAAD) require.NoError(t, err) require.NotEmpty(t, data) decrypted := NewCredentials(data) - require.NoError(t, decrypted.Decrypt(_testSigningKey)) + require.NoError(t, decrypted.Decrypt(_testKeys, _testAAD)) assert.Equal(t, c, decrypted) // credentials a decrypt could not read are refused, so the ciphertext // they came from is never overwritten by an encryption of nothing. - unreadable := NewCredentials([]byte("not-hex-encoded")) - require.Error(t, unreadable.Decrypt(_testSigningKey)) + unreadable := NewCredentials([]byte("not-sealed")) + require.Error(t, unreadable.Decrypt(_testKeys, _testAAD)) - _, err = unreadable.Encrypt(_testSigningKey) + _, err = unreadable.Encrypt(_testKeys, _testAAD) assert.Error(t, err) } @@ -345,22 +353,27 @@ func Test_Credentials_Decrypt(t *testing.T) { t.Parallel() // error - c := NewCredentials([]byte("not-hex-encoded")) - assert.Error(t, c.Decrypt(_testSigningKey)) + c := NewCredentials([]byte("not-sealed")) + assert.Error(t, c.Decrypt(_testKeys, _testAAD)) // what it could not read is dropped rather than left as ciphertext // nobody can use, and says so to every later reader. assert.False(t, c.IsValid()) assert.Equal(t, Credentials{unreadable: true}, c) - // success + // error - sealed for another row original := NewCredentials([]byte(`{"username":"user"}`)) - data, err := original.Encrypt(_testSigningKey) + data, err := original.Encrypt(_testKeys, _testAAD) require.NoError(t, err) c = NewCredentials(data) - require.NoError(t, c.Decrypt(_testSigningKey)) + assert.Error(t, c.Decrypt(_testKeys, []byte("other-id"))) + assert.False(t, c.IsValid()) + + // success + c = NewCredentials(data) + require.NoError(t, c.Decrypt(_testKeys, _testAAD)) assert.Equal(t, original, c) } diff --git a/server/core/internal/datasource/runner.go b/server/core/internal/datasource/runner.go index f8ce153c..66932392 100644 --- a/server/core/internal/datasource/runner.go +++ b/server/core/internal/datasource/runner.go @@ -78,7 +78,7 @@ func (r *runner) Type() Type { // data source in use, which is the observation worth keeping. func (r *runner) TestConnection(ctx context.Context) (processor.ConnectionStatus, error) { if !r.ds.Credentials.IsValid() { - return processor.ConnectionStatusInvalidSigningSecret, nil + return processor.ConnectionStatusInvalidEncryptionKey, nil } if err := r.ensurePrepared(); err != nil { @@ -131,9 +131,9 @@ func connect[C any](ctx context.Context, r *runner, allowed ...Type) (C, error) // attempting: there is nothing left to authenticate with, and the data // source would answer for the empty login rather than for the reason. if !r.ds.Credentials.IsValid() { - r.recordStatus(ctx, processor.ConnectionStatusInvalidSigningSecret) + r.recordStatus(ctx, processor.ConnectionStatusInvalidEncryptionKey) - return zero, processor.ConnectionStatusInvalidSigningSecret.Error() + return zero, processor.ConnectionStatusInvalidEncryptionKey.Error() } if err := r.ensurePrepared(); err != nil { diff --git a/server/core/internal/datasource/runner_test.go b/server/core/internal/datasource/runner_test.go index f09ad872..d63080ec 100644 --- a/server/core/internal/datasource/runner_test.go +++ b/server/core/internal/datasource/runner_test.go @@ -166,7 +166,7 @@ func Test_runner_TestConnection(t *testing.T) { "Credentials that cannot be decrypted": { Type: TypePrometheus, Undecryptable: true, - Result: processor.ConnectionStatusInvalidSigningSecret, + Result: processor.ConnectionStatusInvalidEncryptionKey, }, "A connection that answers": { Type: TypePrometheus, @@ -355,11 +355,11 @@ func Test_connect(t *testing.T) { r.ds.Credentials = undecryptableCredentials(t) _, err = connect[Prometheus](context.Background(), r, TypePrometheus) - assert.Equal(t, processor.ConnectionStatusInvalidSigningSecret.Error(), err) + assert.Equal(t, processor.ConnectionStatusInvalidEncryptionKey.Error(), err) ff = store.UpdateDataSourceStatusCalls() require.Len(t, ff, 1) - assert.Equal(t, processor.ConnectionStatusInvalidSigningSecret, ff[0].Status) + assert.Equal(t, processor.ConnectionStatusInvalidEncryptionKey, ff[0].Status) // the wanted client is not one this data source serves _, err = connect[Prometheus](context.Background(), prepRunner(TypePostgreSQL, "", nil), TypePrometheus) diff --git a/server/core/internal/db/data_source.go b/server/core/internal/db/data_source.go index d0ff235e..3b478141 100644 --- a/server/core/internal/db/data_source.go +++ b/server/core/internal/db/data_source.go @@ -12,7 +12,7 @@ import ( // InsertDataSource inserts a data source into the database. func (a *agent) InsertDataSource(ctx context.Context, ds *datasource.DataSource) error { - safeCredentials, err := ds.Credentials.Encrypt(a.opts.DataSourceCredentialsSigningSecret) + safeCredentials, err := ds.Credentials.Encrypt(a.opts.DataSourceCredentialsKeys, ds.ID.Bytes()) if err != nil { return err } @@ -38,7 +38,7 @@ func (a *agent) InsertDataSource(ctx context.Context, ds *datasource.DataSource) // UpdateDataSource updates an existing data source in the database. func (a *agent) UpdateDataSource(ctx context.Context, ds *datasource.DataSource) error { - safeCredentials, err := ds.Credentials.Encrypt(a.opts.DataSourceCredentialsSigningSecret) + safeCredentials, err := ds.Credentials.Encrypt(a.opts.DataSourceCredentialsKeys, ds.ID.Bytes()) if err != nil { return err } @@ -90,6 +90,70 @@ func (a *agent) UpdateDataSourceStatus( return err } +// ReencryptDataSourceCredentials moves every data source's credentials onto +// the newest key and reports how many rows it rewrote and how many no key +// reads. It is what lets an operator drop a retired key: once it returns, +// nothing is sealed under any other key. +// +// It deliberately spans every organization: it belongs to boot, and no +// consumer interface lists it. Rows already on the newest key are left +// alone without being decrypted. Unreadable rows keep their ciphertext, as +// every read does, and surface through their status on first use. +func (a *agent) ReencryptDataSourceCredentials(ctx context.Context) (int, int, error) { + keys := a.opts.DataSourceCredentialsKeys + + var moved, unreadable int + + q, args := a.builder.Select("id", "credentials"). + From("data_sources"). + MustSql() + + var rows []struct { + ID xid.ID `db:"id"` + Credentials []byte `db:"credentials"` + } + + if err := sqlx.SelectContext(ctx, a.sql, &rows, q, args...); err != nil { + return 0, 0, err + } + + for _, row := range rows { + if keys.IsCurrent(row.Credentials) { + continue + } + + plaintext, err := keys.Decrypt(row.Credentials, row.ID.Bytes()) + if err != nil { + unreadable++ + + continue + } + + sealed, err := keys.Encrypt(plaintext, row.ID.Bytes()) + if err != nil { + // NOCOV: crypto/rand failures cannot be simulated in tests. + return moved, unreadable, err + } + + q, args := a.builder.Update("data_sources"). + SetMap(map[string]any{ + "credentials": sealed, + }). + Where(sq.Eq{ + "id": row.ID, + }). + MustSql() + + if _, err := a.sql.ExecContext(ctx, q, args...); err != nil { + return moved, unreadable, err + } + + moved++ + } + + return moved, unreadable, nil +} + // DeleteDataSource removes a data source from the database. func (a *agent) DeleteDataSource(ctx context.Context, id xid.ID, organizationID string) error { q, args := a.builder.Delete("data_sources"). @@ -119,7 +183,7 @@ func (a *agent) FetchDataSource(ctx context.Context, id xid.ID, organizationID s return nil, err } - if err := ds.Credentials.Decrypt(a.opts.DataSourceCredentialsSigningSecret); err != nil { + if err := ds.Credentials.Decrypt(a.opts.DataSourceCredentialsKeys, ds.ID.Bytes()); err != nil { if merr := a.markCredentialsUndecryptable(ctx, ds); merr != nil { return nil, merr } @@ -144,7 +208,7 @@ func (a *agent) FetchDataSources(ctx context.Context, organizationID string) ([] } for i := range sources { - if err := sources[i].Credentials.Decrypt(a.opts.DataSourceCredentialsSigningSecret); err != nil { + if err := sources[i].Credentials.Decrypt(a.opts.DataSourceCredentialsKeys, sources[i].ID.Bytes()); err != nil { if merr := a.markCredentialsUndecryptable(ctx, &sources[i]); merr != nil { return nil, merr } @@ -155,19 +219,19 @@ func (a *agent) FetchDataSources(ctx context.Context, organizationID string) ([] } // markCredentialsUndecryptable records on a data source's row that its stored -// credentials cannot be read with the configured signing secret. The failed -// Decrypt has already marked the credentials themselves. +// credentials cannot be read with the configured keys. The failed Decrypt has +// already marked the credentials themselves. // -// The ciphertext is left alone — a secret restored later decrypts it again — -// but the data source travels carrying the status that says why, so a rotated -// secret surfaces as one broken connection rather than as a read that fails -// for everything the organization owns. +// The ciphertext is left alone — a key restored later decrypts it again — but +// the data source travels carrying the status that says why, so a dropped +// key surfaces as one broken connection rather than as a read that fails for +// everything the organization owns. func (a *agent) markCredentialsUndecryptable(ctx context.Context, ds *datasource.DataSource) error { - if ds.Status == processor.ConnectionStatusInvalidSigningSecret { + if ds.Status == processor.ConnectionStatusInvalidEncryptionKey { return nil } - ds.Status = processor.ConnectionStatusInvalidSigningSecret + ds.Status = processor.ConnectionStatusInvalidEncryptionKey return a.UpdateDataSourceStatus(ctx, ds.ID, ds.OrganizationID, ds.Status) } diff --git a/server/core/internal/db/data_source_test.go b/server/core/internal/db/data_source_test.go index b27bca58..92c7b4be 100644 --- a/server/core/internal/db/data_source_test.go +++ b/server/core/internal/db/data_source_test.go @@ -49,7 +49,7 @@ func prepDataSources(t *testing.T, db *DB, count int, fn func(int, *datasource.D res[i] = ds - safeCredentials, err := ds.Credentials.Encrypt(db.opts.DataSourceCredentialsSigningSecret) + safeCredentials, err := ds.Credentials.Encrypt(db.opts.DataSourceCredentialsKeys, ds.ID.Bytes()) require.NoError(t, err) q, args := db.builder.Insert("data_sources"). @@ -87,19 +87,19 @@ func Test_agent_InsertDataSource(t *testing.T) { } type tcase struct { - InvalidSecret bool - DataSource *datasource.DataSource - Err error + DataSource *datasource.DataSource + Err error } cc := map[string]func(*testing.T, *DB) tcase{ - "Invalid signing secret": func(t *testing.T, db *DB) tcase { - org := prepOrganizations(t, db, 1)[0] + "Credentials that could not be read are refused": func(t *testing.T, db *DB) tcase { + ds := stubDataSource(prepOrganizations(t, db, 1)[0], "Data Source 1") + ds.Credentials = processor.NewCredentials([]byte("garbage")) + require.Error(t, ds.Credentials.Decrypt(_dataSourceKeys, ds.ID.Bytes())) return tcase{ - InvalidSecret: true, - DataSource: stubDataSource(org, "Data Source 1"), - Err: assert.AnError, + DataSource: ds, + Err: assert.AnError, } }, "Duplicate name within the organization": func(t *testing.T, db *DB) tcase { @@ -130,10 +130,6 @@ func Test_agent_InsertDataSource(t *testing.T) { db := prepTempDB(t) c := cfn(t, db) - if c.InvalidSecret { - db.opts.DataSourceCredentialsSigningSecret = "bad" - } - err := db.InsertDataSource(context.Background(), c.DataSource) testutil.RequireEqualError(t, c.Err, err) @@ -150,21 +146,11 @@ func Test_agent_InsertDataSource(t *testing.T) { func Test_agent_UpdateDataSource(t *testing.T) { type tcase struct { - InvalidSecret bool - DataSource *datasource.DataSource - Err error + DataSource *datasource.DataSource + Err error } cc := map[string]func(*testing.T, *DB) tcase{ - "Invalid signing secret": func(t *testing.T, db *DB) tcase { - ds := prepDataSources(t, db, 1, nil)[0] - - return tcase{ - InvalidSecret: true, - DataSource: ds, - Err: assert.AnError, - } - }, "Successful update": func(t *testing.T, db *DB) tcase { ds := prepDataSources(t, db, 1, nil)[0] ds.Name = "Updated Data Source" @@ -186,10 +172,6 @@ func Test_agent_UpdateDataSource(t *testing.T) { db := prepTempDB(t) c := cfn(t, db) - if c.InvalidSecret { - db.opts.DataSourceCredentialsSigningSecret = "bad" - } - err := db.UpdateDataSource(context.Background(), c.DataSource) testutil.RequireEqualError(t, c.Err, err) @@ -269,6 +251,112 @@ func Test_agent_UpdateDataSource(t *testing.T) { }) } +func Test_agent_ReencryptDataSourceCredentials(t *testing.T) { + type tcase struct { + CancelledContext bool + Moved int + Unreadable int + Err error + // Check inspects the rows the sweep left behind. + Check func(t *testing.T, db *DB) + } + + cc := map[string]func(*testing.T, *DB) tcase{ + "Cancelled context": func(t *testing.T, db *DB) tcase { + prepDataSources(t, db, 1, nil) + + return tcase{ + CancelledContext: true, + Err: assert.AnError, + } + }, + "No data sources": func(*testing.T, *DB) tcase { + return tcase{} + }, + "Rows on the newest key are left alone": func(t *testing.T, db *DB) tcase { + sources := prepDataSources(t, db, 2, nil) + before := [][]byte{ + fetchDataSourceCredentials(t, db, sources[0].ID), + fetchDataSourceCredentials(t, db, sources[1].ID), + } + + return tcase{ + Check: func(t *testing.T, db *DB) { + assert.Equal(t, before[0], fetchDataSourceCredentials(t, db, sources[0].ID)) + assert.Equal(t, before[1], fetchDataSourceCredentials(t, db, sources[1].ID)) + }, + } + }, + "Rows sealed under a retired key are moved": func(t *testing.T, db *DB) tcase { + // the rows are written while the retired key was the only + // one, then the keyring gains the key that replaces it. + db.opts.DataSourceCredentialsKeys = mustParseKeys(_retiredDataSourceKey) + retired := prepDataSources(t, db, 1, nil)[0] + + db.opts.DataSourceCredentialsKeys = mustParseKeys(_dataSourceKey + "," + _retiredDataSourceKey) + current := prepDataSources(t, db, 1, nil)[0] + + return tcase{ + Moved: 1, + Check: func(t *testing.T, db *DB) { + assert.True(t, db.opts.DataSourceCredentialsKeys.IsCurrent(fetchDataSourceCredentials(t, db, retired.ID))) + assert.True(t, db.opts.DataSourceCredentialsKeys.IsCurrent(fetchDataSourceCredentials(t, db, current.ID))) + + // only the ciphertext changed: the row still reads as + // it was written, and nothing the user sees moved. + db.opts.DataSourceCredentialsKeys = _dataSourceKeys + + res, err := db.FetchDataSource(context.Background(), retired.ID, retired.OrganizationID) + require.NoError(t, err) + testutil.AssertFilterEqual(t, retired, res) + }, + } + }, + "Unreadable rows are skipped": func(t *testing.T, db *DB) tcase { + ds := prepDataSources(t, db, 1, nil)[0] + poisonDataSourceCredentials(t, db, ds.ID) + + return tcase{ + Unreadable: 1, + Check: func(t *testing.T, db *DB) { + assert.Equal(t, []byte("garbage"), fetchDataSourceCredentials(t, db, ds.ID)) + assert.Equal(t, processor.ConnectionStatusSuccess, fetchDataSourceStatus(t, db, ds.ID)) + }, + } + }, + } + + for cn, cfn := range cc { + t.Run(cn, func(t *testing.T) { + t.Parallel() + + db := prepTempDB(t) + c := cfn(t, db) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + if c.CancelledContext { + cancel() + } + + moved, unreadable, err := db.ReencryptDataSourceCredentials(ctx) + testutil.RequireEqualError(t, c.Err, err) + + if err != nil { + return + } + + assert.Equal(t, c.Moved, moved) + assert.Equal(t, c.Unreadable, unreadable) + + if c.Check != nil { + c.Check(t, db) + } + }) + } +} + func Test_agent_DeleteDataSource(t *testing.T) { type tcase struct { CancelledContext bool @@ -334,8 +422,8 @@ func Test_agent_DeleteDataSource(t *testing.T) { } // poisonDataSourceCredentials overwrites a data source's stored credentials -// with bytes no signing secret decrypts, which is what a secret rotated since -// the credentials were written leaves behind. +// with bytes no key decrypts, which is what a key dropped since the +// credentials were written leaves behind. func poisonDataSourceCredentials(t *testing.T, db *DB, id xid.ID) { t.Helper() @@ -371,26 +459,45 @@ func fetchDataSourceStatus(t *testing.T, db *DB, id xid.ID) processor.Connection return status } -// rejectInvalidSigningSecretStatus installs a constraint the status cannot +// rejectInvalidEncryptionKeyStatus installs a constraint the status cannot // satisfy, so the recording of an undecryptable data source fails while the // row itself still reads fine. It is dropped again when the test ends. -func rejectInvalidSigningSecretStatus(t *testing.T, db *DB) { +func rejectInvalidEncryptionKeyStatus(t *testing.T, db *DB) { t.Helper() // NOT VALID leaves the rows already carrying the status alone; the // constraint still refuses every write that arrives after it. _, err := db.sql.Exec( - `ALTER TABLE data_sources ADD CONSTRAINT status_not_invalid_signing_secret ` + - `CHECK (status <> 'invalid_signing_secret') NOT VALID`, + `ALTER TABLE data_sources ADD CONSTRAINT status_not_invalid_encryption_key ` + + `CHECK (status <> 'invalid_encryption_key') NOT VALID`, ) require.NoError(t, err) t.Cleanup(func() { - _, err := db.sql.Exec(`ALTER TABLE data_sources DROP CONSTRAINT status_not_invalid_signing_secret`) + _, err := db.sql.Exec(`ALTER TABLE data_sources DROP CONSTRAINT status_not_invalid_encryption_key`) require.NoError(t, err) }) } +// fetchDataSourceCredentials reads a data source's stored ciphertext as it +// is. +func fetchDataSourceCredentials(t *testing.T, db *DB, id xid.ID) []byte { + t.Helper() + + q, args := db.builder.Select("credentials"). + From("data_sources"). + Where(sq.Eq{ + "id": id, + }). + MustSql() + + var credentials []byte + + require.NoError(t, db.sql.Get(&credentials, q, args...)) + + return credentials +} + func Test_agent_FetchDataSource(t *testing.T) { db := prepTempDB(t) @@ -407,20 +514,20 @@ func Test_agent_FetchDataSource(t *testing.T) { require.NoError(t, err) require.NotNil(t, res) assert.False(t, res.Credentials.IsValid()) - assert.Equal(t, processor.ConnectionStatusInvalidSigningSecret, res.Status) - assert.Equal(t, processor.ConnectionStatusInvalidSigningSecret, fetchDataSourceStatus(t, db, ds.ID)) + assert.Equal(t, processor.ConnectionStatusInvalidEncryptionKey, res.Status) + assert.Equal(t, processor.ConnectionStatusInvalidEncryptionKey, fetchDataSourceStatus(t, db, ds.ID)) // success - undecryptable credentials already recorded res, err = db.FetchDataSource(context.Background(), ds.ID, ds.OrganizationID) require.NoError(t, err) require.NotNil(t, res) assert.False(t, res.Credentials.IsValid()) - assert.Equal(t, processor.ConnectionStatusInvalidSigningSecret, res.Status) + assert.Equal(t, processor.ConnectionStatusInvalidEncryptionKey, res.Status) // error - undecryptable credentials cannot be recorded ds = prepDataSources(t, db, 1, nil)[0] poisonDataSourceCredentials(t, db, ds.ID) - rejectInvalidSigningSecretStatus(t, db) + rejectInvalidEncryptionKeyStatus(t, db) res, err = db.FetchDataSource(context.Background(), ds.ID, ds.OrganizationID) require.Error(t, err) @@ -465,8 +572,8 @@ func Test_agent_FetchDataSources(t *testing.T) { testutil.AssertFilterEqual(t, *mixed[0], res[0]) assert.False(t, res[1].Credentials.IsValid()) - assert.Equal(t, processor.ConnectionStatusInvalidSigningSecret, res[1].Status) - assert.Equal(t, processor.ConnectionStatusInvalidSigningSecret, fetchDataSourceStatus(t, db, mixed[1].ID)) + assert.Equal(t, processor.ConnectionStatusInvalidEncryptionKey, res[1].Status) + assert.Equal(t, processor.ConnectionStatusInvalidEncryptionKey, fetchDataSourceStatus(t, db, mixed[1].ID)) // error - undecryptable credentials cannot be recorded org = prepOrganizations(t, db, 1)[0] @@ -475,7 +582,7 @@ func Test_agent_FetchDataSources(t *testing.T) { })[0] poisonDataSourceCredentials(t, db, ds.ID) - rejectInvalidSigningSecretStatus(t, db) + rejectInvalidEncryptionKeyStatus(t, db) res, err = db.FetchDataSources(context.Background(), org) require.Error(t, err) diff --git a/server/core/internal/db/db.go b/server/core/internal/db/db.go index 17b57572..a4a13e84 100644 --- a/server/core/internal/db/db.go +++ b/server/core/internal/db/db.go @@ -22,6 +22,7 @@ import ( "github.com/jmoiron/sqlx" "github.com/lann/builder" "github.com/oxynote/oxynote/server/core/internal/tag" + "github.com/oxynote/oxynote/server/core/pkg/cryptoutil" "github.com/oxynote/oxynote/server/core/pkg/errutil" "github.com/oxynote/oxynote/server/core/pkg/ioutil" "github.com/oxynote/oxynote/server/core/pkg/metricutil" @@ -103,9 +104,11 @@ type Options struct { // Zero means no age limit. DocumentHistoryRetention time.Duration - // DataSourceCredentialsSigningSecret is the secret used to sign - // data source credentials. - DataSourceCredentialsSigningSecret string + // DataSourceCredentialsKeys holds the keys data source credentials + // are encrypted with. The first key encrypts; every key decrypts. + // It must be set: New does not check it, and a nil keyring panics on + // the first credential written or read. + DataSourceCredentialsKeys *cryptoutil.Keyring } // New creates a fresh instance of database. diff --git a/server/core/internal/db/db_test.go b/server/core/internal/db/db_test.go index b6907c57..8d5e87c4 100644 --- a/server/core/internal/db/db_test.go +++ b/server/core/internal/db/db_test.go @@ -20,6 +20,7 @@ import ( "github.com/orlangure/gnomock" pgDocker "github.com/orlangure/gnomock/preset/postgres" "github.com/oxynote/oxynote/server/core/internal/tag" + "github.com/oxynote/oxynote/server/core/pkg/cryptoutil" "github.com/oxynote/oxynote/server/core/pkg/errutil" "github.com/oxynote/oxynote/server/core/pkg/ioutil" "github.com/oxynote/oxynote/server/core/pkg/metricutil" @@ -34,12 +35,29 @@ const ( _pgUser string = "pgtest" _pgPass string = "pgpass" - // _dataSourceSecret is the credentials signing secret used by - // all temporary test databases. It must be a valid AES key - // length (32 bytes). - _dataSourceSecret string = "0123456789abcdef0123456789abcdef" + // _dataSourceKey is the base64 credentials encryption key every + // temporary test database is configured with. + _dataSourceKey = "MDEyMzQ1Njc4OWFiY2RlZjAxMjM0NTY3ODlhYmNkZWY=" + + // _retiredDataSourceKey is a base64 credentials encryption key no + // temporary test database is configured with unless a test says so. + _retiredDataSourceKey = "ZmVkY2JhOTg3NjU0MzIxMGZlZGNiYTk4NzY1NDMyMTA=" ) +// _dataSourceKeys is the keyring every temporary test database is +// configured with. +var _dataSourceKeys = mustParseKeys(_dataSourceKey) + +// mustParseKeys parses a keyring list or panics. +func mustParseKeys(list string) *cryptoutil.Keyring { + keys, err := cryptoutil.ParseKeyring(list) + if err != nil { + panic("cannot parse test keyring: " + err.Error()) + } + + return keys +} + func TestMain(m *testing.M) { container, err := gnomock.Start(pgDocker.Preset( pgDocker.WithUser( @@ -281,8 +299,8 @@ func prepTempDB(t *testing.T) *DB { name := uniuri.NewLenChars(10, []byte("abcdefghijklmnopqrstuvwxyz")) opts := Options{ - DSN: fmt.Sprintf("postgres://%s:%s@%s/%s?sslmode=disable", _pgUser, _pgPass, _pgDSN, name), - DataSourceCredentialsSigningSecret: _dataSourceSecret, + DSN: fmt.Sprintf("postgres://%s:%s@%s/%s?sslmode=disable", _pgUser, _pgPass, _pgDSN, name), + DataSourceCredentialsKeys: _dataSourceKeys, } tmpDB, err := sqlx.Connect( diff --git a/server/core/internal/server/internal/datasource/http_test.go b/server/core/internal/server/internal/datasource/http_test.go index 8e003e46..824e97b4 100644 --- a/server/core/internal/server/internal/datasource/http_test.go +++ b/server/core/internal/server/internal/datasource/http_test.go @@ -558,14 +558,14 @@ func Test_Handler_TestDataSourceConnection(t *testing.T) { "Credentials that cannot be decrypted are reported": { DB: func() *DBMock { ds := stubDataSource(datasourceCore.TypePrometheus) - ds.Status = processor.ConnectionStatusInvalidSigningSecret + ds.Status = processor.ConnectionStatusInvalidEncryptionKey return stubDB(ds, nil) }(), - Clients: &clientMocks{status: processor.ConnectionStatusInvalidSigningSecret, statusErr: nil}, + Clients: &clientMocks{status: processor.ConnectionStatusInvalidEncryptionKey, statusErr: nil}, ID: _testID.String(), Checks: checks( - hasResp(http.StatusOK, `{"status":"invalid_signing_secret"}`), + hasResp(http.StatusOK, `{"status":"invalid_encryption_key"}`), // the read already recorded the status, and a write here // would store an encryption of the credentials it could // not read in place of the ones it could not. diff --git a/server/core/pkg/cryptoutil/cryptoutil_test.go b/server/core/pkg/cryptoutil/cryptoutil_test.go index 105fd528..ea9995d2 100644 --- a/server/core/pkg/cryptoutil/cryptoutil_test.go +++ b/server/core/pkg/cryptoutil/cryptoutil_test.go @@ -6,8 +6,13 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.uber.org/goleak" ) +func TestMain(m *testing.M) { + goleak.VerifyTestMain(m) +} + func Test_EncryptText(t *testing.T) { // Invalid key length. encryptedText, err := EncryptText("text", []byte("key")) diff --git a/server/core/pkg/cryptoutil/keyring.go b/server/core/pkg/cryptoutil/keyring.go new file mode 100644 index 00000000..823898be --- /dev/null +++ b/server/core/pkg/cryptoutil/keyring.go @@ -0,0 +1,171 @@ +package cryptoutil + +import ( + "crypto/aes" + "crypto/cipher" + "crypto/rand" + "crypto/sha256" + "encoding/base64" + "errors" + "fmt" + "io" + "strings" +) + +const ( + // _keyringVersion is the version byte every Keyring ciphertext starts + // with. A later layout bumps it, so rows written by both can coexist. + _keyringVersion byte = 1 + + // _keyIDSize is the length of the key id a ciphertext carries. + _keyIDSize = 8 + + // _keyringHeaderSize is the length of the version byte and key id + // that precede the nonce. + _keyringHeaderSize = 1 + _keyIDSize +) + +var ( + // ErrEmptyKeyring is returned when the keyring list holds no key. + ErrEmptyKeyring = errors.New("keyring holds no keys") + + // ErrDuplicateKey is returned when the keyring lists a key twice. + ErrDuplicateKey = errors.New("keyring lists the same key twice") + + // ErrUnknownVersion is returned when a ciphertext starts with a + // version byte this keyring does not read. + ErrUnknownVersion = errors.New("ciphertext version is not supported") + + // ErrUnknownKey is returned when a ciphertext names a key the keyring + // does not hold. + ErrUnknownKey = errors.New("ciphertext was sealed with a key the keyring does not hold") + + // ErrCiphertextTooShort is returned when a ciphertext cannot even hold + // its header and nonce. + ErrCiphertextTooShort = errors.New("ciphertext too short") +) + +// keyringKey is one key of a Keyring, identified by the first bytes of its +// SHA-256 so operators never have to name keys. +type keyringKey struct { + id [_keyIDSize]byte + aead cipher.AEAD +} + +// Keyring holds the AES-256-GCM keys data is sealed and opened with. The +// first key seals; every key opens what it sealed. Rotation is prepending a +// key, re-encrypting what the old one sealed, and dropping the old one. +type Keyring struct { + keys []keyringKey + byID map[[_keyIDSize]byte]cipher.AEAD +} + +// ParseKeyring parses a comma-separated list of keys, newest first, each +// the standard base64 encoding of exactly KeySize random bytes. +func ParseKeyring(list string) (*Keyring, error) { + if strings.TrimSpace(list) == "" { + return nil, ErrEmptyKeyring + } + + kr := &Keyring{ + byID: make(map[[_keyIDSize]byte]cipher.AEAD), + } + + for i, entry := range strings.Split(list, ",") { + key, err := base64.StdEncoding.Strict().DecodeString(strings.TrimSpace(entry)) + if err != nil { + return nil, fmt.Errorf("key %d: %w", i+1, err) + } + + if len(key) != KeySize { + return nil, fmt.Errorf("key %d: %w", i+1, ErrInvalidKeySize) + } + + kk, err := newKeyringKey(key) + if err != nil { + // NOCOV: aes.NewCipher and cipher.NewGCM cannot fail with a + // key of KeySize bytes. + return nil, fmt.Errorf("key %d: %w", i+1, err) + } + + if _, ok := kr.byID[kk.id]; ok { + return nil, fmt.Errorf("key %d: %w", i+1, ErrDuplicateKey) + } + + kr.keys = append(kr.keys, kk) + kr.byID[kk.id] = kk.aead + } + + return kr, nil +} + +// Encrypt seals the plaintext under the newest key, bound to the associated +// data, which Decrypt must be handed again. The result is the version byte, +// the key id, the nonce and the sealed bytes. +func (k *Keyring) Encrypt(plaintext, aad []byte) ([]byte, error) { + kk := k.keys[0] + nonceSize := kk.aead.NonceSize() + + out := make([]byte, _keyringHeaderSize+nonceSize, _keyringHeaderSize+nonceSize+len(plaintext)+kk.aead.Overhead()) + out[0] = _keyringVersion + copy(out[1:], kk.id[:]) + + if _, err := io.ReadFull(rand.Reader, out[_keyringHeaderSize:]); err != nil { + // NOCOV: crypto/rand failures cannot be simulated in tests. + return nil, err + } + + return kk.aead.Seal(out, out[_keyringHeaderSize:], plaintext, aad), nil +} + +// Decrypt opens a ciphertext Encrypt produced under any key the keyring +// holds, given the same associated data. +func (k *Keyring) Decrypt(ciphertext, aad []byte) ([]byte, error) { + if len(ciphertext) < _keyringHeaderSize { + return nil, ErrCiphertextTooShort + } + + if ciphertext[0] != _keyringVersion { + return nil, ErrUnknownVersion + } + + aead, ok := k.byID[[_keyIDSize]byte(ciphertext[1:_keyringHeaderSize])] + if !ok { + return nil, ErrUnknownKey + } + + rest := ciphertext[_keyringHeaderSize:] + if len(rest) < aead.NonceSize() { + return nil, ErrCiphertextTooShort + } + + return aead.Open(nil, rest[:aead.NonceSize()], rest[aead.NonceSize():], aad) +} + +// IsCurrent reports whether the ciphertext was sealed under the newest key. +// Anything the keyring would not read at all is not current either. +func (k *Keyring) IsCurrent(ciphertext []byte) bool { + return len(ciphertext) >= _keyringHeaderSize && + ciphertext[0] == _keyringVersion && + [_keyIDSize]byte(ciphertext[1:_keyringHeaderSize]) == k.keys[0].id +} + +// newKeyringKey derives the key's id and prepares its cipher. +func newKeyringKey(key []byte) (keyringKey, error) { + block, err := aes.NewCipher(key) + if err != nil { + return keyringKey{}, err + } + + aead, err := cipher.NewGCM(block) + if err != nil { + return keyringKey{}, err + } + + sum := sha256.Sum256(key) + + return keyringKey{ + id: [_keyIDSize]byte(sum[:_keyIDSize]), + aead: aead, + }, nil +} diff --git a/server/core/pkg/cryptoutil/keyring_test.go b/server/core/pkg/cryptoutil/keyring_test.go new file mode 100644 index 00000000..614514dd --- /dev/null +++ b/server/core/pkg/cryptoutil/keyring_test.go @@ -0,0 +1,246 @@ +package cryptoutil + +import ( + "bytes" + "crypto/sha256" + "encoding/base64" + "strings" + "testing" + + "github.com/oxynote/oxynote/server/core/pkg/testutil" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +var ( + // _keyA is a base64 test key. + _keyA = base64.StdEncoding.EncodeToString(bytes.Repeat([]byte{'a'}, KeySize)) + + // _keyB is another base64 test key. + _keyB = base64.StdEncoding.EncodeToString(bytes.Repeat([]byte{'b'}, KeySize)) +) + +// testKeyring parses the given keys or fails the test. +func testKeyring(t *testing.T, keys ...string) *Keyring { + t.Helper() + + kr, err := ParseKeyring(joinKeys(keys...)) + require.NoError(t, err) + + return kr +} + +// joinKeys builds the list ParseKeyring reads. +func joinKeys(keys ...string) string { + return strings.Join(keys, ",") +} + +func Test_ParseKeyring(t *testing.T) { + cc := map[string]struct { + List string + Result [][KeySize]byte + Err error + }{ + "Empty list": { + List: "", + Err: ErrEmptyKeyring, + }, + "Whitespace only": { + List: " ", + Err: ErrEmptyKeyring, + }, + "Invalid base64": { + List: "not base64!", + Err: assert.AnError, + }, + "Key of 16 bytes": { + List: base64.StdEncoding.EncodeToString(bytes.Repeat([]byte{'a'}, 16)), + Err: assert.AnError, + }, + "Key of 33 bytes": { + List: base64.StdEncoding.EncodeToString(bytes.Repeat([]byte{'a'}, 33)), + Err: assert.AnError, + }, + "Duplicate key": { + List: joinKeys(_keyA, _keyB, _keyA), + Err: assert.AnError, + }, + "Trailing comma": { + List: _keyA + ",", + Err: assert.AnError, + }, + "One key": { + List: _keyA, + Result: [][KeySize]byte{[KeySize]byte(bytes.Repeat([]byte{'a'}, KeySize))}, + }, + "Two keys keep their order": { + List: " " + _keyB + " , " + _keyA, + Result: [][KeySize]byte{ + [KeySize]byte(bytes.Repeat([]byte{'b'}, KeySize)), + [KeySize]byte(bytes.Repeat([]byte{'a'}, KeySize)), + }, + }, + } + + for cn, c := range cc { + t.Run(cn, func(t *testing.T) { + t.Parallel() + + kr, err := ParseKeyring(c.List) + testutil.AssertEqualError(t, c.Err, err) + + if err != nil { + assert.Nil(t, kr) + + return + } + + require.NotNil(t, kr) + require.Len(t, kr.keys, len(c.Result)) + assert.Len(t, kr.byID, len(c.Result)) + + for i, key := range c.Result { + sum := sha256.Sum256(key[:]) + assert.Equal(t, [_keyIDSize]byte(sum[:_keyIDSize]), kr.keys[i].id) + assert.Equal(t, kr.keys[i].aead, kr.byID[kr.keys[i].id]) + } + }) + } +} + +func Test_Keyring_Encrypt(t *testing.T) { + kr := testKeyring(t, _keyA, _keyB) + + out, err := kr.Encrypt([]byte("text"), []byte("aad")) + require.NoError(t, err) + + sum := sha256.Sum256(bytes.Repeat([]byte{'a'}, KeySize)) + nonceSize := kr.keys[0].aead.NonceSize() + + assert.Equal(t, _keyringVersion, out[0]) + assert.Equal(t, sum[:_keyIDSize], out[1:_keyringHeaderSize]) + assert.Len(t, out, _keyringHeaderSize+nonceSize+len("text")+kr.keys[0].aead.Overhead()) + + // a second call draws a fresh nonce. + again, err := kr.Encrypt([]byte("text"), []byte("aad")) + require.NoError(t, err) + assert.NotEqual(t, out[_keyringHeaderSize:_keyringHeaderSize+nonceSize], again[_keyringHeaderSize:_keyringHeaderSize+nonceSize]) +} + +func Test_Keyring_Decrypt(t *testing.T) { + sealed := func(t *testing.T, kr *Keyring) []byte { + t.Helper() + + out, err := kr.Encrypt([]byte("text"), []byte("aad")) + require.NoError(t, err) + + return out + } + + cc := map[string]struct { + Keyring *Keyring + Ciphertext []byte + AAD []byte + Result []byte + Err error + }{ + "Too short for the header": { + Keyring: testKeyring(t, _keyA), + Ciphertext: []byte{_keyringVersion, 1, 2}, + AAD: []byte("aad"), + Err: ErrCiphertextTooShort, + }, + "Too short for the nonce": { + Keyring: testKeyring(t, _keyA), + Ciphertext: sealed(t, testKeyring(t, _keyA))[:_keyringHeaderSize+3], + AAD: []byte("aad"), + Err: ErrCiphertextTooShort, + }, + "Unknown version": { + Keyring: testKeyring(t, _keyA), + Ciphertext: func() []byte { + out := sealed(t, testKeyring(t, _keyA)) + out[0] = 2 + + return out + }(), + AAD: []byte("aad"), + Err: ErrUnknownVersion, + }, + "Unknown key": { + Keyring: testKeyring(t, _keyA), + Ciphertext: sealed(t, testKeyring(t, _keyB)), + AAD: []byte("aad"), + Err: ErrUnknownKey, + }, + "Associated data mismatch": { + Keyring: testKeyring(t, _keyA), + Ciphertext: sealed(t, testKeyring(t, _keyA)), + AAD: []byte("other"), + Err: assert.AnError, + }, + "Tampered ciphertext": { + Keyring: testKeyring(t, _keyA), + Ciphertext: func() []byte { + out := sealed(t, testKeyring(t, _keyA)) + out[len(out)-1] ^= 1 + + return out + }(), + AAD: []byte("aad"), + Err: assert.AnError, + }, + "Sealed under a retired key": { + Keyring: testKeyring(t, _keyB, _keyA), + Ciphertext: sealed(t, testKeyring(t, _keyA)), + AAD: []byte("aad"), + Result: []byte("text"), + }, + "Sealed under the newest key": { + Keyring: testKeyring(t, _keyA, _keyB), + Ciphertext: sealed(t, testKeyring(t, _keyA)), + AAD: []byte("aad"), + Result: []byte("text"), + }, + } + + for cn, c := range cc { + t.Run(cn, func(t *testing.T) { + t.Parallel() + + res, err := c.Keyring.Decrypt(c.Ciphertext, c.AAD) + testutil.AssertEqualError(t, c.Err, err) + + if err != nil { + assert.Nil(t, res) + + return + } + + assert.Equal(t, c.Result, res) + }) + } +} + +func Test_Keyring_IsCurrent(t *testing.T) { + kr := testKeyring(t, _keyA, _keyB) + + out, err := kr.Encrypt([]byte("text"), nil) + require.NoError(t, err) + + // too short + assert.False(t, kr.IsCurrent(out[:_keyringHeaderSize-1])) + + // unknown version + other := bytes.Clone(out) + other[0] = 2 + assert.False(t, kr.IsCurrent(other)) + + // retired key + retired, err := testKeyring(t, _keyB).Encrypt([]byte("text"), nil) + require.NoError(t, err) + assert.False(t, kr.IsCurrent(retired)) + + // newest key + assert.True(t, kr.IsCurrent(out)) +} diff --git a/web/app/utils/api/data-source/index.ts b/web/app/utils/api/data-source/index.ts index 847e0bb8..ec9527e1 100644 --- a/web/app/utils/api/data-source/index.ts +++ b/web/app/utils/api/data-source/index.ts @@ -26,7 +26,7 @@ export enum DataSourceStatus { Unreachable = "unreachable", VersionNotSupported = "version_not_supported", NotReadOnly = "not_read_only", - InvalidSigningSecret = "invalid_signing_secret", + InvalidEncryptionKey = "invalid_encryption_key", // local-only LocalOptimisticInsert = "local_optimistic_insert", // aka pending @@ -37,7 +37,7 @@ export const DataSourceErrorStatuses = [ DataSourceStatus.Unreachable, DataSourceStatus.VersionNotSupported, DataSourceStatus.NotReadOnly, - DataSourceStatus.InvalidSigningSecret, + DataSourceStatus.InvalidEncryptionKey, ] as const export interface DataSource { diff --git a/web/i18n/locales/en/settings.json b/web/i18n/locales/en/settings.json index 01d4970c..2260ac81 100644 --- a/web/i18n/locales/en/settings.json +++ b/web/i18n/locales/en/settings.json @@ -286,7 +286,7 @@ }, "description": "The data source user has write access, but Oxynote only needs read permissions. Please provide a read-only user to continue" }, - "invalid_signing_secret": { + "invalid_encryption_key": { "title": { "creation": "{name} couldn't be connected", "update": "{name} couldn't be updated"