p2p: Allow whitelisting outgoing connections #27114

[rajarshimaitra]

Session Details

• Date: 23-03-2023
• Time: IST 20:00 (UTC 14:30)
• PR: p2p: Allow whitelisting manual connections bitcoin/bitcoin#27114
• Context: [Net]
• Difficulty: Moderate
• Language: [python][c++]

Notes

By default, a Bitcoin Core node allows up to 125 connections (8 of which are outbound) to other peers. Whitelist is a startup option that allows to add permission flags to the peers connecting from the given IP address or CIDR-notated network. It uses the same permissions as -whitebind: bloomfilter, noban, forcerelay, relay, mempool, download, addr).

The intent for whitelisting a peer is to give a trusted peer special permissions(like- not ever banning that peer, bloom filters etc..). we can currently whitelist inbound peers.

References

This PR is a sub PR of : https://bitcoincore.reviews/26441

Questions

• Did you review the PR? Concept ACK, approach ACK, tested ACK, or NACK? What was your review approach?

• What does this PR do?

• What are NetPermissionFlags? How is it set? When do we set NetPermissionFlags::Implicit?

• What does InitializePermissionFlags() function achieve?

• whitelist only allows to add permission flags to inbound peers. Why only for inbound ones? Does it make sense to extend the permissions to outbound peers? Why?

• ConnectionDirection can be Both, In, Out or None. What does Both mean? What happens in TryParsePermissionFlags if it is None? In which scenarios can this happen?

Learning

• net permission flags
• white listing peers
• inbound and outbound connections
• p2p RPCs

[rajarshimaitra]

Summary

By default, a Bitcoin Core node allows up to 125 connections (8 of which are outbound) to other peers. Whitelist is a startup -whitelist option that allows to add extra permission flags to the peers connecting from the given IP address or CIDR-notated network. It uses the same permissions as -whitebind: bloomfilter, noban, forcerelay, relay, mempool, download, addr).

Previously whitelisting was only allowed for inbound peers, and this PR proposes and adds whitelisting for outbound peers too.

Questions

What does this PR do?

This PR proposes to add a new field in the -whitelist option to specify permission of outbound peers. Previous -whitelist was used with agruments
like bloom,mempool@1.2.3.4. After this peer it will have an in or out specifier at the start of the field. Like in,bloom,mempool@1.2.3.4 or out,bloom,mempool@1.2.3.4. Both the specifier can be used simultaneously.

What are NetPermissionFlags? How is it set? When do we set NetPermissionFlags::Implicit?

NetPermissionFlags is an enum that defines a set of permissions, that can be ascribed to any peer. A peer ascribed with a specific
set of net permissions will be allowed to interact with us in those specific ways. For example a NetPermissionFlags::Mempool will
allow the peer to query for our mempool.

src/pet_permission.h

enum class NetPermissionFlags : uint32_t {
    None = 0,
    // Can query bloomfilter even if -peerbloomfilters is false
    BloomFilter = (1U << 1),
    // Relay and accept transactions from this peer, even if -blocksonly is true
    // This peer is also not subject to limits on how many transaction INVs are tracked
    Relay = (1U << 3),
    // Always relay transactions from this peer, even if already in mempool
    // Keep parameter interaction: forcerelay implies relay
    ForceRelay = (1U << 2) | Relay,
    // Allow getheaders during IBD and block-download after maxuploadtarget limit
    Download = (1U << 6),
    // Can't be banned/disconnected/discouraged for misbehavior
    NoBan = (1U << 4) | Download,
    // Can query the mempool
    Mempool = (1U << 5),
    // Can request addrs without hitting a privacy-preserving cache, and send us
    // unlimited amounts of addrs.
    Addr = (1U << 7),

    // True if the user did not specifically set fine-grained permissions with
    // the -whitebind or -whitelist configuration options.
    Implicit = (1U << 31),
    All = BloomFilter | ForceRelay | Relay | NoBan | Mempool | Download | Addr,
};

