-
Notifications
You must be signed in to change notification settings - Fork 35.7k
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.
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
net: save the network type explicitly in CNetAddr #19534
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,19 +28,35 @@ CNetAddr::CNetAddr() | |
|
||
void CNetAddr::SetIP(const CNetAddr& ipIn) | ||
{ | ||
m_net = ipIn.m_net; | ||
memcpy(ip, ipIn.ip, sizeof(ip)); | ||
} | ||
|
||
void CNetAddr::SetLegacyIPv6(const uint8_t ipv6[16]) | ||
{ | ||
if (memcmp(ipv6, pchIPv4, sizeof(pchIPv4)) == 0) { | ||
m_net = NET_IPV4; | ||
} else if (memcmp(ipv6, pchOnionCat, sizeof(pchOnionCat)) == 0) { | ||
m_net = NET_ONION; | ||
} else if (memcmp(ipv6, g_internal_prefix, sizeof(g_internal_prefix)) == 0) { | ||
m_net = NET_INTERNAL; | ||
} else { | ||
m_net = NET_IPV6; | ||
} | ||
memcpy(ip, ipv6, 16); | ||
} | ||
|
||
void CNetAddr::SetRaw(Network network, const uint8_t *ip_in) | ||
{ | ||
switch(network) | ||
{ | ||
case NET_IPV4: | ||
m_net = NET_IPV4; | ||
memcpy(ip, pchIPv4, 12); | ||
memcpy(ip+12, ip_in, 4); | ||
break; | ||
case NET_IPV6: | ||
memcpy(ip, ip_in, 16); | ||
SetLegacyIPv6(ip_in); | ||
break; | ||
default: | ||
assert(!"invalid network"); | ||
|
@@ -66,6 +82,7 @@ bool CNetAddr::SetInternal(const std::string &name) | |
if (name.empty()) { | ||
return false; | ||
} | ||
m_net = NET_INTERNAL; | ||
unsigned char hash[32] = {}; | ||
CSHA256().Write((const unsigned char*)name.data(), name.size()).Finalize(hash); | ||
memcpy(ip, g_internal_prefix, sizeof(g_internal_prefix)); | ||
|
@@ -89,6 +106,7 @@ bool CNetAddr::SetSpecial(const std::string &strName) | |
std::vector<unsigned char> vchAddr = DecodeBase32(strName.substr(0, strName.size() - 6).c_str()); | ||
if (vchAddr.size() != 16-sizeof(pchOnionCat)) | ||
return false; | ||
m_net = NET_ONION; | ||
memcpy(ip, pchOnionCat, sizeof(pchOnionCat)); | ||
for (unsigned int i=0; i<16-sizeof(pchOnionCat); i++) | ||
ip[i + sizeof(pchOnionCat)] = vchAddr[i]; | ||
|
@@ -123,15 +141,9 @@ bool CNetAddr::IsBindAny() const | |
return true; | ||
} | ||
|
||
bool CNetAddr::IsIPv4() const | ||
{ | ||
return (memcmp(ip, pchIPv4, sizeof(pchIPv4)) == 0); | ||
} | ||
bool CNetAddr::IsIPv4() const { return m_net == NET_IPV4; } | ||
|
||
bool CNetAddr::IsIPv6() const | ||
{ | ||
return (!IsIPv4() && !IsTor() && !IsInternal()); | ||
} | ||
bool CNetAddr::IsIPv6() const { return m_net == NET_IPV6; } | ||
|
||
bool CNetAddr::IsRFC1918() const | ||
{ | ||
|
@@ -165,50 +177,54 @@ bool CNetAddr::IsRFC5737() const | |
|
||
bool CNetAddr::IsRFC3849() const | ||
{ | ||
return GetByte(15) == 0x20 && GetByte(14) == 0x01 && GetByte(13) == 0x0D && GetByte(12) == 0xB8; | ||
return IsIPv6() && GetByte(15) == 0x20 && GetByte(14) == 0x01 && | ||
GetByte(13) == 0x0D && GetByte(12) == 0xB8; | ||
} | ||
|
||
bool CNetAddr::IsRFC3964() const | ||
{ | ||
return (GetByte(15) == 0x20 && GetByte(14) == 0x02); | ||
return IsIPv6() && GetByte(15) == 0x20 && GetByte(14) == 0x02; | ||
} | ||
|
||
bool CNetAddr::IsRFC6052() const | ||
{ | ||
static const unsigned char pchRFC6052[] = {0,0x64,0xFF,0x9B,0,0,0,0,0,0,0,0}; | ||
return (memcmp(ip, pchRFC6052, sizeof(pchRFC6052)) == 0); | ||
return IsIPv6() && memcmp(ip, pchRFC6052, sizeof(pchRFC6052)) == 0; | ||
} | ||
|
||
bool CNetAddr::IsRFC4380() const | ||
{ | ||
return (GetByte(15) == 0x20 && GetByte(14) == 0x01 && GetByte(13) == 0 && GetByte(12) == 0); | ||
return IsIPv6() && GetByte(15) == 0x20 && GetByte(14) == 0x01 && GetByte(13) == 0 && | ||
GetByte(12) == 0; | ||
} | ||
|
||
bool CNetAddr::IsRFC4862() const | ||
{ | ||
static const unsigned char pchRFC4862[] = {0xFE,0x80,0,0,0,0,0,0}; | ||
return (memcmp(ip, pchRFC4862, sizeof(pchRFC4862)) == 0); | ||
return IsIPv6() && memcmp(ip, pchRFC4862, sizeof(pchRFC4862)) == 0; | ||
} | ||
|
||
bool CNetAddr::IsRFC4193() const | ||
{ | ||
return ((GetByte(15) & 0xFE) == 0xFC); | ||
return IsIPv6() && (GetByte(15) & 0xFE) == 0xFC; | ||
} | ||
|
||
bool CNetAddr::IsRFC6145() const | ||
{ | ||
static const unsigned char pchRFC6145[] = {0,0,0,0,0,0,0,0,0xFF,0xFF,0,0}; | ||
return (memcmp(ip, pchRFC6145, sizeof(pchRFC6145)) == 0); | ||
return IsIPv6() && memcmp(ip, pchRFC6145, sizeof(pchRFC6145)) == 0; | ||
} | ||
|
||
bool CNetAddr::IsRFC4843() const | ||
{ | ||
return (GetByte(15) == 0x20 && GetByte(14) == 0x01 && GetByte(13) == 0x00 && (GetByte(12) & 0xF0) == 0x10); | ||
return IsIPv6() && GetByte(15) == 0x20 && GetByte(14) == 0x01 && | ||
GetByte(13) == 0x00 && (GetByte(12) & 0xF0) == 0x10; | ||
} | ||
|
||
bool CNetAddr::IsRFC7343() const | ||
{ | ||
return (GetByte(15) == 0x20 && GetByte(14) == 0x01 && GetByte(13) == 0x00 && (GetByte(12) & 0xF0) == 0x20); | ||
return IsIPv6() && GetByte(15) == 0x20 && GetByte(14) == 0x01 && | ||
GetByte(13) == 0x00 && (GetByte(12) & 0xF0) == 0x20; | ||
} | ||
|
||
bool CNetAddr::IsHeNet() const | ||
|
@@ -222,10 +238,7 @@ bool CNetAddr::IsHeNet() const | |
* | ||
* @see CNetAddr::SetSpecial(const std::string &) | ||
*/ | ||
bool CNetAddr::IsTor() const | ||
{ | ||
return (memcmp(ip, pchOnionCat, sizeof(pchOnionCat)) == 0); | ||
} | ||
bool CNetAddr::IsTor() const { return m_net == NET_ONION; } | ||
|
||
bool CNetAddr::IsLocal() const | ||
{ | ||
|
@@ -235,7 +248,7 @@ bool CNetAddr::IsLocal() const | |
|
||
// IPv6 loopback (::1/128) | ||
static const unsigned char pchLocal[16] = {0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,1}; | ||
if (memcmp(ip, pchLocal, 16) == 0) | ||
if (IsIPv6() && memcmp(ip, pchLocal, 16) == 0) | ||
return true; | ||
Comment on lines
-238
to
252
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In general some of this might be optimized out by the compiler, I would prefer constructions for all those ipv6 overrides in the rest of the code to look like this
And some logging here and in the other overrides i.e.
|
||
|
||
return false; | ||
|
@@ -259,12 +272,12 @@ bool CNetAddr::IsValid() const | |
// header20 vectorlen3 addr26 addr26 addr26 header20 vectorlen3 addr26 addr26 addr26... | ||
// so if the first length field is garbled, it reads the second batch | ||
// of addr misaligned by 3 bytes. | ||
if (memcmp(ip, pchIPv4+3, sizeof(pchIPv4)-3) == 0) | ||
if (IsIPv6() && memcmp(ip, pchIPv4+3, sizeof(pchIPv4)-3) == 0) | ||
return false; | ||
|
||
// unspecified IPv6 address (::/128) | ||
unsigned char ipNone6[16] = {}; | ||
if (memcmp(ip, ipNone6, 16) == 0) | ||
if (IsIPv6() && memcmp(ip, ipNone6, 16) == 0) | ||
return false; | ||
|
||
// documentation IPv6 address | ||
|
@@ -311,7 +324,7 @@ bool CNetAddr::IsRoutable() const | |
*/ | ||
bool CNetAddr::IsInternal() const | ||
{ | ||
return memcmp(ip, g_internal_prefix, sizeof(g_internal_prefix)) == 0; | ||
return m_net == NET_INTERNAL; | ||
} | ||
|
||
enum Network CNetAddr::GetNetwork() const | ||
|
@@ -322,13 +335,7 @@ enum Network CNetAddr::GetNetwork() const | |
if (!IsRoutable()) | ||
return NET_UNROUTABLE; | ||
|
||
if (IsIPv4()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this code still needed in if (IsInternal())
return NET_INTERNAL; There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good question - no. But it was not necessary before this PR either. Some good simplification can be done here even on There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correction - it is needed, otherwise this test would fail: https://github.com/bitcoin/bitcoin/blob/476436b/src/test/netbase_tests.cpp#L45 Anyway, I made some simplifications on top of this PR that remove the "extending" of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks @vasild, looking forward to reviewing the simplifications. |
||
return NET_IPV4; | ||
|
||
if (IsTor()) | ||
return NET_ONION; | ||
|
||
return NET_IPV6; | ||
return m_net; | ||
} | ||
|
||
std::string CNetAddr::ToStringIP() const | ||
|
@@ -362,12 +369,12 @@ std::string CNetAddr::ToString() const | |
|
||
bool operator==(const CNetAddr& a, const CNetAddr& b) | ||
{ | ||
return (memcmp(a.ip, b.ip, 16) == 0); | ||
return a.m_net == b.m_net && memcmp(a.ip, b.ip, 16) == 0; | ||
} | ||
|
||
bool operator<(const CNetAddr& a, const CNetAddr& b) | ||
{ | ||
return (memcmp(a.ip, b.ip, 16) < 0); | ||
return a.m_net < b.m_net || (a.m_net == b.m_net && memcmp(a.ip, b.ip, 16) < 0); | ||
} | ||
|
||
/** | ||
|
@@ -813,7 +820,7 @@ CSubNet::CSubNet(const CNetAddr &addr): | |
*/ | ||
bool CSubNet::Match(const CNetAddr &addr) const | ||
{ | ||
if (!valid || !addr.IsValid()) | ||
if (!valid || !addr.IsValid() || network.m_net != addr.m_net) | ||
return false; | ||
for(int x=0; x<16; ++x) | ||
if ((addr.ip[x] & netmask[x]) != network.ip[x]) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason to pass around fixed-length data with a raw data pointer (of arbitrary length) when type safe alternatives like span or std::array (probably not applicable in this case) are available?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The input has to be 16 bytes, in this case. Is there a way to enforce this in the type, for spans?
(sure, the
[16]
doesn't do anything here either except as a signal to the programmer)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure, but I presumed spans could be of non-dynamic extent as well. Does this not work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, in this case the
[16]
is just a "signal to the programmer". InSetRaw()
we have a bare pointerconst uint8_t*
when calling this function.I have removed
SetRaw()
in the subsequent commit and will revisit this code.Some experiments:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, in #19628 I changed the argument to
const uint8_t (&ipv6)[ADDR_IPv6_SIZE]
mostly to make the interface clean. From 3 callers 2 need an abusive typecasts because they don't have an array (one has bare pointer (the fuzz tests) and the other hasstruct in6_addr
).Feel free to comment there: https://github.com/bitcoin/bitcoin/pull/19628/files#diff-b64569708508232923e5fe3059396334R28