From f59fa54ecf0677cc7a96ec9a5e72957d6e0dea13 Mon Sep 17 00:00:00 2001 From: Robert Nelson Date: Fri, 29 Nov 2013 19:29:57 -0800 Subject: [PATCH] Fix ascii dearmor and signature verification bugs --- src/encoding/openpgp.encoding.asciiarmor.js | 106 +++++++++++++------- src/openpgp.js | 2 +- src/openpgp.msg.message.js | 10 +- src/packet/openpgp.packet.signature.js | 12 ++- 4 files changed, 83 insertions(+), 47 deletions(-) diff --git a/src/encoding/openpgp.encoding.asciiarmor.js b/src/encoding/openpgp.encoding.asciiarmor.js index ede8443c..098e6482 100644 --- a/src/encoding/openpgp.encoding.asciiarmor.js +++ b/src/encoding/openpgp.encoding.asciiarmor.js @@ -25,8 +25,6 @@ */ function openpgp_encoding_deArmor(text) { text = text.replace(/\r/g, ''); - // remove whitespace of blank line to allow later split at \n\n - text = text.replace(/\n\s+\n/, '\n\n'); var type = openpgp_encoding_get_type(text); @@ -34,69 +32,99 @@ function openpgp_encoding_deArmor(text) { var splittedtext = text.split('-----'); // splittedtext[0] - should be the empty string // splittedtext[1] - should be BEGIN... - // splittedtext[2] - the message and checksum - // splittedtest[3] - should be END... + // splittedtext[2] - \nthe message and checksum + // splittedtext[3] - should be END... // chunks separated by blank lines - var splittedChunks = splittedtext[2] - .split('\n\n'); - var messageAndChecksum = splittedtext[2] - .split('\n\n')[1] - .split('\n='); - - var message = messageAndChecksum[0] - .replace(/\n- /g,"\n"); - // sometimes, there's a blank line between message and checksum - var checksum; - if(messageAndChecksum.length == 1){ - // blank line - checksum = splittedtext[2] - .replace(/[\n=]/g, ""); - } else { - // no blank line - checksum = messageAndChecksum[1] - .split('\n')[0]; - } + var msg = openpgp_encoding_split_headers(splittedtext[2].slice(1)); + var msg_sum = openpgp_encoding_split_checksum(msg.body); var data = { - openpgp: openpgp_encoding_base64_decode(message), + openpgp: openpgp_encoding_base64_decode(msg_sum.body), type: type }; - if (verifyCheckSum(data.openpgp, checksum)) { + if (verifyCheckSum(data.openpgp, msg_sum.checksum)) { return data; } else { util.print_error("Ascii armor integrity check on message failed: '" - + checksum + + msg_sum.checksum + "' should be '" + getCheckSum(data) + "'"); return false; } } else { var splittedtext = text.split('-----'); + // splittedtext[0] - should be the empty string + // splittedtext[1] - should be BEGIN PGP SIGNED MESSAGE + // splittedtext[2] - \nthe message + // splittedtext[3] - should be BEGIN PGP SIGNATURE + // splittedtext[4] - \nthe signature and checksum + // splittedtext[5] - should be END PGP SIGNATURE + + var msg = openpgp_encoding_split_headers(splittedtext[2].slice(1)); + var sig = openpgp_encoding_split_headers(splittedtext[4].slice(1)); + var sig_sum = openpgp_encoding_split_checksum(sig.body); var result = { - text: splittedtext[2] - .replace(/\n- /g,"\n") - .split("\n\n")[1], - openpgp: openpgp_encoding_base64_decode(splittedtext[4] - .split("\n\n")[1] - .split("\n=")[0]), + text: msg.body.replace(/\n\n$/, "\n").replace(/\n/g, "\r\n"), + openpgp: openpgp_encoding_base64_decode(sig_sum.body), type: type }; - if (verifyCheckSum(result.openpgp, splittedtext[4] - .split("\n\n")[1] - .split("\n=")[1])) - - return result; - else { + if (verifyCheckSum(result.openpgp, sig_sum.checksum)) { + return result; + } else { util.print_error("Ascii armor integrity check on message failed"); return false; } } } +/** + * Splits a message into two parts, the headers and the body. This is an internal function + * @param {String} text OpenPGP armored message part + * @returns {(Boolean|Object)} Either false in case of an error + * or an object with attribute "headers" containing the headers and + * and an attribute "body" containing the body. + */ +function openpgp_encoding_split_headers(text) { + var reEmptyLine = /^[\t ]*\n/m; + var headers = ""; + var body = text; + + var matchResult = reEmptyLine.exec(text); + + if (matchResult != null) { + headers = text.slice(0, matchResult.index); + body = text.slice(matchResult.index + matchResult[0].length); + } + + return { headers: headers, body: body }; +} + +/** + * Splits a message into two parts, the body and the checksum. This is an internal function + * @param {String} text OpenPGP armored message part + * @returns {(Boolean|Object)} Either false in case of an error + * or an object with attribute "body" containing the body + * and an attribute "checksum" containing the checksum. + */ +function openpgp_encoding_split_checksum(text) { + var reChecksumStart = /^=/m; + var body = text; + var checksum = ""; + + var matchResult = reChecksumStart.exec(text); + + if (matchResult != null) { + body = text.slice(0, matchResult.index); + checksum = text.slice(matchResult.index + 1); + } + + return { body: body, checksum: checksum }; +} + /** * Finds out which Ascii Armoring type is used. This is an internal function * @param {String} text [String] ascii armored text @@ -124,7 +152,7 @@ function openpgp_encoding_get_type(text) { return 1; } else - // BEGIN PGP SIGNATURE + // BEGIN PGP SIGNED MESSAGE // Used for detached signatures, OpenPGP/MIME signatures, and // cleartext signatures. Note that PGP 2.x uses BEGIN PGP MESSAGE // for detached signatures. diff --git a/src/openpgp.js b/src/openpgp.js index 6774ae42..5a8ec6c0 100644 --- a/src/openpgp.js +++ b/src/openpgp.js @@ -130,7 +130,7 @@ function _openpgp () { function read_message(armoredText) { var dearmored; try{ - dearmored = openpgp_encoding_deArmor(armoredText.replace(/\r/g,'')); + dearmored = openpgp_encoding_deArmor(armoredText); } catch(e){ util.print_error('no message found!'); diff --git a/src/openpgp.msg.message.js b/src/openpgp.msg.message.js index 3df1d66a..418a38de 100644 --- a/src/openpgp.msg.message.js +++ b/src/openpgp.msg.message.js @@ -82,7 +82,7 @@ function openpgp_msg_message() { function verifySignature(pubkey) { var result = false; if (this.signature.tagType == 2) { - if(!pubkey || pubkey.length == 0){ + if (!pubkey || pubkey.length == 0) { var pubkey; if (this.signature.version == 4) { pubkey = openpgp.keyring.getPublicKeysForKeyId(this.signature.issuerKeyId); @@ -93,14 +93,14 @@ function openpgp_msg_message() { return false; } } - if (pubkey.length == 0) + if (pubkey.length == 0) { util.print_warning("Unable to verify signature of issuer: "+util.hexstrdump(this.signature.issuerKeyId)+". Public key not found in keyring."); - else { + } else { for (var i = 0 ; i < pubkey.length; i++) { - var tohash = this.text.replace(/\r\n/g,"\n").replace(/\n/g,"\r\n"); - if (this.signature.verify(tohash, pubkey[i])) { + if (this.signature.verify(this.text, pubkey[i])) { util.print_info("Found Good Signature from "+pubkey[i].obj.userIds[0].text+" (0x"+util.hexstrdump(pubkey[i].obj.getKeyId()).substring(8)+")"); result = true; + break; } else { util.print_error("Signature verification failed: Bad Signature from "+pubkey[i].obj.userIds[0].text+" (0x"+util.hexstrdump(pubkey[0].obj.getKeyId()).substring(8)+")"); } diff --git a/src/packet/openpgp.packet.signature.js b/src/packet/openpgp.packet.signature.js index 32eb465d..476f1e4e 100644 --- a/src/packet/openpgp.packet.signature.js +++ b/src/packet/openpgp.packet.signature.js @@ -497,14 +497,21 @@ function openpgp_packet_signature() { if (this.version == 4) { this.verified = openpgp_crypto_verifySignature(this.publicKeyAlgorithm, this.hashAlgorithm, this.MPIs, key.obj.publicKeyPacket.MPIs, data+this.signatureData+trailer); + } else { + this.verified = false; } break; case 1: // 0x01: Signature of a canonical text document. if (this.version == 4) { + var tohash = data + .replace(/\r\n/g,"\n") + .replace(/[\t ]+\n/g, "\n") + .replace(/\n/g,"\r\n"); this.verified = openpgp_crypto_verifySignature(this.publicKeyAlgorithm, this.hashAlgorithm, - this.MPIs, key.obj.publicKeyPacket.MPIs, data+this.signatureData+trailer); - return this.verified; + this.MPIs, key.obj.publicKeyPacket.MPIs, tohash+this.signatureData+trailer); + } else { + this.verified = false; } break; @@ -630,6 +637,7 @@ function openpgp_packet_signature() { // document) that cannot include a target subpacket. default: util.print_error("openpgp.packet.signature.js\n"+"signature verification for type"+ this.signatureType+" not implemented"); + this.verified = false; break; } return this.verified;