insecure default ivLength for AES

[BubbLess]

Hi, I found that the ivLength is set to 13 in the data_encryption service, then it will be padded with zero bytes to 16.

trilium/src/services/data_encryption.js

Lines 20 to 42 in 80887fd

	function pad(data) {
	if (data.length > 16) {
	data = data.slice(0, 16);
	}
	else if (data.length < 16) {
	const zeros = Array(16 - data.length).fill(0);
	data = Buffer.concat([data, Buffer.from(zeros)]);
	}
	return Buffer.from(data);
	}
	function encrypt(key, plainText, ivLength = 13) {
	if (!key) {
	throw new Error("No data key!");
	}
	const plainTextBuffer = Buffer.from(plainText);
	const iv = crypto.randomBytes(ivLength);
	const cipher = crypto.createCipheriv('aes-128-cbc', pad(key), pad(iv));

This causes the last 3 bytes of IV to be fixed, and will leak the information about the last 3 bytes of the first cipher block. I really don't understand the reason of setting the ivLength to 13, I think just set it to 16 will be better.
And I noticed that while encrypting password, the ivLength is correctly set to 16.

trilium/src/services/password_encryption.js

Line 21 in 37eb16b

const newEncryptedDataKey = dataEncryptionService.encrypt(passwordDerivedKey, plainTextDataKey, 16);

[zadam]

Hi, good question. IIRC there was a bug in a very old version of Trilium which stored only the first 13 bytes of IV. That was soon discovered, however it's not possibly to just decrypt and reencrypt existing protected notes during database migration (since it's encrypted using user's password which has to be entered by the user). So I just made 13-byte IV standard.

You mention that this is insecure, but AFAIK it's not. The main requirement for IV is that it is unique. 13 bytes provides 104 bits of entropy. The risk of reusing an IV is essentially a birthday problem. Trilium is designed to handle 100 000 notes, let's calculate with a million. A birthday problem of 2^104 "days" and 1 million "children" gives probability of a collision 1/10^20, which for me still provides very good security.

[BubbLess]

OK, this may not be a big problem, I'm just curious about the reason for using 13 bytes IV. I think add a version to the encrypt data will make it more flexible for migrating, like turtl do in their design.

[zadam]

These encrypted records don't have format versioning, but it's easy to distinguish them - records with old IVs have always size % 16 = 13, new records with 16 byte IVs will have size % 16 = 0, so it's easy to distinguish.