From 7bad317d0ed1f8b644e7f882e5b9283bb36c529f Mon Sep 17 00:00:00 2001 From: Belma Gutlic Date: Sun, 29 Dec 2019 18:23:43 +0100 Subject: [PATCH] Address PR comments --- src/encoder.ts | 8 ++++---- src/handshakes/abstract-handshake.ts | 8 ++++---- src/handshakes/ik.ts | 14 +++++++------- src/handshakes/xx.ts | 16 ++++++++-------- src/noise.ts | 6 +++--- src/utils.ts | 4 ++++ 6 files changed, 30 insertions(+), 26 deletions(-) diff --git a/src/encoder.ts b/src/encoder.ts index 8837d4f..e4e2c30 100644 --- a/src/encoder.ts +++ b/src/encoder.ts @@ -2,17 +2,17 @@ import {Buffer} from "buffer"; import {bytes} from "./@types/basic"; import {MessageBuffer} from "./@types/handshake"; -export const int16BEEncode = (value, target, offset) => { +export const uint16BEEncode = (value, target, offset) => { target = target || Buffer.allocUnsafe(2); return target.writeUInt16BE(value, offset); }; -int16BEEncode.bytes = 2; +uint16BEEncode.bytes = 2; -export const int16BEDecode = data => { +export const uint16BEDecode = data => { if (data.length < 2) throw RangeError('Could not decode int16BE'); return data.readUInt16BE(0); }; -int16BEDecode.bytes = 2; +uint16BEDecode.bytes = 2; export function encodeMessageBuffer(message: MessageBuffer): bytes { return Buffer.concat([message.ne, message.ns, message.ciphertext]); diff --git a/src/handshakes/abstract-handshake.ts b/src/handshakes/abstract-handshake.ts index f775fe7..24d1d6c 100644 --- a/src/handshakes/abstract-handshake.ts +++ b/src/handshakes/abstract-handshake.ts @@ -1,13 +1,13 @@ import {Buffer} from "buffer"; -import { AEAD, x25519, HKDF, SHA256 } from 'bcrypto'; +import { AEAD, x25519, SHA256 } from 'bcrypto'; import {bytes, bytes32, uint32} from "../@types/basic"; import {CipherState, MessageBuffer, SymmetricState} from "../@types/handshake"; import {getHkdf} from "../utils"; -export abstract class AbstractHandshake { - protected minNonce = 0; +const minNonce = 0; +export abstract class AbstractHandshake { public encryptWithAd(cs: CipherState, ad: bytes, plaintext: bytes): bytes { const e = this.encrypt(cs.k, cs.n, ad, plaintext); this.setNonce(cs, this.incrementNonce(cs.n)); @@ -122,7 +122,7 @@ export abstract class AbstractHandshake { } protected initializeKey(k: bytes32): CipherState { - const n = this.minNonce; + const n = minNonce; return { k, n }; } diff --git a/src/handshakes/ik.ts b/src/handshakes/ik.ts index 03b2c52..f7c04ff 100644 --- a/src/handshakes/ik.ts +++ b/src/handshakes/ik.ts @@ -1,12 +1,11 @@ import {Buffer} from "buffer"; -import {x25519} from "bcrypto"; +import {BN} from "bn.js"; -import {CipherState, HandshakeState, MessageBuffer, NoiseSession, SymmetricState} from "../@types/handshake"; +import {HandshakeState, MessageBuffer, NoiseSession} from "../@types/handshake"; import {bytes, bytes32} from "../@types/basic"; -import {generateKeypair, getHkdf} from "../utils"; +import {generateKeypair, getHkdf, isValidPublicKey} from "../utils"; import {AbstractHandshake} from "./abstract-handshake"; import {KeyPair} from "../@types/libp2p"; -import {BN} from "bn.js"; export class IKHandshake extends AbstractHandshake { @@ -69,6 +68,7 @@ export class IKHandshake extends AbstractHandshake { session.h = h; session.cs1 = cs1; session.cs2 = cs2; + delete session.hs; } else if (session.mc.gtn(1)) { if (session.i) { if (!session.cs2) { @@ -119,14 +119,14 @@ export class IKHandshake extends AbstractHandshake { } private readMessageA(hs: HandshakeState, message: MessageBuffer): bytes { - if (x25519.publicKeyVerify(message.ne)) { + if (isValidPublicKey(message.ne)) { hs.re = message.ne; } this.mixHash(hs.ss, hs.re); this.mixKey(hs.ss, this.dh(hs.s.privateKey, hs.re)); const ns = this.decryptAndHash(hs.ss, message.ns); - if (ns.length === 32 && x25519.publicKeyVerify(message.ns)) { + if (ns.length === 32 && isValidPublicKey(message.ns)) { hs.rs = ns; } this.mixKey(hs.ss, this.dh(hs.s.privateKey, hs.rs)); @@ -134,7 +134,7 @@ export class IKHandshake extends AbstractHandshake { } private readMessageB(hs: HandshakeState, message: MessageBuffer) { - if (x25519.publicKeyVerify(message.ne)) { + if (isValidPublicKey(message.ne)) { hs.re = message.ne; } diff --git a/src/handshakes/xx.ts b/src/handshakes/xx.ts index 3bee635..3b8b0a9 100644 --- a/src/handshakes/xx.ts +++ b/src/handshakes/xx.ts @@ -1,11 +1,10 @@ import { Buffer } from 'buffer'; -import { AEAD, x25519, HKDF, SHA256 } from 'bcrypto'; import { BN } from 'bn.js'; -import { bytes32, uint32, uint64, bytes } from '../@types/basic' +import { bytes32, bytes } from '../@types/basic' import { KeyPair } from '../@types/libp2p' -import {generateKeypair, getHkdf} from '../utils'; -import { CipherState, HandshakeState, Hkdf, MessageBuffer, NoiseSession, SymmetricState } from "../@types/handshake"; +import {generateKeypair, getHkdf, isValidPublicKey} from '../utils'; +import { HandshakeState, MessageBuffer, NoiseSession } from "../@types/handshake"; import {AbstractHandshake} from "./abstract-handshake"; @@ -68,7 +67,7 @@ export class XXHandshake extends AbstractHandshake { } private readMessageA(hs: HandshakeState, message: MessageBuffer): bytes { - if (x25519.publicKeyVerify(message.ne)) { + if (isValidPublicKey(message.ne)) { hs.re = message.ne; } @@ -77,7 +76,7 @@ export class XXHandshake extends AbstractHandshake { } private readMessageB(hs: HandshakeState, message: MessageBuffer): bytes { - if (x25519.publicKeyVerify(message.ne)) { + if (isValidPublicKey(message.ne)) { hs.re = message.ne; } @@ -87,7 +86,7 @@ export class XXHandshake extends AbstractHandshake { } this.mixKey(hs.ss, this.dh(hs.e.privateKey, hs.re)); const ns = this.decryptAndHash(hs.ss, message.ns); - if (ns.length === 32 && x25519.publicKeyVerify(message.ns)) { + if (ns.length === 32 && isValidPublicKey(message.ns)) { hs.rs = ns; } this.mixKey(hs.ss, this.dh(hs.e.privateKey, hs.rs)); @@ -96,7 +95,7 @@ export class XXHandshake extends AbstractHandshake { private readMessageC(hs: HandshakeState, message: MessageBuffer) { const ns = this.decryptAndHash(hs.ss, message.ns); - if (ns.length === 32 && x25519.publicKeyVerify(message.ns)) { + if (ns.length === 32 && isValidPublicKey(message.ns)) { hs.rs = ns; } @@ -141,6 +140,7 @@ export class XXHandshake extends AbstractHandshake { session.h = h; session.cs1 = cs1; session.cs2 = cs2; + delete session.hs; } else if (session.mc.gtn(2)) { if (session.i) { if (!session.cs1) { diff --git a/src/noise.ts b/src/noise.ts index 83b313e..068ade3 100644 --- a/src/noise.ts +++ b/src/noise.ts @@ -8,7 +8,7 @@ import lp from 'it-length-prefixed'; import { Handshake } from "./handshake"; import { generateKeypair } from "./utils"; -import { int16BEDecode, int16BEEncode } from "./encoder"; +import { uint16BEDecode, uint16BEEncode } from "./encoder"; import { decryptStream, encryptStream } from "./crypto"; import { bytes } from "./@types/basic"; import { NoiseConnection, PeerId, KeyPair, SecureOutbound } from "./@types/libp2p"; @@ -108,9 +108,9 @@ export class Noise implements NoiseConnection { secure, // write to wrapper ensureBuffer, // ensure any type of data is converted to buffer encryptStream(handshake), // data is encrypted - lp.encode({ lengthEncoder: int16BEEncode }), // prefix with message length + lp.encode({ lengthEncoder: uint16BEEncode }), // prefix with message length network, // send to the remote peer - lp.decode({ lengthDecoder: int16BEDecode }), // read message length prefix + lp.decode({ lengthDecoder: uint16BEDecode }), // read message length prefix ensureBuffer, // ensure any type of data is converted to buffer decryptStream(handshake), // decrypt the incoming data secure // pipe to the wrapper diff --git a/src/utils.ts b/src/utils.ts index e749040..005f826 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -101,3 +101,7 @@ export function getHkdf(ck: bytes32, ikm: bytes): Hkdf { return [ k1, k2, k3 ]; } + +export function isValidPublicKey(pk: bytes): boolean { + return x25519.publicKeyVerify(pk); +}