NetPermissionFlags is a uint32_t bitflag. This means a peer can have multiple permission flags set to them. The NetPermissionFlags::HasFlag() method checks if the peer's permission flags has a specific flag set in it.

The NetPermissionFlags::Implicit is the default set of permission flags. This is ascribed to all peers for which we don't set permissions manually. The default permissions are Mempool and NoBan. In the original codebase, the CConnman::CreateNodeFromAcceptedSocket() which is called when a new node is accepted in a network socket, handles the Implicit permissions.

src/net.cpp ()

void CConnman::CreateNodeFromAcceptedSocket(std::unique_ptr<Sock>&& sock,
                                            NetPermissionFlags permission_flags,
                                            const CAddress& addr_bind,
                                            const CAddress& addr)
{
    int nInbound = 0;
    int nMaxInbound = nMaxConnections - m_max_outbound;

    AddWhitelistPermissionFlags(permission_flags, addr);
    if (NetPermissions::HasFlag(permission_flags, NetPermissionFlags::Implicit)) {
        NetPermissions::ClearFlag(permission_flags, NetPermissionFlags::Implicit);
        if (gArgs.GetBoolArg("-whitelistforcerelay", DEFAULT_WHITELISTFORCERELAY)) NetPermissions::AddFlag(permission_flags, NetPermissionFlags::ForceRelay);
        if (gArgs.GetBoolArg("-whitelistrelay", DEFAULT_WHITELISTRELAY)) NetPermissions::AddFlag(permission_flags, NetPermissionFlags::Relay);
        NetPermissions::AddFlag(permission_flags, NetPermissionFlags::Mempool);
        NetPermissions::AddFlag(permission_flags, NetPermissionFlags::NoBan);
    }

...

What does InitializePermissionFlags() function achieve?

The InitializePermissionFlags is a newly added function in this PR, which does the Implicite permission setting, that was previously handled in
CreateNodeFromAcceptedSocket.

void CConnman::InitializePermissionFlags(NetPermissionFlags& flags, ServiceFlags& service_flags) {
    if (NetPermissions::HasFlag(flags, NetPermissionFlags::Implicit)) {
        NetPermissions::ClearFlag(flags, NetPermissionFlags::Implicit);
        if (gArgs.GetBoolArg("-whitelistforcerelay", DEFAULT_WHITELISTFORCERELAY)) NetPermissions::AddFlag(flags, NetPermissionFlags::ForceRelay);
        if (gArgs.GetBoolArg("-whitelistrelay", DEFAULT_WHITELISTRELAY)) NetPermissions::AddFlag(flags, NetPermissionFlags::Relay);
        NetPermissions::AddFlag(flags, NetPermissionFlags::Mempool);
        NetPermissions::AddFlag(flags, NetPermissionFlags::NoBan);
    }

    if (NetPermissions::HasFlag(flags, NetPermissionFlags::BloomFilter)) {
        service_flags = static_cast<ServiceFlags>(service_flags | NODE_BLOOM);
    }
}

Additionally, handling the permission flags, it also sets the service flags of our node if the permission is BloomFilter.

whitelist only allows to add permission flags to inbound peers. Why only for inbound ones? Does it make sense to extend the permissions to outbound peers? Why?

Whitelisting was perviously only allowed for inbound connections as the outbound connections already enjoys certain privilleges. The first proposed
PR for whitelisting outbound had concern by gmaxwell that it can have unintended consequence in the transaction propagation. bitcoin/bitcoin#10051 (comment).

However no critical or clear consequences of whitelisting outbound have been reported by anybody and various people Concept ACKed the larger PR
of this work. bitcoin/bitcoin#26441.

For certain situation where a node operator needs to conenct to specific trusted PR while giving it certain privilleges whitelisting outbounds
makes sense.

ConnectionDirection can be Both, In, Out or None. What does Both mean? What happens in TryParsePermissionFlags if it is None? In which scenarios can this happen?

In case when both In and Out connection direction is specified then the peer will have those specific permission irrespective of
it's inbound or outbound.

The TryParsePermissionFlags tries to parse the permission flags from a permission string.

src/net_permission.cpp

// Parse the following format: "perm1,perm2@xxxxxx"
bool TryParsePermissionFlags(const std::string& str, NetPermissionFlags& output, ConnectionDirection* output_connection_direction, size_t& readen, bilingual_str& error)
{
 ...
            if (permission == "bloomfilter" || permission == "bloom") NetPermissions::AddFlag(flags, NetPermissionFlags::BloomFilter);
            else if (permission == "noban") NetPermissions::AddFlag(flags, NetPermissionFlags::NoBan);
            else if (permission == "forcerelay") NetPermissions::AddFlag(flags, NetPermissionFlags::ForceRelay);
            else if (permission == "mempool") NetPermissions::AddFlag(flags, NetPermissionFlags::Mempool);
            else if (permission == "download") NetPermissions::AddFlag(flags, NetPermissionFlags::Download);
            else if (permission == "all") NetPermissions::AddFlag(flags, NetPermissionFlags::All);
            else if (permission == "relay") NetPermissions::AddFlag(flags, NetPermissionFlags::Relay);
            else if (permission == "addr") NetPermissions::AddFlag(flags, NetPermissionFlags::Addr);
            else if (permission == "in") connection_direction |= ConnectionDirection::In;
            else if (permission == "out") {
                if (!output_connection_direction) {
                    error = _("whitebind is only used for incoming connections");
                    return false;
                }
                connection_direction |= ConnectionDirection::Out;
            }
            else if (permission.length() == 0); // Allow empty entries
            else {
                error = strprintf(_("Invalid P2P permission: '%s'"), permission);
                return false;
            }
        }
        readen++;
    }

    // By default, whitelist only applies to incoming connections
    if (connection_direction == ConnectionDirection::None) connection_direction = ConnectionDirection::In;

    output = flags;
    if (output_connection_direction) *output_connection_direction = connection_direction;
    error = Untranslated("");
    return true;
}

