From 7852c6dd0f00f40463227a39a9ec6ea5782c5d36 Mon Sep 17 00:00:00 2001 From: Giovanni Petrantoni Date: Sat, 20 Jun 2020 19:56:55 +0900 Subject: [PATCH] Noise and eth2/nbc fixes (#226) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Remove noise padding payload (spec removed it) * add log scope in secure * avoid defect array out of range in switch secure when "na" * improve identify traces * wip noise fixes * noise protobuf adjustments (trying) * add more debugging messages/traces, improve their actual contents * re-enable ID check in noise * bump go daemon tag version * bump go daemon tag version * enable noise in daemonapi * interop testing, (both secio and noise will be tested) * azure cache bump (p2pd) * CI changes - Travis: use Go 1.14 - azure-pipelines.yml: big cleanup - Azure: bump cache keys - build 64-bit p2pd on 32-bit Windows - install both Mingw-w64 architectures * noise logging fixes * alternate testing between noise and secio * increase timeout to avoid VM errors in CI (multistream tests) * refactor heartbeat management in gossipsub * remove locking within heartbeat * refactor heartbeat management in gossipsub * remove locking within heartbeat Co-authored-by: Ștefan Talpalaru --- .appveyor.yml | 2 +- .travis.yml | 5 +- azure-pipelines.yml | 208 ++++++++------------------ libp2p.nimble | 17 ++- libp2p/daemon/daemonapi.nim | 1 + libp2p/muxers/muxer.nim | 3 +- libp2p/protocols/identify.nim | 2 +- libp2p/protocols/pubsub/gossipsub.nim | 31 ++-- libp2p/protocols/secure/noise.nim | 75 ++++------ libp2p/protocols/secure/secure.nim | 3 + libp2p/standard_setup.nim | 4 +- libp2p/switch.nim | 15 +- tests/pubsub/testfloodsub.nim | 4 +- tests/pubsub/testgossipsub.nim | 8 +- tests/testinterop.nim | 15 +- tests/testmultistream.nim | 4 +- tests/testswitch.nim | 16 +- 17 files changed, 173 insertions(+), 240 deletions(-) diff --git a/.appveyor.yml b/.appveyor.yml index 29d8a80..22b0141 100644 --- a/.appveyor.yml +++ b/.appveyor.yml @@ -40,7 +40,7 @@ install: # install and build go-libp2p-daemon - curl -O -L -s -S https://raw.githubusercontent.com/status-im/nimbus-build-system/master/scripts/build_p2pd.sh - - bash build_p2pd.sh p2pdCache + - bash build_p2pd.sh p2pdCache v0.2.4 build_script: - nimble install -y --depsOnly diff --git a/.travis.yml b/.travis.yml index 201eca6..c116e17 100644 --- a/.travis.yml +++ b/.travis.yml @@ -12,7 +12,7 @@ git: # when multiple CI builds are queued, the tested commit needs to be in the last X commits cloned with "--depth X" depth: 10 -go: "1.12.x" +go: "1.14.x" matrix: include: @@ -45,9 +45,10 @@ install: # install and build go-libp2p-daemon - curl -O -L -s -S https://raw.githubusercontent.com/status-im/nimbus-build-system/master/scripts/build_p2pd.sh - - bash build_p2pd.sh p2pdCache v0.2.1 + - bash build_p2pd.sh p2pdCache v0.2.4 script: - nimble install -y --depsOnly - nimble test - nimble examples_build + diff --git a/azure-pipelines.yml b/azure-pipelines.yml index caa9581..5e412bc 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -13,186 +13,108 @@ strategy: VM: 'windows-latest' PLATFORM: x64 TEST_LANG: c - # Windows_cpp_64bit: - # VM: 'windows-latest' - # UCPU: amd64 - # PLATFORM: x64 - # TEST_LANG: cpp - #Linux_64bit: - #VM: 'ubuntu-16.04' - #PLATFORM: x64 - #TEST_LANG: c - # Linux_cpp_64bit: - # VM: 'ubuntu-16.04' - # UCPU: amd64 - # PLATFORM: x64 - # TEST_LANG: cpp - - # TODO this requires gcc -m32 as the Ubuntu image is 64-bit - # Linux_32bit: - # VM: 'ubuntu-16.04' - # ARCH: x86 - # PLATFORM: x86 - # TEST_LANG: c - - # TODO: go version on Azure is problematic, - # we have xerrors package issue - # MacOS_64bit: - # VM: 'macOS-10.14' - # PLATFORM: x64 - # TEST_LANG: c pool: vmImage: $(VM) variables: - GOPATH: '$(system.defaultWorkingDirectory)/go' V: 0 # Scripts verbosity, 1 for debugging build scripts steps: - task: CacheBeta@1 displayName: 'cache Nim binaries' inputs: - key: NimBinaries | $(Agent.OS) | $(PLATFORM) | "$(Build.SourceBranchName)" | "v3" + key: NimBinaries | $(Agent.OS) | $(PLATFORM) | "$(Build.SourceBranchName)" | "v4" path: NimBinaries - task: CacheBeta@1 displayName: 'cache Go libp2p daemon' inputs: - key: p2pdCache | $(Agent.OS) | $(PLATFORM) + key: p2pdCache | $(Agent.OS) | $(PLATFORM) | "v3" path: p2pdCache - task: CacheBeta@1 displayName: 'cache MinGW-w64' inputs: - key: mingwCache | 8_1_0 | $(PLATFORM) + key: mingwCache | 8_1_0 | $(PLATFORM) | "v1" path: mingwCache - condition: eq(variables['Agent.OS'], 'Windows_NT') - powershell: | Set-ItemProperty -Path 'HKLM:\SYSTEM\CurrentControlSet\Control\FileSystem' -Name 'LongPathsEnabled' -Value 1 displayName: 'long path support' - condition: eq(variables['Agent.OS'], 'Windows_NT') + - bash: | set -e - echo "PATH=${PATH}" + + # custom MinGW-w64 versions for both 32-bit and 64-bit, since we need a 64-bit build of p2pd echo "Installing MinGW-w64" + + install_mingw() { + mkdir -p mingwCache + cd mingwCache + if [[ ! -e "$MINGW_FILE" ]]; then + rm -f *.7z + curl -OLsS "$MINGW_URL" + fi + 7z x -y -bd "$MINGW_FILE" >/dev/null + mkdir -p /c/custom + mv "$MINGW_DIR" /c/custom/ + cd .. + } + + # 32-bit + MINGW_FILE="i686-8.1.0-release-posix-dwarf-rt_v6-rev0.7z" + MINGW_URL="https://sourceforge.net/projects/mingw-w64/files/Toolchains%20targetting%20Win32/Personal%20Builds/mingw-builds/8.1.0/threads-posix/dwarf/${MINGW_FILE}" + MINGW_DIR="mingw32" + install_mingw + + # 64-bit + MINGW_FILE="x86_64-8.1.0-release-posix-seh-rt_v6-rev0.7z" + MINGW_URL="https://sourceforge.net/projects/mingw-w64/files/Toolchains%20targetting%20Win64/Personal%20Builds/mingw-builds/8.1.0/threads-posix/seh/${MINGW_FILE}" + MINGW_DIR="mingw64" + install_mingw + if [[ $PLATFORM == "x86" ]]; then - MINGW_FILE="i686-8.1.0-release-posix-dwarf-rt_v6-rev0.7z" - MINGW_URL="https://sourceforge.net/projects/mingw-w64/files/Toolchains%20targetting%20Win32/Personal%20Builds/mingw-builds/8.1.0/threads-posix/dwarf/${MINGW_FILE}" MINGW_DIR="mingw32" else - MINGW_FILE="x86_64-8.1.0-release-posix-seh-rt_v6-rev0.7z" - MINGW_URL="https://sourceforge.net/projects/mingw-w64/files/Toolchains%20targetting%20Win64/Personal%20Builds/mingw-builds/8.1.0/threads-posix/seh/${MINGW_FILE}" MINGW_DIR="mingw64" fi - mkdir -p mingwCache - pushd mingwCache - if [[ ! -e "$MINGW_FILE" ]]; then - rm -f *.7z - curl -OLsS "$MINGW_URL" - fi - 7z x -y -bd "$MINGW_FILE" >/dev/null - mkdir -p /c/custom - mv "$MINGW_DIR" /c/custom/ - popd - - # Workaround https://developercommunity.visualstudio.com/content/problem/891929/windows-2019-cygheap-base-mismatch-detected-git-ba.html - echo "##vso[task.prependpath]/usr/bin" - echo "##vso[task.prependpath]/mingw64/bin" - - echo "##vso[task.setvariable variable=MINGW_DIR;]$MINGW_DIR" - displayName: 'Install dependencies (Windows)' - condition: eq(variables['Agent.OS'], 'Windows_NT') - - powershell: | - # export custom mingw PATH to other tasks - echo "##vso[task.prependpath]c:\custom\$(MINGW_DIR)\bin" - displayName: 'Mingw PATH (Windows)' - condition: eq(variables['Agent.OS'], 'Windows_NT') - - - bash: | - set -e - echo "PATH=${PATH}" - export ncpu= - case '$(Agent.OS)' in - 'Linux') - ncpu=$(nproc) - ;; - 'Darwin') - ncpu=$(sysctl -n hw.ncpu) - ;; - 'Windows_NT') - ncpu=$NUMBER_OF_PROCESSORS - ;; - esac - [[ -z "$ncpu" || $ncpu -le 0 ]] && ncpu=1 - echo "Found ${ncpu} cores" - echo "##vso[task.setvariable variable=ncpu;]$ncpu" - displayName: 'Detecting number of cores' - - - bash: | - set -e - echo "PATH=${PATH}" - - # build nim from our own branch - this to avoid the day-to-day churn and - # regressions of the fast-paced Nim development while maintaining the - # flexibility to apply patches - curl -O -L -s -S https://raw.githubusercontent.com/status-im/nimbus-build-system/master/scripts/build_nim.sh - env MAKE="mingw32-make -j${ncpu}" ARCH_OVERRIDE=$(PLATFORM) bash build_nim.sh Nim csources dist/nimble NimBinaries - - displayName: 'Building Nim (Windows)' - condition: eq(variables['Agent.OS'], 'Windows_NT') - - powershell: | - echo "##vso[task.prependpath]$pwd\Nim\bin" - displayName: 'Set env variable (Windows)' - condition: eq(variables['Agent.OS'], 'Windows_NT') - - - bash: | - set -e - echo "PATH=${PATH}" - - # build nim from our own branch - this to avoid the day-to-day churn and - # regressions of the fast-paced Nim development while maintaining the - # flexibility to apply patches - curl -O -L -s -S https://raw.githubusercontent.com/status-im/nimbus-build-system/master/scripts/build_nim.sh - env MAKE="make -j${ncpu}" ARCH_OVERRIDE=$(PLATFORM) bash build_nim.sh Nim csources dist/nimble NimBinaries - displayName: 'Building Nim (POSIX)' - condition: ne(variables['Agent.OS'], 'Windows_NT') - - bash: | - echo "##vso[task.prependpath]$PWD/Nim/bin" - displayName: 'Set env variable (Posix)' - condition: ne(variables['Agent.OS'], 'Windows_NT') - - - bash: | - set -e - # install and build go-libp2p-daemon - go version - - echo "##vso[task.prependpath]$(GOPATH)/bin" - curl -O -L -s -S https://raw.githubusercontent.com/status-im/nimbus-build-system/master/scripts/build_p2pd.sh - bash build_p2pd.sh p2pdCache - displayName: 'Building Go libp2p daemon' - - - bash: | - set -e - echo "PATH=${PATH}" - nimble refresh - nimble install -y --depsOnly - displayName: 'Building dependencies (Posix)' - condition: ne(variables['Agent.OS'], 'Windows_NT') - - powershell: | - echo $Env:Path - nimble refresh - nimble install -y --depsOnly - displayName: 'Building dependencies (Windows)' - condition: eq(variables['Agent.OS'], 'Windows_NT') - - - bash: | - set -e + export PATH="/c/custom/${MINGW_DIR}/bin:${PATH}" echo "PATH=${PATH}" which gcc gcc -v + # detect number of cores + export ncpu="$NUMBER_OF_PROCESSORS" + [[ -z "$ncpu" || $ncpu -le 0 ]] && ncpu=2 + echo "Found ${ncpu} cores" + + # build nim from our own branch - this to avoid the day-to-day churn and + # regressions of the fast-paced Nim development while maintaining the + # flexibility to apply patches + curl -OLsS https://raw.githubusercontent.com/status-im/nimbus-build-system/master/scripts/build_nim.sh + env MAKE="mingw32-make -j${ncpu}" ARCH_OVERRIDE=$(PLATFORM) bash build_nim.sh Nim csources dist/nimble NimBinaries + + export PATH="${PWD}/Nim/bin:${PATH}" + echo "PATH=${PATH}" + + # install and build go-libp2p-daemon + go version + + export GOPATH="${PWD}/go" + export PATH="${GOPATH}/bin:${PATH}" + echo "PATH=${PATH}" + + curl -OLsS https://raw.githubusercontent.com/status-im/nimbus-build-system/master/scripts/build_p2pd.sh + # we can't seem to be able to build a 32-bit p2pd + env PATH="/c/custom/mingw64/bin:${PATH}" bash build_p2pd.sh p2pdCache v0.2.4 + + # install dependencies + nimble refresh + nimble install -y --depsOnly + + # run tests nimble test nimble examples_build - displayName: 'Testing the package' + displayName: 'build and test' + diff --git a/libp2p.nimble b/libp2p.nimble index 914da3e..2675990 100644 --- a/libp2p.nimble +++ b/libp2p.nimble @@ -17,18 +17,19 @@ requires "nim >= 1.2.0", "stew >= 0.1.0" proc runTest(filename: string, verify: bool = true, sign: bool = true) = - var excstr: string = "nim c -r --opt:speed -d:debug --verbosity:0 --hints:off -d:chronicles_log_level=info" - excstr.add(" ") - excstr.add("-d:libp2p_pubsub_sign=" & $sign) - excstr.add(" ") - excstr.add("-d:libp2p_pubsub_verify=" & $verify) - excstr.add(" ") - excstr.add("tests/" & filename) + var excstr = "nim c -r --opt:speed -d:debug --verbosity:0 --hints:off -d:chronicles_log_level=info" + excstr.add(" --warning[CaseTransition]:off --warning[ObservableStores]:off --warning[LockLevel]:off") + excstr.add(" -d:libp2p_pubsub_sign=" & $sign) + excstr.add(" -d:libp2p_pubsub_verify=" & $verify) + excstr.add(" tests/" & filename) exec excstr rmFile "tests/" & filename.toExe proc buildSample(filename: string) = - exec "nim c --opt:speed --threads:on -d:debug --verbosity:0 --hints:off examples/" & filename + var excstr = "nim c --opt:speed --threads:on -d:debug --verbosity:0 --hints:off" + excstr.add(" --warning[CaseTransition]:off --warning[ObservableStores]:off --warning[LockLevel]:off") + excstr.add(" examples/" & filename) + exec excstr rmFile "examples" & filename.toExe task testnative, "Runs libp2p native tests": diff --git a/libp2p/daemon/daemonapi.nim b/libp2p/daemon/daemonapi.nim index b15a0b2..68570dd 100644 --- a/libp2p/daemon/daemonapi.nim +++ b/libp2p/daemon/daemonapi.nim @@ -736,6 +736,7 @@ proc newDaemonApi*(flags: set[P2PDaemonFlags] = {}, if i > 0: opt.add "," opt.add $address args.add(opt) + args.add("-noise=true") args.add("-listen=" & $api.address) # We are trying to get absolute daemon path. diff --git a/libp2p/muxers/muxer.nim b/libp2p/muxers/muxer.nim index 6189e25..efe31a8 100644 --- a/libp2p/muxers/muxer.nim +++ b/libp2p/muxers/muxer.nim @@ -47,6 +47,7 @@ proc newMuxerProvider*(creator: MuxerConstructor, codec: string): MuxerProvider method init(c: MuxerProvider) = proc handler(conn: Connection, proto: string) {.async, gcsafe, closure.} = + trace "starting muxer handler", proto=proto, peer=conn try: let muxer = c.newMuxer(conn) @@ -63,7 +64,7 @@ method init(c: MuxerProvider) = checkFutures(await allFinished(futs)) except CatchableError as exc: - trace "exception in muxer handler", exc = exc.msg + trace "exception in muxer handler", exc = exc.msg, peer=conn, proto=proto c.handler = handler diff --git a/libp2p/protocols/identify.nim b/libp2p/protocols/identify.nim index ef86b38..da46d6c 100644 --- a/libp2p/protocols/identify.nim +++ b/libp2p/protocols/identify.nim @@ -129,7 +129,7 @@ method init*(p: Identify) = proc identify*(p: Identify, conn: Connection, remotePeerInfo: PeerInfo): Future[IdentifyInfo] {.async, gcsafe.} = - trace "initiating identify" + trace "initiating identify", peer=conn var message = await conn.readLp(64*1024) if len(message) == 0: trace "identify: Invalid or empty message received!" diff --git a/libp2p/protocols/pubsub/gossipsub.nim b/libp2p/protocols/pubsub/gossipsub.nim index 4b328c4..85bff0f 100644 --- a/libp2p/protocols/pubsub/gossipsub.nim +++ b/libp2p/protocols/pubsub/gossipsub.nim @@ -53,7 +53,8 @@ type gossip*: Table[string, seq[ControlIHave]] # pending gossip control*: Table[string, ControlMessage] # pending control messages mcache*: MCache # messages cache - heartbeatCancel*: Future[void] # cancellation future for heartbeat interval + heartbeatFut: Future[void] # cancellation future for heartbeat interval + heartbeatRunning: bool heartbeatLock: AsyncLock # heartbeat lock to prevent two consecutive concurrent heartbeats declareGauge(libp2p_gossipsub_peers_per_topic_mesh, "gossipsub peers per topic in mesh", labels = ["topic"]) @@ -211,9 +212,8 @@ proc getGossipPeers(g: GossipSub): Table[string, ControlMessage] {.gcsafe.} = .set(g.gossipsub.getOrDefault(topic).len.int64, labelValues = [topic]) proc heartbeat(g: GossipSub) {.async.} = - while true: + while g.heartbeatRunning: try: - await g.heartbeatLock.acquire() trace "running heartbeat" for t in toSeq(g.topics.keys): @@ -230,9 +230,10 @@ proc heartbeat(g: GossipSub) {.async.} = g.mcache.shift() # shift the cache except CatchableError as exc: trace "exception ocurred in gossipsub heartbeat", exc = exc.msg + # sleep less in the case of an error + # but still throttle + await sleepAsync(100.millis) continue - finally: - g.heartbeatLock.release() await sleepAsync(1.seconds) @@ -497,21 +498,33 @@ method publish*(g: GossipSub, libp2p_pubsub_messages_published.inc(labelValues = [topic]) method start*(g: GossipSub) {.async.} = + debug "gossipsub start" + ## start pubsub ## start long running/repeating procedures + + # interlock start to to avoid overlapping to stops + await g.heartbeatLock.acquire() # setup the heartbeat interval - g.heartbeatCancel = g.heartbeat() + g.heartbeatRunning = true + g.heartbeatFut = g.heartbeat() + + g.heartbeatLock.release() method stop*(g: GossipSub) {.async.} = - ## stopt pubsub + debug "gossipsub stop" + + ## stop pubsub ## stop long running tasks await g.heartbeatLock.acquire() # stop heartbeat interval - if not g.heartbeatCancel.finished: - g.heartbeatCancel.complete() + g.heartbeatRunning = false + if not g.heartbeatFut.finished: + debug "awaiting last heartbeat" + await g.heartbeatFut g.heartbeatLock.release() diff --git a/libp2p/protocols/secure/noise.nim b/libp2p/protocols/secure/noise.nim index 2dd3fe7..0e8d204 100644 --- a/libp2p/protocols/secure/noise.nim +++ b/libp2p/protocols/secure/noise.nim @@ -285,27 +285,6 @@ proc sendHSMessage(sconn: Connection; buf: seq[byte]) {.async.} = outbuf &= buf await sconn.write(outbuf) -proc packNoisePayload(payload: openarray[byte]): seq[byte] = - result &= payload.len.uint16.toBytesBE - result &= payload - - if result.len > uint16.high.int: - raise newException(NoiseOversizedPayloadError, "Trying to send an unsupported oversized payload over Noise") - - trace "packed noise payload", size = payload.len - -proc unpackNoisePayload(payload: var seq[byte]) = - let - besize = payload[0..1] - size = uint16.fromBytesBE(besize).int - - if size != (payload.len - 2): - raise newException(NoiseOversizedPayloadError, "Received a wrong payload size") - - payload = payload[2..^1] - - trace "unpacked noise payload", size = payload.len - proc handshakeXXOutbound(p: Noise, conn: Connection, p2pProof: ProtoBuffer): Future[HandshakeResult] {.async.} = const initiator = true @@ -336,8 +315,7 @@ proc handshakeXXOutbound(p: Noise, conn: Connection, p2pProof: ProtoBuffer): Fut read_s() dh_es() - var remoteP2psecret = hs.ss.decryptAndHash(msg) - unpackNoisePayload(remoteP2psecret) + let remoteP2psecret = hs.ss.decryptAndHash(msg) # -> s, se @@ -347,8 +325,7 @@ proc handshakeXXOutbound(p: Noise, conn: Connection, p2pProof: ProtoBuffer): Fut dh_se() # last payload must follow the ecrypted way of sending - var packed = packNoisePayload(p2psecret) - msg &= hs.ss.encryptAndHash(packed) + msg &= hs.ss.encryptAndHash(p2psecret) await conn.sendHSMessage(msg) @@ -384,8 +361,7 @@ proc handshakeXXInbound(p: Noise, conn: Connection, p2pProof: ProtoBuffer): Futu write_s() dh_es() - var packedSecret = packNoisePayload(p2psecret) - msg &= hs.ss.encryptAndHash(packedSecret) + msg &= hs.ss.encryptAndHash(p2psecret) await conn.sendHSMessage(msg) @@ -396,8 +372,7 @@ proc handshakeXXInbound(p: Noise, conn: Connection, p2pProof: ProtoBuffer): Futu read_s() dh_se() - var remoteP2psecret = hs.ss.decryptAndHash(msg) - unpackNoisePayload(remoteP2psecret) + let remoteP2psecret = hs.ss.decryptAndHash(msg) let (cs1, cs2) = hs.ss.split() return HandshakeResult(cs1: cs1, cs2: cs2, remoteP2psecret: remoteP2psecret, rs: hs.rs) @@ -411,9 +386,7 @@ method readMessage*(sconn: NoiseConnection): Future[seq[byte]] {.async.} = if size > 0: var buffer = newSeq[byte](size) await sconn.stream.readExactly(addr buffer[0], buffer.len) - var plain = sconn.readCs.decryptWithAd([], buffer) - unpackNoisePayload(plain) - return plain + return sconn.readCs.decryptWithAd([], buffer) else: trace "Received 0-length message", conn = $sconn @@ -427,8 +400,7 @@ method write*(sconn: NoiseConnection, message: seq[byte]): Future[void] {.async. while left > 0: let chunkSize = if left > MaxPlainSize: MaxPlainSize else: left - packed = packNoisePayload(message.toOpenArray(offset, offset + chunkSize - 1)) - cipher = sconn.writeCs.encryptWithAd([], packed) + cipher = sconn.writeCs.encryptWithAd([], message.toOpenArray(offset, offset + chunkSize - 1)) left = left - chunkSize offset = offset + chunkSize var @@ -440,8 +412,8 @@ method write*(sconn: NoiseConnection, message: seq[byte]): Future[void] {.async. outbuf &= cipher await sconn.stream.write(outbuf) -method handshake*(p: Noise, conn: Connection, initiator: bool = false): Future[SecureConn] {.async.} = - trace "Starting Noise handshake", initiator +method handshake*(p: Noise, conn: Connection, initiator: bool): Future[SecureConn] {.async.} = + debug "Starting Noise handshake", initiator, peer=conn # https://github.com/libp2p/specs/tree/master/noise#libp2p-data-in-handshake-messages let @@ -450,9 +422,8 @@ method handshake*(p: Noise, conn: Connection, initiator: bool = false): Future[S var libp2pProof = initProtoBuffer() - - libp2pProof.write(initProtoField(1, p.localPublicKey)) - libp2pProof.write(initProtoField(2, signedPayload)) + libp2pProof.write(initProtoField(1, p.localPublicKey.getBytes.tryGet())) + libp2pProof.write(initProtoField(2, signedPayload.getBytes())) # data field also there but not used! libp2pProof.finish() @@ -465,23 +436,35 @@ method handshake*(p: Noise, conn: Connection, initiator: bool = false): Future[S var remoteProof = initProtoBuffer(handshakeRes.remoteP2psecret) remotePubKey: PublicKey + remotePubKeyBytes: seq[byte] remoteSig: Signature - if remoteProof.getValue(1, remotePubKey) <= 0: - raise newException(NoiseHandshakeError, "Failed to deserialize remote public key.") - if remoteProof.getValue(2, remoteSig) <= 0: - raise newException(NoiseHandshakeError, "Failed to deserialize remote public key.") + remoteSigBytes: seq[byte] + + if remoteProof.getLengthValue(1, remotePubKeyBytes) <= 0: + raise newException(NoiseHandshakeError, "Failed to deserialize remote public key bytes. (initiator: " & $initiator & ", peer: " & $conn.peerInfo.peerId & ")") + if remoteProof.getLengthValue(2, remoteSigBytes) <= 0: + raise newException(NoiseHandshakeError, "Failed to deserialize remote signature bytes. (initiator: " & $initiator & ", peer: " & $conn.peerInfo.peerId & ")") + + if not remotePubKey.init(remotePubKeyBytes): + raise newException(NoiseHandshakeError, "Failed to decode remote public key. (initiator: " & $initiator & ", peer: " & $conn.peerInfo.peerId & ")") + if not remoteSig.init(remoteSigBytes): + raise newException(NoiseHandshakeError, "Failed to decode remote signature. (initiator: " & $initiator & ", peer: " & $conn.peerInfo.peerId & ")") let verifyPayload = PayloadString.toBytes & handshakeRes.rs.getBytes if not remoteSig.verify(verifyPayload, remotePubKey): raise newException(NoiseHandshakeError, "Noise handshake signature verify failed.") else: - trace "Remote signature verified" + debug "Remote signature verified", peer=conn if initiator and not isNil(conn.peerInfo): let pid = PeerID.init(remotePubKey) if not conn.peerInfo.peerId.validate(): raise newException(NoiseHandshakeError, "Failed to validate peerId.") if pid != conn.peerInfo.peerId: + var + failedKey: PublicKey + discard extractPublicKey(conn.peerInfo.peerId, failedKey) + debug "Noise handshake, peer infos don't match!", initiator, dealt_peer=conn.peerInfo.peerId, dealt_key=failedKey, received_peer=pid, received_key=remotePubKey raise newException(NoiseHandshakeError, "Noise handshake, peer infos don't match! " & $pid & " != " & $conn.peerInfo.peerId) var secure = new NoiseConnection @@ -498,7 +481,7 @@ method handshake*(p: Noise, conn: Connection, initiator: bool = false): Future[S secure.readCs = handshakeRes.cs1 secure.writeCs = handshakeRes.cs2 - trace "Noise handshake completed!" + debug "Noise handshake completed!", initiator, peer=secure.peerInfo return secure @@ -511,7 +494,7 @@ proc newNoise*(privateKey: PrivateKey; outgoing: bool = true; commonPrologue: se result.outgoing = outgoing result.localPrivateKey = privateKey result.localPublicKey = privateKey.getKey().tryGet() - discard randomBytes(result.noisePrivateKey) + result.noisePrivateKey = Curve25519Key.random().tryGet() result.noisePublicKey = result.noisePrivateKey.public() result.commonPrologue = commonPrologue result.init() diff --git a/libp2p/protocols/secure/secure.nim b/libp2p/protocols/secure/secure.nim index d4e77ec..a12c35f 100644 --- a/libp2p/protocols/secure/secure.nim +++ b/libp2p/protocols/secure/secure.nim @@ -14,6 +14,9 @@ import ../protocol, ../../stream/connection, ../../peerinfo +logScope: + topics = "secure" + type Secure* = ref object of LPProtocol # base type for secure managers diff --git a/libp2p/standard_setup.nim b/libp2p/standard_setup.nim index 8671364..17fcb87 100644 --- a/libp2p/standard_setup.nim +++ b/libp2p/standard_setup.nim @@ -28,9 +28,9 @@ proc newStandardSwitch*(privKey = none(PrivateKey), triggerSelf = false, gossip = false, secureManagers: openarray[SecureProtocol] = [ - # TODO investigate why we're getting fewer peers on public testnets with noise + # array cos order matters SecureProtocol.Secio, - SecureProtocol.Noise, # array cos order matters + SecureProtocol.Noise, ], verifySignature = libp2p_pubsub_verify, sign = libp2p_pubsub_sign, diff --git a/libp2p/switch.nim b/libp2p/switch.nim index c998154..becd99f 100644 --- a/libp2p/switch.nim +++ b/libp2p/switch.nim @@ -63,10 +63,15 @@ proc secure(s: Switch, conn: Connection): Future[Connection] {.async, gcsafe.} = raise newException(CatchableError, "No secure managers registered!") let manager = await s.ms.select(conn, s.secureManagers.mapIt(it.codec)) - if manager.len == 0: + if manager.len == 0 or manager == "na": raise newException(CatchableError, "Unable to negotiate a secure channel!") - result = await s.secureManagers.filterIt(it.codec == manager)[0].secure(conn, true) + trace "securing connection", codec=manager + let secureProtocol = s.secureManagers.filterIt(it.codec == manager) + # ms.select should deal with the correctness of this + # let's avoid duplicating checks but detect if it fails to do it properly + doAssert(secureProtocol.len > 0) + result = await secureProtocol[0].secure(conn, true) proc identify(s: Switch, conn: Connection): Future[PeerInfo] {.async, gcsafe.} = ## identify the connection @@ -107,7 +112,7 @@ proc identify(s: Switch, conn: Connection): Future[PeerInfo] {.async, gcsafe.} = proc mux(s: Switch, conn: Connection): Future[void] {.async, gcsafe.} = ## mux incoming connection - trace "muxing connection" + trace "muxing connection", peer=conn let muxers = toSeq(s.muxers.keys) if muxers.len == 0: warn "no muxers registered, skipping upgrade flow" @@ -115,10 +120,14 @@ proc mux(s: Switch, conn: Connection): Future[void] {.async, gcsafe.} = let muxerName = await s.ms.select(conn, muxers) if muxerName.len == 0 or muxerName == "na": + debug "no muxer available, early exit", peer=conn return # create new muxer for connection let muxer = s.muxers[muxerName].newMuxer(conn) + + trace "found a muxer", name=muxerName, peer=conn + # install stream handler muxer.streamHandler = s.streamHandler diff --git a/tests/pubsub/testfloodsub.nim b/tests/pubsub/testfloodsub.nim index 19f9fdd..319a41e 100644 --- a/tests/pubsub/testfloodsub.nim +++ b/tests/pubsub/testfloodsub.nim @@ -232,7 +232,7 @@ suite "FloodSub": var nodes: seq[Switch] = newSeq[Switch]() for i in 0..