Skip to content
This repository has been archived by the owner on Aug 16, 2021. It is now read-only.

Fix rpc default binding #3960

Closed

Conversation

rowandh
Copy link
Collaborator

@rowandh rowandh commented Sep 19, 2019

Closes #3959.

Note: this just replicates the original RPC default binding behaviour, which is expressed kind of strangely.

@dangershony @zeptin @quantumagi do these defaults look correct to you:

  • If allowip isn't specified, use "::1" and "127.0.0.1".
  • If allowip is specified, but no bindings are specified and there's no default bindings, use "::" and "0.0.0.0".

@fassadlr
Copy link
Collaborator

@rowandh can you merge latest master? Also, I think some of the tests that are failing is due to your changes 👍

@rowandh
Copy link
Collaborator Author

rowandh commented Sep 23, 2019

@fassadlr there is a test failing due to the change in defaults, was just waiting for confirmation that they are correct and then I will fix it 👍

@zeptin
Copy link
Contributor

zeptin commented Oct 2, 2019

Here is the relevant code from bitcoind:

/** Bind HTTP server to specified addresses */
static bool HTTPBindAddresses(struct evhttp* http)
{
    int http_port = gArgs.GetArg("-rpcport", BaseParams().RPCPort());
    std::vector<std::pair<std::string, uint16_t> > endpoints;

    // Determine what addresses to bind to
    if (!(gArgs.IsArgSet("-rpcallowip") && gArgs.IsArgSet("-rpcbind"))) { // Default to loopback if not allowing external IPs
        endpoints.push_back(std::make_pair("::1", http_port));
        endpoints.push_back(std::make_pair("127.0.0.1", http_port));
        if (gArgs.IsArgSet("-rpcallowip")) {
            LogPrintf("WARNING: option -rpcallowip was specified without -rpcbind; this doesn't usually make sense\n");
        }
        if (gArgs.IsArgSet("-rpcbind")) {
            LogPrintf("WARNING: option -rpcbind was ignored because -rpcallowip was not specified, refusing to allow everyone to connect\n");
        }
    } else if (gArgs.IsArgSet("-rpcbind")) { // Specific bind address
        for (const std::string& strRPCBind : gArgs.GetArgs("-rpcbind")) {
            int port = http_port;
            std::string host;
            SplitHostPort(strRPCBind, port, host);
            endpoints.push_back(std::make_pair(host, port));
        }
    }

    // Bind addresses
    for (std::vector<std::pair<std::string, uint16_t> >::iterator i = endpoints.begin(); i != endpoints.end(); ++i) {
        LogPrint(BCLog::HTTP, "Binding RPC on address %s port %i\n", i->first, i->second);
        evhttp_bound_socket *bind_handle = evhttp_bind_socket_with_handle(http, i->first.empty() ? nullptr : i->first.c_str(), i->second);
        if (bind_handle) {
            CNetAddr addr;
            if (i->first.empty() || (LookupHost(i->first.c_str(), addr, false) && addr.IsBindAny())) {
                LogPrintf("WARNING: the RPC server is not safe to expose to untrusted networks such as the public internet\n");
            }
            boundSockets.push_back(bind_handle);
        } else {
            LogPrintf("Binding RPC on address %s port %i failed.\n", i->first, i->second);
        }
    }
    return !boundSockets.empty();
}

So If allowip isn't specified, use "::1" and "127.0.0.1". definitely appears to be correct. I'm not so sure about the latter statement - the bitcoind code warns against binding to wildcard addresses. Although the one comment in the C++ code doesn't really make sense to me because there isn't code that will "allow everyone to connect".

@fassadlr
Copy link
Collaborator

@rowandh can you update this PR with latest and then give an update on where we are with this one?

@rowandh rowandh changed the base branch from master to release/3.0.8.0 November 19, 2019 01:46
@zeptin zeptin closed this Nov 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RPC default binding not configured if username and password are empty
3 participants