mirror of
https://github.com/openpgpjs/openpgpjs.git
synced 2025-03-30 15:08:32 +00:00
Remove armor checksum check
The crypto refresh says that we MUST NOT reject messages where the CRC24 checksum is incorrect. So, we remove the check for it. Also, remove the checksumRequired config.
This commit is contained in:
Daniel Huigens
larabr
parent
6f1eb06119
commit
939622e827
1
openpgp.d.ts
vendored
1
openpgp.d.ts
vendored
@ -317,7 +317,6 @@ interface Config {
|
||||
aeadProtect: boolean;
|
||||
allowUnauthenticatedMessages: boolean;
|
||||
allowUnauthenticatedStream: boolean;
|
||||
checksumRequired: boolean;
|
||||
minRSABits: number;
|
||||
passwordCollisionCheck: boolean;
|
||||
revocationsExpire: boolean;
|
||||
|
||||
@ -130,11 +130,6 @@ export default {
|
||||
* @property {Boolean} allowUnauthenticatedStream
|
||||
*/
|
||||
allowUnauthenticatedStream: false,
|
||||
/**
|
||||
* @memberof module:config
|
||||
* @property {Boolean} checksumRequired Do not throw error when armor is missing a checksum
|
||||
*/
|
||||
checksumRequired: false,
|
||||
/**
|
||||
* Minimum RSA key size allowed for key generation and message signing, verification and encryption.
|
||||
* The default is 2047 since due to a bug, previous versions of OpenPGP.js could generate 2047-bit keys instead of 2048-bit ones.
|
||||
|
||||
@ -108,79 +108,6 @@ function addheader(customComment, config) {
|
||||
}
|
||||
|
||||
|
||||
/**
|
||||
* Calculates a checksum over the given data and returns it base64 encoded
|
||||
* @param {String | ReadableStream<String>} data - Data to create a CRC-24 checksum for
|
||||
* @returns {String | ReadableStream<String>} Base64 encoded checksum.
|
||||
* @private
|
||||
*/
|
||||
function getCheckSum(data) {
|
||||
const crc = createcrc24(data);
|
||||
return base64.encode(crc);
|
||||
}
|
||||
|
||||
// https://create.stephan-brumme.com/crc32/#slicing-by-8-overview
|
||||
|
||||
const crc_table = [
|
||||
new Array(0xFF),
|
||||
new Array(0xFF),
|
||||
new Array(0xFF),
|
||||
new Array(0xFF)
|
||||
];
|
||||
|
||||
for (let i = 0; i <= 0xFF; i++) {
|
||||
let crc = i << 16;
|
||||
for (let j = 0; j < 8; j++) {
|
||||
crc = (crc << 1) ^ ((crc & 0x800000) !== 0 ? 0x864CFB : 0);
|
||||
}
|
||||
crc_table[0][i] =
|
||||
((crc & 0xFF0000) >> 16) |
|
||||
(crc & 0x00FF00) |
|
||||
((crc & 0x0000FF) << 16);
|
||||
}
|
||||
for (let i = 0; i <= 0xFF; i++) {
|
||||
crc_table[1][i] = (crc_table[0][i] >> 8) ^ crc_table[0][crc_table[0][i] & 0xFF];
|
||||
}
|
||||
for (let i = 0; i <= 0xFF; i++) {
|
||||
crc_table[2][i] = (crc_table[1][i] >> 8) ^ crc_table[0][crc_table[1][i] & 0xFF];
|
||||
}
|
||||
for (let i = 0; i <= 0xFF; i++) {
|
||||
crc_table[3][i] = (crc_table[2][i] >> 8) ^ crc_table[0][crc_table[2][i] & 0xFF];
|
||||
}
|
||||
|
||||
// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/DataView#Endianness
|
||||
const isLittleEndian = (function() {
|
||||
const buffer = new ArrayBuffer(2);
|
||||
new DataView(buffer).setInt16(0, 0xFF, true /* littleEndian */);
|
||||
// Int16Array uses the platform's endianness.
|
||||
return new Int16Array(buffer)[0] === 0xFF;
|
||||
}());
|
||||
|
||||
/**
|
||||
* Internal function to calculate a CRC-24 checksum over a given string (data)
|
||||
* @param {String | ReadableStream<String>} input - Data to create a CRC-24 checksum for
|
||||
* @returns {Uint8Array | ReadableStream<Uint8Array>} The CRC-24 checksum.
|
||||
* @private
|
||||
*/
|
||||
function createcrc24(input) {
|
||||
let crc = 0xCE04B7;
|
||||
return stream.transform(input, value => {
|
||||
const len32 = isLittleEndian ? Math.floor(value.length / 4) : 0;
|
||||
const arr32 = new Uint32Array(value.buffer, value.byteOffset, len32);
|
||||
for (let i = 0; i < len32; i++) {
|
||||
crc ^= arr32[i];
|
||||
crc =
|
||||
crc_table[0][(crc >> 24) & 0xFF] ^
|
||||
crc_table[1][(crc >> 16) & 0xFF] ^
|
||||
crc_table[2][(crc >> 8) & 0xFF] ^
|
||||
crc_table[3][(crc >> 0) & 0xFF];
|
||||
}
|
||||
for (let i = len32 * 4; i < value.length; i++) {
|
||||
crc = (crc >> 8) ^ crc_table[0][(crc & 0xFF) ^ value[i]];
|
||||
}
|
||||
}, () => new Uint8Array([crc, crc >> 8, crc >> 16]));
|
||||
}
|
||||
|
||||
/**
|
||||
* Verify armored headers. crypto-refresh-06, section 6.2:
|
||||
* "An OpenPGP implementation may consider improperly formatted Armor
|
||||
@ -321,24 +248,6 @@ export function unarmor(input, config = defaultConfig) {
|
||||
await writer.abort(e);
|
||||
}
|
||||
}));
|
||||
data = stream.transformPair(data, async (readable, writable) => {
|
||||
const checksumVerified = stream.readToEnd(getCheckSum(stream.passiveClone(readable)));
|
||||
checksumVerified.catch(() => {});
|
||||
await stream.pipe(readable, writable, {
|
||||
preventClose: true
|
||||
});
|
||||
const writer = stream.getWriter(writable);
|
||||
try {
|
||||
const checksumVerifiedString = (await checksumVerified).replace('\n', '');
|
||||
if (checksum !== checksumVerifiedString && (checksum || config.checksumRequired)) {
|
||||
throw new Error('Ascii armor integrity check failed');
|
||||
}
|
||||
await writer.ready;
|
||||
await writer.close();
|
||||
} catch (e) {
|
||||
await writer.abort(e);
|
||||
}
|
||||
});
|
||||
} catch (e) {
|
||||
reject(e);
|
||||
}
|
||||
|
||||
@ -171,15 +171,7 @@ export default () => describe('ASCII armor', function() {
|
||||
'-----END PGP PRIVATE KEY BLOCK-----'
|
||||
].join('\n');
|
||||
|
||||
// try with default config
|
||||
await expect(openpgp.readKey({ armoredKey: privKey })).to.be.rejectedWith(/Ascii armor integrity check failed/);
|
||||
|
||||
// try opposite config
|
||||
openpgp.config.checksumRequired = !openpgp.config.checksumRequired;
|
||||
await expect(openpgp.readKey({ armoredKey: privKey })).to.be.rejectedWith(/Ascii armor integrity check failed/);
|
||||
|
||||
// back to default
|
||||
openpgp.config.checksumRequired = !openpgp.config.checksumRequired;
|
||||
await openpgp.readKey({ armoredKey: privKey });
|
||||
});
|
||||
|
||||
it('Armor checksum validation - valid', async function () {
|
||||
@ -203,15 +195,7 @@ export default () => describe('ASCII armor', function() {
|
||||
'=wJNM',
|
||||
'-----END PGP PRIVATE KEY BLOCK-----'].join('\n');
|
||||
|
||||
// try with default config
|
||||
await openpgp.readKey({ armoredKey: privKey });
|
||||
|
||||
// try opposite config
|
||||
openpgp.config.checksumRequired = !openpgp.config.checksumRequired;
|
||||
await openpgp.readKey({ armoredKey: privKey });
|
||||
|
||||
// back to default
|
||||
openpgp.config.checksumRequired = !openpgp.config.checksumRequired;
|
||||
});
|
||||
|
||||
it('Armor checksum validation - missing', async function () {
|
||||
@ -234,23 +218,7 @@ export default () => describe('ASCII armor', function() {
|
||||
'71vlXMJNXvoCeuejiRw=',
|
||||
'-----END PGP PRIVATE KEY BLOCK-----'].join('\n');
|
||||
|
||||
// try with default config
|
||||
if (openpgp.config.checksumRequired) {
|
||||
await expect(openpgp.readKey({ armoredKey: privKeyNoCheckSum })).to.be.rejectedWith(/Ascii armor integrity check failed/);
|
||||
} else {
|
||||
await openpgp.readKey({ armoredKey: privKeyNoCheckSum });
|
||||
}
|
||||
|
||||
// try opposite config
|
||||
openpgp.config.checksumRequired = !openpgp.config.checksumRequired;
|
||||
if (openpgp.config.checksumRequired) {
|
||||
await expect(openpgp.readKey({ armoredKey: privKeyNoCheckSum })).to.be.rejectedWith(/Ascii armor integrity check failed/);
|
||||
} else {
|
||||
await openpgp.readKey({ armoredKey: privKeyNoCheckSum });
|
||||
}
|
||||
|
||||
// back to default
|
||||
openpgp.config.checksumRequired = !openpgp.config.checksumRequired;
|
||||
await openpgp.readKey({ armoredKey: privKeyNoCheckSum });
|
||||
});
|
||||
|
||||
it('Armor checksum validation - missing - trailing newline', async function () {
|
||||
@ -274,23 +242,7 @@ export default () => describe('ASCII armor', function() {
|
||||
'-----END PGP PRIVATE KEY BLOCK-----',
|
||||
''].join('\n');
|
||||
|
||||
// try with default config
|
||||
if (openpgp.config.checksumRequired) {
|
||||
await expect(openpgp.readKey({ armoredKey: privKeyNoCheckSumWithTrailingNewline })).to.be.rejectedWith(/Ascii armor integrity check failed/);
|
||||
} else {
|
||||
await openpgp.readKey({ armoredKey: privKeyNoCheckSumWithTrailingNewline });
|
||||
}
|
||||
|
||||
// try opposite config
|
||||
openpgp.config.checksumRequired = !openpgp.config.checksumRequired;
|
||||
if (openpgp.config.checksumRequired) {
|
||||
await expect(openpgp.readKey({ armoredKey: privKeyNoCheckSumWithTrailingNewline })).to.be.rejectedWith(/Ascii armor integrity check failed/);
|
||||
} else {
|
||||
await openpgp.readKey({ armoredKey: privKeyNoCheckSumWithTrailingNewline });
|
||||
}
|
||||
|
||||
// back to default
|
||||
openpgp.config.checksumRequired = !openpgp.config.checksumRequired;
|
||||
await openpgp.readKey({ armoredKey: privKeyNoCheckSumWithTrailingNewline });
|
||||
});
|
||||
|
||||
it('Accept header with trailing whitespace', async function () {
|
||||
|
||||
@ -3057,60 +3057,51 @@ XfA3pqV4mTzF
|
||||
expect(await isAEADSupported([key])).to.equal(openpgp.config.aeadProtect);
|
||||
|
||||
const data = await openpgp.encrypt({ message: await openpgp.createMessage({ binary: new Uint8Array(500) }), encryptionKeys: [key.toPublic()] });
|
||||
let badSumEncrypted = data.replace(/\n=[a-zA-Z0-9/+]{4}/, '\n=aaaa');
|
||||
if (badSumEncrypted === data) { // checksum was already =aaaa
|
||||
badSumEncrypted = data.replace(/\n=[a-zA-Z0-9/+]{4}/, '\n=bbbb');
|
||||
}
|
||||
if (badSumEncrypted === data) {
|
||||
throw new Error('Was not able to successfully modify checksum');
|
||||
}
|
||||
const badBodyEncrypted = data.replace(/\n=([a-zA-Z0-9/+]{4})/, 'aaa\n=$1');
|
||||
const encrypted = data.substr(0, 500) + (data[500] === 'a' ? 'b' : 'a') + data.substr(501);
|
||||
await loadStreamsPolyfill();
|
||||
try {
|
||||
for (const allowStreaming of [true, false]) {
|
||||
openpgp.config.allowUnauthenticatedStream = allowStreaming;
|
||||
await Promise.all([badSumEncrypted, badBodyEncrypted].map(async (encrypted, i) => {
|
||||
await Promise.all([
|
||||
encrypted,
|
||||
new ReadableStream({
|
||||
start(controller) {
|
||||
controller.enqueue(encrypted);
|
||||
for (const [i, encryptedData] of [
|
||||
encrypted,
|
||||
new ReadableStream({
|
||||
start(controller) {
|
||||
controller.enqueue(encrypted);
|
||||
controller.close();
|
||||
}
|
||||
}),
|
||||
new ReadableStream({
|
||||
start() {
|
||||
this.remaining = encrypted.split('\n');
|
||||
},
|
||||
async pull(controller) {
|
||||
if (this.remaining.length) {
|
||||
await new Promise(res => setTimeout(res));
|
||||
controller.enqueue(this.remaining.shift() + '\n');
|
||||
} else {
|
||||
controller.close();
|
||||
}
|
||||
}),
|
||||
new ReadableStream({
|
||||
start() {
|
||||
this.remaining = encrypted.split('\n');
|
||||
},
|
||||
async pull(controller) {
|
||||
if (this.remaining.length) {
|
||||
await new Promise(res => { setTimeout(res); });
|
||||
controller.enqueue(this.remaining.shift() + '\n');
|
||||
} else {
|
||||
controller.close();
|
||||
}
|
||||
}
|
||||
})
|
||||
].map(async (encrypted, j) => {
|
||||
let stepReached = 0;
|
||||
try {
|
||||
const message = await openpgp.readMessage({ armoredMessage: encrypted });
|
||||
stepReached = 1;
|
||||
const { data: decrypted } = await openpgp.decrypt({ message: message, decryptionKeys: [key] });
|
||||
stepReached = 2;
|
||||
await stream.readToEnd(decrypted);
|
||||
} catch (e) {
|
||||
expect(e.message).to.match(/Ascii armor integrity check failed/);
|
||||
expect(stepReached).to.equal(
|
||||
j === 0 ? 0 :
|
||||
(openpgp.config.aeadChunkSizeByte === 0 && (j === 2 || detectNode() || util.getHardwareConcurrency() < 8)) || (!openpgp.config.aeadProtect && openpgp.config.allowUnauthenticatedStream) ? 2 :
|
||||
1
|
||||
);
|
||||
return;
|
||||
}
|
||||
throw new Error(`Expected "Ascii armor integrity check failed" error in subtest ${i}.${j}`);
|
||||
}));
|
||||
}));
|
||||
})
|
||||
].entries()) {
|
||||
let stepReached = 0;
|
||||
try {
|
||||
const message = await openpgp.readMessage({ armoredMessage: encryptedData });
|
||||
stepReached = 1;
|
||||
const { data: decrypted } = await openpgp.decrypt({ message: message, decryptionKeys: [key] });
|
||||
stepReached = 2;
|
||||
await stream.readToEnd(decrypted);
|
||||
} catch (e) {
|
||||
expect(e.message).to.match(/Modification detected|Authentication tag mismatch|Unsupported state or unable to authenticate data/);
|
||||
expect(stepReached).to.equal(
|
||||
i === 0 ? 1 :
|
||||
(openpgp.config.aeadChunkSizeByte === 0 && (i === 2 || detectNode() || util.getHardwareConcurrency() < 8)) || (!openpgp.config.aeadProtect && openpgp.config.allowUnauthenticatedStream) ? 2 :
|
||||
1
|
||||
);
|
||||
continue;
|
||||
}
|
||||
throw new Error(`Expected "Modification detected" error in subtest ${i}`);
|
||||
}
|
||||
}
|
||||
} finally {
|
||||
openpgp.config.allowUnauthenticatedStream = allowUnauthenticatedStream;
|
||||
|
||||
@ -403,7 +403,7 @@ function tests() {
|
||||
}
|
||||
});
|
||||
|
||||
it('Detect MDC modifications (allowUnauthenticatedStream=true)', async function() {
|
||||
it('Detect modification (allowUnauthenticatedStream=true)', async function() {
|
||||
const aeadProtectValue = openpgp.config.aeadProtect;
|
||||
openpgp.config.aeadProtect = false;
|
||||
const allowUnauthenticatedStreamValue = openpgp.config.allowUnauthenticatedStream;
|
||||
@ -418,7 +418,6 @@ function tests() {
|
||||
const message = await openpgp.readMessage({
|
||||
armoredMessage: stream[expectedType === 'node' ? 'webToNode' : 'toStream'](stream.transform(encrypted, value => {
|
||||
value += '';
|
||||
if (value === '=' || value.length === 5) return; // Remove checksum
|
||||
const newlineIndex = value.indexOf('\n', 500);
|
||||
if (value.length > 1000) return value.slice(0, newlineIndex - 1) + (value[newlineIndex - 1] === 'a' ? 'b' : 'a') + value.slice(newlineIndex);
|
||||
return value;
|
||||
@ -441,44 +440,7 @@ function tests() {
|
||||
}
|
||||
});
|
||||
|
||||
it('Detect armor checksum error (allowUnauthenticatedStream=true)', async function() {
|
||||
const allowUnauthenticatedStreamValue = openpgp.config.allowUnauthenticatedStream;
|
||||
openpgp.config.allowUnauthenticatedStream = true;
|
||||
try {
|
||||
const encrypted = await openpgp.encrypt({
|
||||
message: await openpgp.createMessage({ binary: data }),
|
||||
encryptionKeys: pubKey,
|
||||
signingKeys: privKey,
|
||||
config: { minRSABits: 1024 }
|
||||
});
|
||||
expect(stream.isStream(encrypted)).to.equal(expectedType);
|
||||
|
||||
const message = await openpgp.readMessage({
|
||||
armoredMessage: stream[expectedType === 'node' ? 'webToNode' : 'toStream'](stream.transform(encrypted, value => {
|
||||
value += '';
|
||||
const newlineIndex = value.indexOf('\n', 500);
|
||||
if (value.length > 1000) return value.slice(0, newlineIndex - 1) + (value[newlineIndex - 1] === 'a' ? 'b' : 'a') + value.slice(newlineIndex);
|
||||
return value;
|
||||
}), { encoding: 'utf8' })
|
||||
});
|
||||
const decrypted = await openpgp.decrypt({
|
||||
verificationKeys: pubKey,
|
||||
decryptionKeys: privKey,
|
||||
message,
|
||||
format: 'binary'
|
||||
});
|
||||
expect(stream.isStream(decrypted.data)).to.equal(expectedType);
|
||||
const reader = stream.getReader(decrypted.data);
|
||||
expect(await reader.peekBytes(1024)).not.to.deep.equal(plaintext[0]);
|
||||
dataArrived();
|
||||
await expect(reader.readToEnd()).to.be.rejectedWith('Ascii armor integrity check failed');
|
||||
expect(decrypted.signatures).to.exist.and.have.length(1);
|
||||
} finally {
|
||||
openpgp.config.allowUnauthenticatedStream = allowUnauthenticatedStreamValue;
|
||||
}
|
||||
});
|
||||
|
||||
it('Detect armor checksum error when not passing public keys (allowUnauthenticatedStream=true)', async function() {
|
||||
it('Detect modification when not passing public keys (allowUnauthenticatedStream=true)', async function() {
|
||||
const allowUnauthenticatedStreamValue = openpgp.config.allowUnauthenticatedStream;
|
||||
openpgp.config.allowUnauthenticatedStream = true;
|
||||
try {
|
||||
@ -507,7 +469,7 @@ function tests() {
|
||||
const reader = stream.getReader(decrypted.data);
|
||||
expect(await reader.peekBytes(1024)).not.to.deep.equal(plaintext[0]);
|
||||
dataArrived();
|
||||
await expect(reader.readToEnd()).to.be.rejectedWith('Ascii armor integrity check failed');
|
||||
await expect(reader.readToEnd()).to.be.rejectedWith('Modification detected.');
|
||||
expect(decrypted.signatures).to.exist.and.have.length(1);
|
||||
await expect(decrypted.signatures[0].verified).to.be.eventually.rejectedWith(/Could not find signing key/);
|
||||
} finally {
|
||||
@ -515,7 +477,7 @@ function tests() {
|
||||
}
|
||||
});
|
||||
|
||||
it('Sign/verify: Detect armor checksum error', async function() {
|
||||
it('Sign/verify: Detect modification', async function() {
|
||||
const signed = await openpgp.sign({
|
||||
message: await openpgp.createMessage({ binary: data }),
|
||||
signingKeys: privKey,
|
||||
@ -541,8 +503,9 @@ function tests() {
|
||||
const reader = stream.getReader(verified.data);
|
||||
expect(await reader.peekBytes(1024)).not.to.deep.equal(plaintext[0]);
|
||||
dataArrived();
|
||||
await expect(reader.readToEnd()).to.be.rejectedWith('Ascii armor integrity check failed');
|
||||
expect(verified.signatures).to.exist.and.have.length(1);
|
||||
await reader.readToEnd();
|
||||
await expect(verified.signatures[0].verified).to.be.rejectedWith('Signed digest did not match');
|
||||
});
|
||||
|
||||
it('stream.transformPair()', async function() {
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user