Skip to content

fix(security): remove storekey=insecure encryption bypass #1496

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
bbopen wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(security): remove storekey=insecure encryption bypass #1496

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion core/keybag/key-bag.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,29 @@ export class KeyBag implements KeyBagIf {
);
}

/**
* Ensures a store key is present in the URL, generating one if needed.
* Rejects attempts to use insecure (unencrypted) storage.
*
* @param url - The URI to ensure has a store key
* @param keyFactory - Factory function to generate a key name if none exists
* @returns Result containing the URI with store key, or error if insecure mode attempted
* @throws Error if storekey=insecure is attempted (removed for security)
*/
async ensureKeyFromUrl(url: URI, keyFactory: () => string): Promise<Result<URI>> {
// add storekey to url
const storeKey = url.getParam(PARAM.STORE_KEY);
if (storeKey === "insecure") {
return Result.Ok(url);
return Result.Err(
this.logger
.Error()
.Msg(
"storekey=insecure is no longer supported. " +
"Data must be encrypted. Remove the storekey=insecure parameter " +
"to use automatic key generation, or provide a valid encryption key.",
)
.AsError(),
);
}
if (!storeKey) {
const keyName = `@${keyFactory()}@`;
Expand Down
120 changes: 35 additions & 85 deletions core/runtime/keyed-crypto.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,96 +153,46 @@ class cryptoAction implements CryptoAction {
}
}

class nullCodec implements AsyncBlockCodec<24, Uint8Array, IvKeyIdData> {
readonly code = 24;
readonly name = "Fireproof@unencrypted-block";
readonly empty = new Uint8Array();

async encode(data: Uint8Array): Promise<Uint8Array> {
return data;
}
async decode(data: Uint8Array): Promise<IvKeyIdData> {
return {
iv: this.empty,
keyId: this.empty,
data: data,
};
}
}

class noCrypto implements CryptoAction {
readonly ivLength = 0;
readonly code = 0x0;
readonly name = "Fireproof@unencrypted-block";
readonly logger: Logger;
readonly crypto: CryptoRuntime;
readonly key: KeysByFingerprint;
readonly isEncrypting = false;
readonly _fingerPrint = "noCrypto:" + Math.random();
readonly url: URI;
constructor(url: URI, cyrt: CryptoRuntime, sthis: SuperThis) {
this.logger = ensureLogger(sthis, "noCrypto");
this.crypto = cyrt;
this.key = {
id: sthis.nextId().str,
name: "noCrypto",
get: () => {
throw this.logger.Error().Msg("noCrypto.get not implemented").AsError();
},
upsert: () => {
throw this.logger.Error().Msg("noCrypto.upsert not implemented").AsError();
},
asV2StorageKeyItem: () => {
throw this.logger.Error().Msg("noCrypto.asV2KeysItem not implemented").AsError();
},
};
this.url = url;
}

fingerPrint(): Promise<string> {
return Promise.resolve(this._fingerPrint);
}
// eslint-disable-next-line @typescript-eslint/no-unused-vars
codec(iv?: Uint8Array): AsyncBlockCodec<24, Uint8Array, IvKeyIdData> {
return new nullCodec();
}
// eslint-disable-next-line @typescript-eslint/no-unused-vars
algo(iv?: Uint8Array): { name: string; iv: Uint8Array; tagLength: number } {
return {
name: "noCrypto",
iv: new Uint8Array(),
tagLength: 0,
};
}
_decrypt(): Promise<Uint8Array> {
throw this.logger.Error().Msg("noCrypto.decrypt not implemented").AsError();
}
_encrypt(): Promise<Uint8Array> {
throw this.logger.Error().Msg("noCrypto.decrypt not implemented").AsError();
}
}

/**
* Factory function to create a CryptoAction for encrypting/decrypting data.
* Encryption is mandatory - insecure mode is no longer supported.
*
* @param url - URI containing store configuration including store key
* @param kb - KeyBag interface for key management
* @param sthis - SuperThis context for logging and runtime access
* @returns CryptoAction instance configured for encryption
* @throws Error if storekey=insecure is attempted or if key retrieval fails
*/
export async function keyedCryptoFactory(url: URI, kb: KeyBagIf, sthis: SuperThis): Promise<CryptoAction> {
const storekey = url.getParam(PARAM.STORE_KEY);
if (storekey && storekey !== "insecure") {
if (storekey === "insecure") {
throw sthis.logger
.Error()
.Str("url", url.toString())
.Msg(
"storekey=insecure is no longer supported. " +
"Data must be encrypted. Remove the storekey=insecure parameter " +
"to use automatic key generation, or provide a valid encryption key.",
)
.AsError();
}
if (storekey) {
const rkey = await kb.getNamedKey(storekey, false);
if (rkey.isErr()) {
// try {
// rkey = await kb.toKeyWithFingerPrint(storekey);
// } catch (e) {
throw (
sthis.logger
.Error()
// .Err(e)
.Str("keybag", kb.rt.id())
// .Result("key", rkey)
.Str("name", storekey)
.Msg("getNamedKey failed")
.AsError()
);
// }
throw sthis.logger
.Error()
.Str("keybag", kb.rt.id())
.Str("name", storekey)
.Msg("getNamedKey failed")
.AsError();
}
return new cryptoAction(url, rkey.Ok(), kb.rt.crypto, sthis);
}
return new noCrypto(url, kb.rt.crypto, sthis);
// No storekey specified - this should not happen in normal operation
// as ensureKeyFromUrl should always add one, but handle gracefully
throw sthis.logger
.Error()
.Str("url", url.toString())
.Msg("No store key specified. Use ensureKeyFromUrl to add encryption key to URL.")
.AsError();
}
18 changes: 10 additions & 8 deletions core/tests/blockstore/keyed-crypto-indexeddb-file.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,14 +121,16 @@ describe("KeyedCryptoStore", () => {
baseUrl = baseUrl.build().defParam(PARAM.NAME, "test").URI();
loader = mockLoader(sthis);
});
it("no crypto", async () => {
/**
* Tests that creating stores with storekey=insecure is rejected.
* The insecure mode has been removed for security (see spec-03).
*/
it("rejects insecure storekey", async () => {
const url = baseUrl.build().setParam(PARAM.STORE_KEY, "insecure").URI();
for (const pstore of (await createAttachedStores(url, loader, "insecure")).stores.baseStores) {
const store = await pstore;
// await store.start();
const kc = await store.keyedCrypto();
expect(kc.constructor.name).toBe("noCrypto");
// expect(kc.isEncrypting).toBe(false);
}

// Attempting to create stores with insecure storekey should throw
await expect(createAttachedStores(url, loader, "test-indexeddb-file")).rejects.toThrow(
/storekey=insecure is no longer supported/,
);
});
});
21 changes: 8 additions & 13 deletions core/tests/blockstore/keyed-crypto.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,22 +256,17 @@ describe("KeyedCryptoStore", () => {
kb = await getKeyBag(sthis, {});
loader = mockLoader(sthis);
});
it("no crypto", async () => {
it("rejects insecure storekey", async () => {
const url = baseUrl.build().setParam(PARAM.STORE_KEY, "insecure").URI();

for (const pstore of (await createAttachedStores(url, loader, "insecure")).stores.baseStores) {
const store = await pstore;
// await store.start();
const kc = await store.keyedCrypto();
expect(kc.constructor.name).toBe("noCrypto");
// expect(kc.isEncrypting).toBe(false);
expect(kc.constructor.name).toBe("noCrypto");
// expect(kc.isEncrypting).toBe(false);
}
// Attempting to create stores with insecure storekey should throw
await expect(createAttachedStores(url, loader, "insecure")).rejects.toThrow(
/storekey=insecure is no longer supported/,
);
});

it("create key", async () => {
for (const pstore of (await createAttachedStores(baseUrl, loader, "insecure")).stores.baseStores) {
for (const pstore of (await createAttachedStores(baseUrl, loader, "test-create-key")).stores.baseStores) {
const store = await pstore; // await bs.ensureStart(await pstore, logger);
const kc = await store.keyedCrypto();
expect(kc.constructor.name).toBe("cryptoAction");
Expand All @@ -286,7 +281,7 @@ describe("KeyedCryptoStore", () => {
const key = base58btc.encode(kb.rt.crypto.randomBytes(kb.rt.keyLength));
const genKey = await kb.getNamedKey("@heute@", false, key);
const url = baseUrl.build().setParam(PARAM.STORE_KEY, "@heute@").URI();
for (const pstore of (await createAttachedStores(url, loader, "insecure")).stores.baseStores) {
for (const pstore of (await createAttachedStores(url, loader, "test-key-ref")).stores.baseStores) {
const store = await pstore;
// await store.start();
expect(store.url().getParam(PARAM.STORE_KEY)).toBe(`@heute@`);
Expand All @@ -307,7 +302,7 @@ describe("KeyedCryptoStore", () => {
it("key", async () => {
const key = base58btc.encode(kb.rt.crypto.randomBytes(kb.rt.keyLength));
const url = baseUrl.build().setParam(PARAM.STORE_KEY, key).URI();
for (const pstore of (await createAttachedStores(url, loader, "insecure")).stores.baseStores) {
for (const pstore of (await createAttachedStores(url, loader, "test-direct-key")).stores.baseStores) {
// for (const pstore of [strt.makeDataStore(loader), strt.makeMetaStore(loader), strt.makeWALStore(loader)]) {
const store = await pstore;
// await store.start();
Expand Down
9 changes: 4 additions & 5 deletions core/tests/blockstore/standalone.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,6 @@ describe("standalone", () => {
default:
uri = BuildURI.from("file://dist/standalone")
.setParam(PARAM.NAME, "peer-log")
.setParam(PARAM.STORE_KEY, "insecure")
.URI();
break;
}
Expand All @@ -115,10 +114,10 @@ describe("standalone", () => {
writeQueue: { chunkSize: 32 },
storeUrls: {
data: {
meta: uri.build().setParam(PARAM.STORE, "meta").setParam(PARAM.STORE_KEY, "insecure").URI(),
car: uri.build().setParam(PARAM.STORE, "car").setParam(PARAM.STORE_KEY, "insecure").URI(),
file: uri.build().setParam(PARAM.STORE, "file").setParam(PARAM.STORE_KEY, "insecure").URI(),
wal: uri.build().setParam(PARAM.STORE, "wal").setParam(PARAM.STORE_KEY, "insecure").URI(),
meta: uri.build().setParam(PARAM.STORE, "meta").URI(),
car: uri.build().setParam(PARAM.STORE, "car").URI(),
file: uri.build().setParam(PARAM.STORE, "file").URI(),
wal: uri.build().setParam(PARAM.STORE, "wal").URI(),
},
},
} as LedgerOpts);
Expand Down
Loading