Skip to content

Harden spork6 logic then remove spork6 #3662

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
UdjinM6 merged 5 commits into from
Aug 30, 2020
Merged

Harden spork6 logic then remove spork6 #3662

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6302119
Harden Spork6
PastaPastaPasta Aug 19, 2020
aee38d0
remove now unused SPORK_6_NEW_SIGS
PastaPastaPasta Aug 19, 2020
7b8f41e
force fSporkSixActive to be correct, otherwise return
PastaPastaPasta Aug 21, 2020
43c69ae
Harden spork6 even more
UdjinM6 Aug 22, 2020
e253393
Add TODO in chainparams as a reminder to drop all spork6 related code…
UdjinM6 Aug 22, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/chainparams.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -489,7 +489,7 @@ class CTestNetParams : public CChainParams {
consensus.fPowAllowMinDifficultyBlocks = true;
consensus.fPowNoRetargeting = false;
consensus.nPowKGWHeight = 4002; // nPowKGWHeight >= nPowDGWHeight means "no KGW"
consensus.nPowDGWHeight = 4002;
consensus.nPowDGWHeight = 4002; // TODO: make sure to drop all spork6 related code on next testnet reset
consensus.nRuleChangeActivationThreshold = 1512; // 75% for testchains
consensus.nMinerConfirmationWindow = 2016; // nPowTargetTimespan / nPowTargetSpacing
consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].bit = 28;
Expand Down
19 changes: 6 additions & 13 deletions src/governance/governance-vote.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,8 @@ bool CGovernanceVote::Sign(const CKey& key, const CKeyID& keyID)
{
std::string strError;

if (sporkManager.IsSporkActive(SPORK_6_NEW_SIGS)) {
// Harden Spork6 so that it is active on testnet and no other networks
if (Params().NetworkIDString() == CBaseChainParams::TESTNET) {
uint256 hash = GetSignatureHash();

if (!CHashSigner::SignHash(hash, key, vchSig)) {
Expand Down Expand Up @@ -198,21 +199,13 @@ bool CGovernanceVote::CheckSignature(const CKeyID& keyID) const
{
std::string strError;

if (sporkManager.IsSporkActive(SPORK_6_NEW_SIGS)) {
// Harden Spork6 so that it is active on testnet and no other networks
if (Params().NetworkIDString() == CBaseChainParams::TESTNET) {
uint256 hash = GetSignatureHash();

if (!CHashSigner::VerifyHash(hash, keyID, vchSig, strError)) {
// could be a signature in old format
std::string strMessage = masternodeOutpoint.ToStringShort() + "|" + nParentHash.ToString() + "|" +
std::to_string(nVoteSignal) + "|" +
std::to_string(nVoteOutcome) + "|" +
std::to_string(nTime);

if (!CMessageSigner::VerifyMessage(keyID, vchSig, strMessage, strError)) {
// nope, not in old format either
LogPrint(BCLog::GOBJECT, "CGovernanceVote::IsValid -- VerifyMessage() failed, error: %s\n", strError);
return false;
}
LogPrint(BCLog::GOBJECT, "CGovernanceVote::IsValid -- VerifyHash() failed, error: %s\n", strError);
return false;
}
} else {
std::string strMessage = masternodeOutpoint.ToStringShort() + "|" + nParentHash.ToString() + "|" +
Expand Down
65 changes: 25 additions & 40 deletions src/spork.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ const std::string CSporkManager::SERIALIZATION_VERSION_STRING = "CSporkManager-V
std::vector<CSporkDef> sporkDefs = {
MAKE_SPORK_DEF(SPORK_2_INSTANTSEND_ENABLED, 4070908800ULL), // OFF
MAKE_SPORK_DEF(SPORK_3_INSTANTSEND_BLOCK_FILTERING, 4070908800ULL), // OFF
MAKE_SPORK_DEF(SPORK_6_NEW_SIGS, 4070908800ULL), // OFF
MAKE_SPORK_DEF(SPORK_9_SUPERBLOCKS_ENABLED, 4070908800ULL), // OFF
MAKE_SPORK_DEF(SPORK_17_QUORUM_DKG_ENABLED, 4070908800ULL), // OFF
MAKE_SPORK_DEF(SPORK_19_CHAINLOCKS_ENABLED, 4070908800ULL), // OFF
Expand Down Expand Up @@ -81,12 +80,10 @@ void CSporkManager::CheckAndRemove()
mapSporksByHash.erase(itSignerPair->second.GetHash());
continue;
}
if (!itSignerPair->second.CheckSignature(itSignerPair->first, false)) {
if (!itSignerPair->second.CheckSignature(itSignerPair->first, true)) {
mapSporksByHash.erase(itSignerPair->second.GetHash());
itActive->second.erase(itSignerPair++);
continue;
}
if (!itSignerPair->second.CheckSignature(itSignerPair->first)) {
mapSporksByHash.erase(itSignerPair->second.GetHash());
itActive->second.erase(itSignerPair++);
continue;
}
++itSignerPair;
}
Expand All @@ -101,8 +98,7 @@ void CSporkManager::CheckAndRemove()
while (itByHash != mapSporksByHash.end()) {
bool found = false;
for (const auto& signer: setSporkPubKeyIDs) {
if (itByHash->second.CheckSignature(signer, false) ||
itByHash->second.CheckSignature(signer, true)) {
if (itByHash->second.CheckSignature(signer)) {
found = true;
break;
}
Expand Down Expand Up @@ -141,17 +137,12 @@ void CSporkManager::ProcessSpork(CNode* pfrom, const std::string& strCommand, CD
}

CKeyID keyIDSigner;
bool fSpork6IsActive = IsSporkActive(SPORK_6_NEW_SIGS);
if (!spork.GetSignerKeyID(keyIDSigner, fSpork6IsActive) || !setSporkPubKeyIDs.count(keyIDSigner)) {
// Note: unlike for other messages we have to check for new format even with SPORK_6_NEW_SIGS
// inactive because SPORK_6_NEW_SIGS default is OFF and it is not the first spork to sync
// (and even if it would, spork order can't be guaranteed anyway).
if (!spork.GetSignerKeyID(keyIDSigner, !fSpork6IsActive) || !setSporkPubKeyIDs.count(keyIDSigner)) {
LOCK(cs_main);
LogPrint(BCLog::SPORK, "CSporkManager::ProcessSpork -- ERROR: invalid signature\n");
Misbehaving(pfrom->GetId(), 100);
return;
}

if (!spork.GetSignerKeyID(keyIDSigner) || !setSporkPubKeyIDs.count(keyIDSigner)) {
LOCK(cs_main);
LogPrint(BCLog::SPORK, "CSporkManager::ProcessSpork -- ERROR: invalid signature\n");
Misbehaving(pfrom->GetId(), 100);
return;
}

{
Expand Down Expand Up @@ -197,14 +188,13 @@ bool CSporkManager::UpdateSpork(SporkId nSporkID, int64_t nValue, CConnman& conn

LOCK(cs);

bool fSpork6IsActive = IsSporkActive(SPORK_6_NEW_SIGS);
if (!spork.Sign(sporkPrivKey, fSpork6IsActive)) {
if (!spork.Sign(sporkPrivKey)) {
LogPrintf("CSporkManager::%s -- ERROR: signing failed for spork %d\n", __func__, nSporkID);
return false;
}

CKeyID keyIDSigner;
if (!spork.GetSignerKeyID(keyIDSigner, fSpork6IsActive) || !setSporkPubKeyIDs.count(keyIDSigner)) {
if (!spork.GetSignerKeyID(keyIDSigner) || !setSporkPubKeyIDs.count(keyIDSigner)) {
LogPrintf("CSporkManager::UpdateSpork: failed to find keyid for private key\n");
return false;
}
Expand Down Expand Up @@ -314,7 +304,7 @@ bool CSporkManager::SetPrivKey(const std::string& strPrivKey)
}

CSporkMessage spork;
if (!spork.Sign(key, IsSporkActive(SPORK_6_NEW_SIGS))) {
if (!spork.Sign(key)) {
LogPrintf("CSporkManager::SetPrivKey -- Test signing failed\n");
return false;
}
Expand Down Expand Up @@ -346,7 +336,7 @@ uint256 CSporkMessage::GetSignatureHash() const
return s.GetHash();
}

bool CSporkMessage::Sign(const CKey& key, bool fSporkSixActive)
bool CSporkMessage::Sign(const CKey& key)
{
if (!key.IsValid()) {
LogPrintf("CSporkMessage::Sign -- signing key is not valid\n");
Expand All @@ -356,7 +346,8 @@ bool CSporkMessage::Sign(const CKey& key, bool fSporkSixActive)
CKeyID pubKeyId = key.GetPubKey().GetID();
std::string strError = "";

if (fSporkSixActive) {
// Harden Spork6 so that it is active on testnet and no other networks
if (Params().NetworkIDString() == CBaseChainParams::TESTNET) {
uint256 hash = GetSignatureHash();

if(!CHashSigner::SignHash(hash, key, vchSig)) {
Expand Down Expand Up @@ -385,41 +376,35 @@ bool CSporkMessage::Sign(const CKey& key, bool fSporkSixActive)
return true;
}

bool CSporkMessage::CheckSignature(const CKeyID& pubKeyId, bool fSporkSixActive) const
bool CSporkMessage::CheckSignature(const CKeyID& pubKeyId) const
{
std::string strError = "";

if (fSporkSixActive) {
// Harden Spork6 so that it is active on testnet and no other networks
if (Params().NetworkIDString() == CBaseChainParams::TESTNET) {
uint256 hash = GetSignatureHash();

if (!CHashSigner::VerifyHash(hash, pubKeyId, vchSig, strError)) {
// Note: unlike for many other messages when SPORK_6_NEW_SIGS is ON sporks with sigs in old format
// and newer timestamps should not be accepted, so if we failed here - that's it
LogPrint(BCLog::SPORK, "CSporkMessage::CheckSignature -- VerifyHash() failed, error: %s\n", strError);
return false;
}
} else {
std::string strMessage = std::to_string(nSporkID) + std::to_string(nValue) + std::to_string(nTimeSigned);

if (!CMessageSigner::VerifyMessage(pubKeyId, vchSig, strMessage, strError)){
// Note: unlike for other messages we have to check for new format even with SPORK_6_NEW_SIGS
// inactive because SPORK_6_NEW_SIGS default is OFF and it is not the first spork to sync
// (and even if it would, spork order can't be guaranteed anyway).
uint256 hash = GetSignatureHash();
if (!CHashSigner::VerifyHash(hash, pubKeyId, vchSig, strError)) {
LogPrint(BCLog::SPORK, "CSporkMessage::CheckSignature -- VerifyHash() failed, error: %s\n", strError);
return false;
}
LogPrint(BCLog::SPORK, "CSporkMessage::CheckSignature -- VerifyMessage() failed, error: %s\n", strError);
return false;
}
}

return true;
}

bool CSporkMessage::GetSignerKeyID(CKeyID &retKeyidSporkSigner, bool fSporkSixActive)
bool CSporkMessage::GetSignerKeyID(CKeyID &retKeyidSporkSigner)
{
CPubKey pubkeyFromSig;
if (fSporkSixActive) {
// Harden Spork6 so that it is active on testnet and no other networks
if (Params().NetworkIDString() == CBaseChainParams::TESTNET) {
if (!pubkeyFromSig.RecoverCompact(GetSignatureHash(), vchSig)) {
return false;
}
Expand Down
7 changes: 3 additions & 4 deletions src/spork.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ class CSporkManager;
enum SporkId : int32_t {
SPORK_2_INSTANTSEND_ENABLED = 10001,
SPORK_3_INSTANTSEND_BLOCK_FILTERING = 10002,
SPORK_6_NEW_SIGS = 10005,
SPORK_9_SUPERBLOCKS_ENABLED = 10008,
SPORK_17_QUORUM_DKG_ENABLED = 10016,
SPORK_19_CHAINLOCKS_ENABLED = 10018,
Expand Down Expand Up @@ -121,13 +120,13 @@ class CSporkMessage
/**
* Sign will sign the spork message with the given key.
*/
bool Sign(const CKey& key, bool fSporkSixActive);
bool Sign(const CKey& key);

/**
* CheckSignature will ensure the spork signature matches the provided public
* key hash.
*/
bool CheckSignature(const CKeyID& pubKeyId, bool fSporkSixActive) const;
bool CheckSignature(const CKeyID& pubKeyId) const;

/**
* GetSignerKeyID is used to recover the spork address of the key used to
Expand All @@ -136,7 +135,7 @@ class CSporkMessage
* This method was introduced along with the multi-signer sporks feature,
* in order to identify which spork key signed this message.
*/
bool GetSignerKeyID(CKeyID& retKeyidSporkSigner, bool fSporkSixActive);
bool GetSignerKeyID(CKeyID& retKeyidSporkSigner);

/**
* Relay is used to send this spork message to other peers.
Expand Down