enc1 said what a value was but not where it belonged. The blob in one user's nextcloud_token would decrypt perfectly if copied onto another user's row, and that account's exports would then go into someone else's storage. Nothing about the ciphertext objected. Adds enc2, which binds a context string — 'users:nextcloud_token:41' — as AES-GCM additional authenticated data. The auth tag covers it, so a moved ciphertext fails to open rather than opening somewhere wrong. The context is authenticated, not encrypted: it is not a secret, it is a claim about location. enc1 is still read, unchanged and forever — every existing row is enc1. Bound rows are distinguishable (isBound), so the Nextcloud token is rebound in place the next time it is successfully used, the same way a legacy plaintext one was already upgraded. That gate moved from isEncrypted to isBound; isEncrypted is true for enc1, so it would have skipped exactly the rows that need rebinding. Bound so far: Nextcloud app passwords, and invite codes — which bind to their own code_hash, since unlike the row id that exists at INSERT time. Decrypting an invite moved into list(), so the cipher and the hash that opens it no longer leave the module together. Mutation-tested: dropping the AAD, dropping the guard, gating on isEncrypted, or dropping a context at a call site each fail a test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Dv6sqaY6Vq3ChZHMem3cnU
107 lines
5.2 KiB
JavaScript
107 lines
5.2 KiB
JavaScript
// A ciphertext used to say what a value was but not where it belonged. Copy the
|
|
// enc1 blob in one user's nextcloud_token onto another user's row and it
|
|
// decrypted perfectly — that account's exports then land in someone else's
|
|
// storage. AES-GCM's additional authenticated data closes that: the auth tag
|
|
// covers a context string naming the row, so a moved ciphertext fails to open.
|
|
//
|
|
// These tests exercise the real primitive, not a description of it.
|
|
const test = require('node:test');
|
|
const assert = require('node:assert/strict');
|
|
const fs = require('node:fs');
|
|
const path = require('node:path');
|
|
|
|
// The module reads the key once at load, so it has to be set before requiring.
|
|
process.env.DATA_ENCRYPTION_KEY = require('node:crypto').randomBytes(32).toString('hex');
|
|
const cryptoUtil = require('../src/utils/crypto');
|
|
|
|
const AAD = cryptoUtil.context('users', 'nextcloud_token', 41);
|
|
|
|
test('a bound value reads back under its own context', () => {
|
|
const sealed = cryptoUtil.encryptString('app-password', AAD);
|
|
assert.equal(cryptoUtil.decryptString(sealed, AAD), 'app-password');
|
|
});
|
|
|
|
test('the same value moved to another row does not open', () => {
|
|
const sealed = cryptoUtil.encryptString('app-password', AAD);
|
|
const otherRow = cryptoUtil.context('users', 'nextcloud_token', 99);
|
|
assert.throws(() => cryptoUtil.decryptString(sealed, otherRow));
|
|
});
|
|
|
|
test('a bound value does not open with no context at all', () => {
|
|
// GCM would refuse this regardless — an absent AAD is a different AAD, so the
|
|
// tag fails. The explicit guard exists so the fault reads as "you forgot the
|
|
// context" rather than the generic unable-to-authenticate, which is what a
|
|
// call site that was never updated will actually hit. Assert the message, or
|
|
// losing the guard looks like a passing test.
|
|
const sealed = cryptoUtil.encryptString('app-password', AAD);
|
|
assert.throws(() => cryptoUtil.decryptString(sealed), /bound to a location/);
|
|
});
|
|
|
|
test('binding to a different column of the same row does not open', () => {
|
|
const sealed = cryptoUtil.encryptString('app-password', AAD);
|
|
assert.throws(() =>
|
|
cryptoUtil.decryptString(sealed, cryptoUtil.context('users', 'other_secret', 41)));
|
|
});
|
|
|
|
test('enc1 rows written before binding still read, with or without a context', () => {
|
|
// Every row in the live database is enc1. If this breaks, every Nextcloud
|
|
// connection, note and saved chat becomes unreadable.
|
|
const legacy = cryptoUtil.encryptString('written-last-year');
|
|
assert.equal(legacy.slice(0, 5), 'enc1:');
|
|
assert.equal(cryptoUtil.decryptString(legacy), 'written-last-year');
|
|
assert.equal(cryptoUtil.decryptString(legacy, AAD), 'written-last-year');
|
|
});
|
|
|
|
test('the two formats are distinguishable, which is how upgrade-on-read works', () => {
|
|
assert.equal(cryptoUtil.isBound(cryptoUtil.encryptString('x', AAD)), true);
|
|
assert.equal(cryptoUtil.isBound(cryptoUtil.encryptString('x')), false);
|
|
assert.equal(cryptoUtil.isBound('plaintext'), false);
|
|
// Both are still encrypted, so nothing that asks that question changes.
|
|
assert.equal(cryptoUtil.isEncrypted(cryptoUtil.encryptString('x', AAD)), true);
|
|
});
|
|
|
|
test('two rows produce two different contexts', () => {
|
|
// A constant context would bind nothing at all.
|
|
assert.notEqual(cryptoUtil.context('users', 'nextcloud_token', 1),
|
|
cryptoUtil.context('users', 'nextcloud_token', 2));
|
|
});
|
|
|
|
// ---- the call sites that matter -------------------------------------------
|
|
|
|
const read = (f) => fs.readFileSync(path.join(__dirname, '..', f), 'utf8');
|
|
|
|
test('both Nextcloud token readers and writers use one shared context', () => {
|
|
// A route defining its own copy is how the two drift into a mismatch that
|
|
// reads as a corrupt token.
|
|
const files = read('src/utils/nextcloudFiles.js');
|
|
const route = read('src/routes/nextcloud.js');
|
|
assert.match(files, /function tokenContext\(userId\)/);
|
|
assert.match(files, /module\.exports = \{[^}]*tokenContext/);
|
|
assert.match(route, /require\('\.\.\/utils\/nextcloudFiles'\)/);
|
|
assert.doesNotMatch(route, /function tokenContext/);
|
|
|
|
for (const [name, src] of [['nextcloudFiles.js', files], ['nextcloud.js', route]]) {
|
|
for (const call of src.match(/crypt\w*\.(en|de)cryptString\([^;]*nextcloud_token[^;]*\)|crypt\w*\.(en|de)cryptString\((?:appPassword|pw|place\.password)[^)]*\)/g) || []) {
|
|
assert.match(call, /tokenContext/, name + ' has an unbound token call: ' + call);
|
|
}
|
|
}
|
|
});
|
|
|
|
test('an older token is rebound the next time it is used', () => {
|
|
// isEncrypted() was the old test; it is true for enc1, so upgrade-on-read
|
|
// would never fire for the rows that most need it.
|
|
for (const f of ['src/utils/nextcloudFiles.js', 'src/routes/nextcloud.js']) {
|
|
const src = read(f);
|
|
assert.match(src, /if \(!cryptoUtil\.isBound\(/, f + ' still gates on isEncrypted');
|
|
}
|
|
});
|
|
|
|
test('an invite code is bound to its own row', () => {
|
|
const src = read('src/utils/registrationInvites.js');
|
|
assert.match(src, /function codeContext\(codeHash\)/);
|
|
assert.match(src, /encryptString\(normalize\(code\), codeContext\(hash\(code\)\)\)/);
|
|
// The cipher and the hash that opens it must not leave the module together.
|
|
assert.match(src, /delete row\.code_cipher/);
|
|
assert.match(src, /delete row\.code_hash/);
|
|
assert.doesNotMatch(read('src/routes/adminConfig.js'), /decryptCode/);
|
|
});
|