Skip to content

Commit

Permalink
chore: clean up logic (#165)
Browse files Browse the repository at this point in the history
- decodePayload is synchronous
- remove isValidPeerId - only decode peer id once
- don't use the return value of verifySignedPayload anywhere
- remove duplicate length check, use isValidPublicKey
  • Loading branch information
wemeetagain authored Jul 14, 2022
1 parent 1e3d207 commit 7a0d8c0
Show file tree
Hide file tree
Showing 6 changed files with 15 additions and 21 deletions.
4 changes: 2 additions & 2 deletions src/handshake-ik.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ export class IKHandshake implements IHandshake {
throw new Error('ik handshake stage 0 decryption validation fail')
}
logger('IK Stage 0 - Responder got message, going to verify payload.')
const decodedPayload = await decodePayload(plaintext)
const decodedPayload = decodePayload(plaintext)
this.remotePeer = this.remotePeer || await getPeerIdFromPayload(decodedPayload)
await verifySignedPayload(this.session.hs.rs, decodedPayload, this.remotePeer)
this.setRemoteEarlyData(decodedPayload.data)
Expand All @@ -99,7 +99,7 @@ export class IKHandshake implements IHandshake {
if (!valid) {
throw new Error('ik stage 1 decryption validation fail')
}
const decodedPayload = await decodePayload(plaintext)
const decodedPayload = decodePayload(plaintext)
this.remotePeer = this.remotePeer || await getPeerIdFromPayload(decodedPayload)
await verifySignedPayload(receivedMessageBuffer.ns.slice(0, 32), decodedPayload, this.remotePeer)
this.setRemoteEarlyData(decodedPayload.data)
Expand Down
2 changes: 1 addition & 1 deletion src/handshake-xx-fallback.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ export class XXFallbackHandshake extends XXHandshake {

logger("Initiator going to check remote's signature...")
try {
const decodedPayload = await decodePayload(plaintext)
const decodedPayload = decodePayload(plaintext)
this.remotePeer = this.remotePeer || await getPeerIdFromPayload(decodedPayload)
await verifySignedPayload(this.session.hs.rs, decodedPayload, this.remotePeer)
this.setRemoteEarlyData(decodedPayload.data)
Expand Down
6 changes: 3 additions & 3 deletions src/handshake-xx.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,9 @@ export class XXHandshake implements IHandshake {

logger("Initiator going to check remote's signature...")
try {
const decodedPayload = await decodePayload(plaintext)
const decodedPayload = decodePayload(plaintext)
this.remotePeer = this.remotePeer || await getPeerIdFromPayload(decodedPayload)
this.remotePeer = await verifySignedPayload(this.session.hs.rs, decodedPayload, this.remotePeer)
await verifySignedPayload(this.session.hs.rs, decodedPayload, this.remotePeer)
this.setRemoteEarlyData(decodedPayload.data)
} catch (e) {
const err = e as Error
Expand Down Expand Up @@ -129,7 +129,7 @@ export class XXHandshake implements IHandshake {
logger('Stage 2 - Responder received the message, finished handshake.')

try {
const decodedPayload = await decodePayload(plaintext)
const decodedPayload = decodePayload(plaintext)
this.remotePeer = this.remotePeer || await getPeerIdFromPayload(decodedPayload)
await verifySignedPayload(this.session.hs.rs, decodedPayload, this.remotePeer)
this.setRemoteEarlyData(decodedPayload.data)
Expand Down
2 changes: 1 addition & 1 deletion src/handshakes/ik.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ export class IK extends AbstractHandshake {
this.mixHash(hs.ss, hs.re)
this.mixKey(hs.ss, this.dh(hs.s.privateKey, hs.re))
const { plaintext: ns, valid: valid1 } = this.decryptAndHash(hs.ss, message.ns)
if (valid1 && ns.length === 32 && isValidPublicKey(ns)) {
if (valid1 && isValidPublicKey(ns)) {
hs.rs = ns
}
this.mixKey(hs.ss, this.dh(hs.s.privateKey, hs.rs))
Expand Down
4 changes: 2 additions & 2 deletions src/handshakes/xx.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ export class XX extends AbstractHandshake {
}
this.mixKey(hs.ss, this.dh(hs.e.privateKey, hs.re))
const { plaintext: ns, valid: valid1 } = this.decryptAndHash(hs.ss, message.ns)
if (valid1 && ns.length === 32 && isValidPublicKey(ns)) {
if (valid1 && isValidPublicKey(ns)) {
hs.rs = ns
}
this.mixKey(hs.ss, this.dh(hs.e.privateKey, hs.rs))
Expand All @@ -97,7 +97,7 @@ export class XX extends AbstractHandshake {

private readMessageC (hs: HandshakeState, message: MessageBuffer): {h: bytes, plaintext: bytes, valid: boolean, cs1: CipherState, cs2: CipherState} {
const { plaintext: ns, valid: valid1 } = this.decryptAndHash(hs.ss, message.ns)
if (valid1 && ns.length === 32 && isValidPublicKey(ns)) {
if (valid1 && isValidPublicKey(ns)) {
hs.rs = ns
}
if (!hs.e) {
Expand Down
18 changes: 6 additions & 12 deletions src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,6 @@ export function getHandshakePayload (publicKey: bytes): bytes {
return uint8ArrayConcat([prefix, publicKey], prefix.length + publicKey.length)
}

async function isValidPeerId (peerId: PeerId, publicKeyProtobuf: bytes): Promise<boolean> {
const generatedPeerId = await peerIdFromKeys(publicKeyProtobuf)
return generatedPeerId.equals(peerId)
}

/**
* Verifies signed payload, throws on any irregularities.
*
Expand All @@ -80,31 +75,30 @@ export async function verifySignedPayload (
payload: pb.NoiseHandshakePayload,
remotePeer: PeerId
): Promise<PeerId> {
const identityKey = payload.identityKey
if (!(await isValidPeerId(remotePeer, identityKey))) {
// Unmarshaling from PublicKey protobuf
const payloadPeerId = await peerIdFromKeys(payload.identityKey)
if (!payloadPeerId.equals(remotePeer)) {
throw new Error("Peer ID doesn't match libp2p public key.")
}
const generatedPayload = getHandshakePayload(noiseStaticKey)
// Unmarshaling from PublicKey protobuf
const peerId = await peerIdFromKeys(identityKey)

if (peerId.publicKey == null) {
if (payloadPeerId.publicKey == null) {
throw new Error('PublicKey was missing from PeerId')
}

if (payload.identitySig == null) {
throw new Error('Signature was missing from message')
}

const publicKey = unmarshalPublicKey(peerId.publicKey)
const publicKey = unmarshalPublicKey(payloadPeerId.publicKey)

const valid = await publicKey.verify(generatedPayload, payload.identitySig)

if (!valid) {
throw new Error("Static key doesn't match to peer that signed payload!")
}

return peerId
return payloadPeerId
}

export function isValidPublicKey (pk: bytes): boolean {
Expand Down

0 comments on commit 7a0d8c0

Please sign in to comment.