diff --git a/src/avalanche/delegation.h b/src/avalanche/delegation.h --- a/src/avalanche/delegation.h +++ b/src/avalanche/delegation.h @@ -43,14 +43,15 @@ const DelegationId &getId() const { return dgid; } const ProofId &getProofId() const { return proofid; } + std::optional getMaster() const; SERIALIZE_METHODS(Delegation, obj) { READWRITE(obj.proofid, obj.levels); SER_READ(obj, obj.dgid = obj.computeDelegationId()); } - bool verify(DelegationState &state, const Proof &proof, - CPubKey &auth) const; + bool verify(DelegationState &state, const CPubKey &proofMaster, + const LimitedProofId &limitedProofId, CPubKey &auth) const; }; } // namespace avalanche diff --git a/src/avalanche/delegation.cpp b/src/avalanche/delegation.cpp --- a/src/avalanche/delegation.cpp +++ b/src/avalanche/delegation.cpp @@ -37,14 +37,17 @@ return DelegationId(hash); } -bool Delegation::verify(DelegationState &state, const Proof &proof, +bool Delegation::verify(DelegationState &state, const CPubKey &proofMaster, + const LimitedProofId &limitedProofId, CPubKey &auth) const { - if (proof.getId() != proofid) { + ProofId computedProofId = + Proof::computeIdFromLimitedId(limitedProofId, proofMaster); + if (computedProofId != proofid) { return state.Invalid(DelegationResult::INCORRECT_PROOF); } uint256 hash = proofid; - const CPubKey *pauth = &proof.getMaster(); + const CPubKey *pauth = &proofMaster; bool ret = reduceLevels(hash, levels, [&](const Level &l) { if (!pauth->VerifySchnorr(hash, l.sig)) { @@ -60,4 +63,10 @@ return ret; } +std::optional Delegation::getMaster() const { + return levels.size() == 0 + ? std::nullopt + : std::make_optional(levels.back().pubkey); +} + } // namespace avalanche diff --git a/src/avalanche/peermanager.cpp b/src/avalanche/peermanager.cpp --- a/src/avalanche/peermanager.cpp +++ b/src/avalanche/peermanager.cpp @@ -22,12 +22,15 @@ const PeerId peerid = it->peerid; - DelegationState state; - CPubKey pubkey; - if (!delegation.verify(state, proof, pubkey)) { + // This can only be wrong if proof-delegation pairing is broken in the + // network layer. + if (proof.getId() != delegation.getProofId()) { return false; } + auto delegationMaster = delegation.getMaster(); + CPubKey pubkey = delegationMaster ? *delegationMaster : proof.getMaster(); + auto nit = nodes.find(nodeid); if (nit == nodes.end()) { if (!nodes.emplace(nodeid, peerid, std::move(pubkey)).second) { diff --git a/src/avalanche/processor.cpp b/src/avalanche/processor.cpp --- a/src/avalanche/processor.cpp +++ b/src/avalanche/processor.cpp @@ -494,9 +494,12 @@ } } - connman->PushMessage(pfrom, CNetMsgMaker(pfrom->GetCommonVersion()) - .Make(NetMsgType::AVAHELLO, - Hello(peerData->delegation, sig))); + connman->PushMessage( + pfrom, + CNetMsgMaker(pfrom->GetCommonVersion()) + .Make(NetMsgType::AVAHELLO, + Hello(peerData->delegation, peerData->proof.getLimitedId(), + peerData->proof.getMaster(), sig))); return true; } diff --git a/src/avalanche/proof.h b/src/avalanche/proof.h --- a/src/avalanche/proof.h +++ b/src/avalanche/proof.h @@ -113,6 +113,9 @@ bool verify(ProofValidationState &state) const; bool verify(ProofValidationState &state, const CCoinsView &view) const; + + static ProofId computeIdFromLimitedId(const LimitedProofId &limitedId, + const CPubKey &masterIn); }; } // namespace avalanche diff --git a/src/avalanche/proof.cpp b/src/avalanche/proof.cpp --- a/src/avalanche/proof.cpp +++ b/src/avalanche/proof.cpp @@ -47,6 +47,14 @@ return true; } +ProofId Proof::computeIdFromLimitedId(const LimitedProofId &limitedId, + const CPubKey &master) { + CHashWriter ss(SER_GETHASH, 0); + ss << limitedId; + ss << master; + return ProofId(ss.GetHash()); +} + void Proof::computeProofId() { CHashWriter ss(SER_GETHASH, 0); ss << sequence; @@ -58,12 +66,7 @@ } limitedProofId = LimitedProofId(ss.GetHash()); - - CHashWriter ss2(SER_GETHASH, 0); - ss2 << limitedProofId; - ss2 << master; - - proofid = ProofId(ss2.GetHash()); + proofid = computeIdFromLimitedId(limitedProofId, master); } uint32_t Proof::getScore() const { diff --git a/src/avalanche/protocol.h b/src/avalanche/protocol.h --- a/src/avalanche/protocol.h +++ b/src/avalanche/protocol.h @@ -67,16 +67,22 @@ class Hello { Delegation delegation; + LimitedProofId limitedProofId; + CPubKey proofMaster; SchnorrSig sig; public: - Hello(Delegation delegationIn, SchnorrSig sigIn) - : delegation(std::move(delegationIn)), sig(sigIn) {} + Hello(Delegation delegationIn, LimitedProofId limitedProofIdIn, + CPubKey proofMasterIn, SchnorrSig sigIn) + : delegation(std::move(delegationIn)), limitedProofId(limitedProofIdIn), + proofMaster(proofMasterIn), sig(sigIn) {} SchnorrSig GetSig() { return sig; } // serialization support - SERIALIZE_METHODS(Hello, obj) { READWRITE(obj.delegation, obj.sig); } + SERIALIZE_METHODS(Hello, obj) { + READWRITE(obj.delegation, obj.limitedProofId, obj.proofMaster, obj.sig); + } }; } // namespace avalanche diff --git a/src/avalanche/test/delegation_tests.cpp b/src/avalanche/test/delegation_tests.cpp --- a/src/avalanche/test/delegation_tests.cpp +++ b/src/avalanche/test/delegation_tests.cpp @@ -23,12 +23,13 @@ const CPubKey &expected_pubkey) { DelegationState state; CPubKey pubkey; - BOOST_CHECK(dg.verify(state, p, pubkey)); + BOOST_CHECK(dg.verify(state, p.getMaster(), p.getLimitedId(), pubkey)); BOOST_CHECK(state.GetResult() == DelegationResult::NONE); BOOST_CHECK(pubkey == expected_pubkey); const Proof altp = buildRandomProof(654321); - BOOST_CHECK(!dg.verify(state, altp, pubkey)); + BOOST_CHECK( + !dg.verify(state, altp.getMaster(), altp.getLimitedId(), pubkey)); BOOST_CHECK(state.GetResult() == DelegationResult::INCORRECT_PROOF); } @@ -180,8 +181,9 @@ DelegationState state; CPubKey pubkey; - BOOST_CHECK_EQUAL(dg.verify(state, p, pubkey), - c.result == DelegationResult::NONE); + BOOST_CHECK_EQUAL( + dg.verify(state, p.getMaster(), p.getLimitedId(), pubkey), + c.result == DelegationResult::NONE); BOOST_CHECK(state.GetResult() == c.result); BOOST_CHECK(pubkey == CPubKey(ParseHex(c.pubkey))); } diff --git a/src/net_processing.cpp b/src/net_processing.cpp --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -4007,16 +4007,30 @@ avalanche::Delegation &delegation = pfrom.m_avalanche_state->delegation; verifier >> delegation; - avalanche::Proof proof; + avalanche::LimitedProofId limitedProofId; + verifier >> limitedProofId; + + CPubKey proofMaster; + verifier >> proofMaster; + avalanche::DelegationState state; CPubKey pubkey; - if (!delegation.verify(state, proof, pubkey)) { + if (!delegation.verify(state, proofMaster, limitedProofId, pubkey)) { Misbehaving(pfrom, 100, "invalid-delegation"); return; } SchnorrSig sig; verifier >> sig; + if (!pubkey.VerifySchnorr(g_avalanche->buildRemoteSighash(&pfrom), + sig)) { + Misbehaving(pfrom, 100, "invalid-avahello-signature"); + return; + } + LogPrint(BCLog::NET, + "Successfully checked AVAHELLO signature for peer=%d\n", + pfrom.GetId()); + return; } if (msg_type == NetMsgType::AVAPOLL) { diff --git a/src/rpc/avalanche.cpp b/src/rpc/avalanche.cpp --- a/src/rpc/avalanche.cpp +++ b/src/rpc/avalanche.cpp @@ -353,7 +353,8 @@ ss >> dg; avalanche::DelegationState dgState; - if (!dg.verify(dgState, proof, auth)) { + if (!dg.verify(dgState, proof.getMaster(), proof.getLimitedId(), + auth)) { throw JSONRPCError(RPC_INVALID_PARAMETER, "The supplied delegation is not valid"); } diff --git a/test/functional/abc_p2p_avalanche.py b/test/functional/abc_p2p_avalanche.py --- a/test/functional/abc_p2p_avalanche.py +++ b/test/functional/abc_p2p_avalanche.py @@ -4,17 +4,24 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test the resolution of forks via avalanche.""" import random +import struct from test_framework.avatools import create_coinbase_stakes from test_framework.key import ( + bytes_to_wif, ECKey, ECPubKey, ) from test_framework.mininode import P2PInterface, mininode_lock from test_framework.messages import ( + AvalancheDelegation, + AvalancheProof, AvalancheResponse, AvalancheVote, CInv, + FromHex, + hash256, + msg_avahello, msg_avapoll, msg_tcpavaresponse, NODE_AVALANCHE, @@ -110,6 +117,38 @@ with mininode_lock: return self.avahello + def _getLocalSigHash(self, delegation_id: bytes) -> bytes: + b = delegation_id + b += struct.pack(" P2PInterface)") + interface = get_node() + avahello = interface.wait_for_avahello().hello avakey.set(bytes.fromhex(node.getavalanchekey())) assert avakey.verify_schnorr( - avahello.sig, avahello.get_sighash(poll_node)) + avahello.sig, avahello.get_sighash(interface)) + assert_equal(avahello.master, pubkey.get_bytes()) + assert_equal(avahello.limited_proofid, + FromHex(AvalancheProof(), proof).limited_proofid) + + self.log.info("Test the avahello signature (P2PInterface -> node)") + stakes = create_coinbase_stakes(node, [blockhashes[1]], addrkey0.key) + interface_proof_hex = node.buildavalancheproof( + proof_sequence, proof_expiration, pubkey.get_bytes().hex(), + stakes) + limitedid = FromHex( + AvalancheProof(), + interface_proof_hex).limited_proofid + # delegate + delegated_privkey = ECKey() + delegated_privkey.generate() + interface_delegation_hex = node.delegateavalancheproof( + interface_proof_hex, + bytes_to_wif(privkey.get_bytes()), + delegated_privkey.get_pubkey().get_bytes().hex(), + None + ) + + with self.nodes[0].assert_debug_log(["Successfully checked AVAHELLO signature for peer=0"]): + interface.send_avahello( + interface_delegation_hex, + limitedid, + pubkey.get_bytes(), + delegated_privkey) + + self.log.info("Test a wrong avahello signature") + interface = get_node() + with self.nodes[0].assert_debug_log( + ["Misbehaving", "peer=1 (0 -> 100) BAN THRESHOLD EXCEEDED: invalid-avahello-signature"]): + interface.send_avahello_wrong_signature( + interface_delegation_hex, limitedid, pubkey.get_bytes()) + + self.log.info("Test a wrong avahello proofid") + interface = get_node() + with self.nodes[0].assert_debug_log( + ["Misbehaving", "peer=2 (0 -> 100) BAN THRESHOLD EXCEEDED: invalid-delegation"]): + interface.send_avahello( + interface_delegation_hex, + limitedid - 1, + pubkey.get_bytes(), + delegated_privkey) + + self.log.info("Test a wrong avahello proof master key") + interface = get_node() + + wrong_pubkey = pubkey.get_bytes()[::-1] + with self.nodes[0].assert_debug_log( + ["Misbehaving", "peer=3 (0 -> 100) BAN THRESHOLD EXCEEDED: invalid-delegation"]): + interface.send_avahello( + interface_delegation_hex, + limitedid, + wrong_pubkey, + delegated_privkey) if __name__ == '__main__': diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py --- a/test/functional/test_framework/messages.py +++ b/test/functional/test_framework/messages.py @@ -1100,25 +1100,34 @@ class AvalancheHello(): - __slots__ = ("delegation", "sig") + __slots__ = ("delegation", "limited_proofid", "master", "sig") - def __init__(self, delegation=AvalancheDelegation(), sig=b"\0" * 64): + def __init__(self, delegation=AvalancheDelegation(), + limited_proofid=0, + master=b"\0", + sig=b"\0" * 64): self.delegation = delegation + self.limited_proofid = limited_proofid + self.master = master self.sig = sig def deserialize(self, f): self.delegation.deserialize(f) + self.limited_proofid = deser_uint256(f) + self.master = deser_string(f) self.sig = f.read(64) def serialize(self): r = b"" r += self.delegation.serialize() + r += ser_uint256(self.limited_proofid) + r += ser_string(self.master) r += self.sig return r def __repr__(self): - return "AvalancheHello(delegation={}, sig={})".format( - repr(self.delegation), self.sig) + return "AvalancheHello(delegation={}, limitedid={:064x}, master={}, sig={})".format( + repr(self.delegation), self.limited_proofid, self.master, self.sig) def get_sighash(self, node): b = self.delegation.getid()