If output_connection_direction is nullptr, then the internal if logic will throw the error "whitebind is only used for incoming connections". This also hints that this parameter is only used for setting the whitelist configuration, and for whitebind it still preserves old behavior of only setting permission to the inbound connections.

The whitelist permission flag setting looks as below

src/net_permission.cpp

bool NetWhitelistPermissions::TryParse(const std::string& str, NetWhitelistPermissions& output, ConnectionDirection& output_connection_direction, bilingual_str& error)
{
    NetPermissionFlags flags;
    size_t offset;
    if (!TryParsePermissionFlags(str, flags, &output_connection_direction, offset, error)) return false;

    const std::string net = str.substr(offset);
    CSubNet subnet;
    LookupSubNet(net, subnet);
    if (!subnet.IsValid()) {
        error = strprintf(_("Invalid netmask specified in -whitelist: '%s'"), net);
        return false;
    }

    output.m_flags = flags;
    output.m_subnet = subnet;
    error = Untranslated("");
    return true;
}

And whitebind is also correspondingly similar.

src/net_permissions.cpp

bool NetWhitebindPermissions::TryParse(const std::string& str, NetWhitebindPermissions& output, bilingual_str& error)
{
    NetPermissionFlags flags;
    size_t offset;
    if (!TryParsePermissionFlags(str, flags, nullptr, offset, error)) return false;

    const std::string strBind = str.substr(offset);
    CService addrBind;
    if (!Lookup(strBind, addrBind, 0, false)) {
        error = ResolveErrMsg("whitebind", strBind);
        return false;
    }
    if (addrBind.GetPort() == 0) {
        error = strprintf(_("Need to specify a port with -whitebind: '%s'"), strBind);
        return false;
    }

    output.m_flags = flags;
    output.m_service = addrBind;
    error = Untranslated("");
    return true;
}