fix: enforce strict online backup keyring entries
This commit is contained in:
@@ -4,6 +4,8 @@ const crypto = require('node:crypto');
|
|||||||
const secretStore = require('./secret-store');
|
const secretStore = require('./secret-store');
|
||||||
const { parseOnlineBackupKey } = require('./online-backup');
|
const { parseOnlineBackupKey } = require('./online-backup');
|
||||||
|
|
||||||
|
const STORED_ENTRY_KEYS = ['createdAt', 'encryptedKey', 'id'];
|
||||||
|
|
||||||
function isObject(value) {
|
function isObject(value) {
|
||||||
return Boolean(value) && typeof value === 'object' && !Array.isArray(value);
|
return Boolean(value) && typeof value === 'object' && !Array.isArray(value);
|
||||||
}
|
}
|
||||||
@@ -24,8 +26,11 @@ function createOnlineBackupKeyring({
|
|||||||
let mutation = Promise.resolve();
|
let mutation = Promise.resolve();
|
||||||
|
|
||||||
function validateEntry(entry) {
|
function validateEntry(entry) {
|
||||||
|
if (!isObject(entry)) return null;
|
||||||
|
const keys = Object.keys(entry).sort();
|
||||||
if (
|
if (
|
||||||
!isObject(entry)
|
keys.length !== STORED_ENTRY_KEYS.length
|
||||||
|
|| !keys.every((key, index) => key === STORED_ENTRY_KEYS[index])
|
||||||
|| typeof entry.id !== 'string'
|
|| typeof entry.id !== 'string'
|
||||||
|| !entry.id
|
|| !entry.id
|
||||||
|| typeof entry.encryptedKey !== 'string'
|
|| typeof entry.encryptedKey !== 'string'
|
||||||
@@ -53,7 +58,7 @@ function createOnlineBackupKeyring({
|
|||||||
return { id: entry.id, encryptedKey: entry.encryptedKey, createdAt, key };
|
return { id: entry.id, encryptedKey: entry.encryptedKey, createdAt, key };
|
||||||
}
|
}
|
||||||
|
|
||||||
async function readEntries() {
|
async function readEntries(rejectInvalidEntries = false) {
|
||||||
let contents;
|
let contents;
|
||||||
try {
|
try {
|
||||||
contents = await fsImpl.readFile(filePath, 'utf8');
|
contents = await fsImpl.readFile(filePath, 'utf8');
|
||||||
@@ -70,7 +75,11 @@ function createOnlineBackupKeyring({
|
|||||||
if (!isObject(document) || document.version !== 1 || !Array.isArray(document.entries)) {
|
if (!isObject(document) || document.version !== 1 || !Array.isArray(document.entries)) {
|
||||||
throw new Error('Gespeicherter Online-Schlüsselbund ist ungültig');
|
throw new Error('Gespeicherter Online-Schlüsselbund ist ungültig');
|
||||||
}
|
}
|
||||||
return document.entries.map(validateEntry).filter(Boolean);
|
const entries = document.entries.map(validateEntry);
|
||||||
|
if (rejectInvalidEntries && entries.some((entry) => !entry)) {
|
||||||
|
throw new Error('Gespeicherter Online-Schlüsselbund ist ungültig');
|
||||||
|
}
|
||||||
|
return entries.filter(Boolean);
|
||||||
}
|
}
|
||||||
|
|
||||||
async function writeEntries(entries) {
|
async function writeEntries(entries) {
|
||||||
@@ -135,7 +144,7 @@ function createOnlineBackupKeyring({
|
|||||||
return serialize(async () => {
|
return serialize(async () => {
|
||||||
const validated = validateEntry(entry);
|
const validated = validateEntry(entry);
|
||||||
if (!validated) throw new Error('Online-Sicherungsschlüssel ist ungültig');
|
if (!validated) throw new Error('Online-Sicherungsschlüssel ist ungültig');
|
||||||
const entries = await readEntries();
|
const entries = await readEntries(true);
|
||||||
if (entries.some((current) => current.id === validated.id)) return;
|
if (entries.some((current) => current.id === validated.id)) return;
|
||||||
await writeEntries([...entries, validated]);
|
await writeEntries([...entries, validated]);
|
||||||
});
|
});
|
||||||
@@ -143,7 +152,7 @@ function createOnlineBackupKeyring({
|
|||||||
|
|
||||||
function remove(id) {
|
function remove(id) {
|
||||||
return serialize(async () => {
|
return serialize(async () => {
|
||||||
const entries = await readEntries();
|
const entries = await readEntries(true);
|
||||||
const remaining = entries.filter((entry) => entry.id !== id);
|
const remaining = entries.filter((entry) => entry.id !== id);
|
||||||
if (remaining.length === entries.length) return false;
|
if (remaining.length === entries.length) return false;
|
||||||
await writeEntries(remaining);
|
await writeEntries(remaining);
|
||||||
|
|||||||
@@ -130,6 +130,21 @@ describe('encrypted online backup keyring', () => {
|
|||||||
assert.equal(fs.readFileSync(filePath, 'utf8'), '{invalid-json');
|
assert.equal(fs.readFileSync(filePath, 'utf8'), '{invalid-json');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('rejects entries with additional properties without legitimizing the unsafe file during mutation', async () => {
|
||||||
|
const { filePath, keyring } = fixture();
|
||||||
|
const key = validKey();
|
||||||
|
const prepared = keyring.prepare(key, '2026-08-22T10:00:00.000Z');
|
||||||
|
const contents = JSON.stringify({
|
||||||
|
version: 1,
|
||||||
|
entries: [{ ...prepared, plaintextKey: key }]
|
||||||
|
});
|
||||||
|
fs.writeFileSync(filePath, contents);
|
||||||
|
|
||||||
|
await assert.rejects(keyring.commit(prepared));
|
||||||
|
assert.equal(fs.readFileSync(filePath, 'utf8'), contents);
|
||||||
|
assert.deepEqual(await keyring.list(), []);
|
||||||
|
});
|
||||||
|
|
||||||
it('keeps the previous valid file readable and removes the temporary file after rename fails', async () => {
|
it('keeps the previous valid file readable and removes the temporary file after rename fails', async () => {
|
||||||
const firstFixture = fixture();
|
const firstFixture = fixture();
|
||||||
const firstKey = validKey();
|
const firstKey = validKey();
|
||||||
|
|||||||
Reference in New Issue
Block a user