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

Add UNIX socket HTTP server to keosd #5425

Merged
merged 10 commits into from
Sep 1, 2018
Merged

Conversation

spoonincode
Copy link
Contributor

The http_plugin now has the capability to serve its HTTP RPC on a UNIX domain socket in addition to the TCP and TLS over TCP sockets already supported. This option is currently only present in keosd even though nodeos also uses the http_plugin.

User facing changes for keosd

  • A new option unix-socket-path configures where to create the unix socket. By default this will be “keosd.sock” and created within keosd's data-dir which by default is ~/eosio.wallet/ . So the socket would reside at ~/eosio.wallet/keosd.sock.
    You can also use an absolute path for unix-socket-path if you want to place it more globally accessible. This may be useful for more advanced deployments where, for example, nodeos and keosd run as different users but are members of the same group. The socket has mode 0660 so only the user who executed keosd as well as members of the group of the user who executed keosd can open it.
    If you wish to disable the unix socket, you can use either unix-socket-path = (in the config file) or --unix-socket-path '' (on the command line) to set an empty path which disables it. If you desire to only use the unix socket, you can similarly set the http-server-address to empty which will disable it.
  • The default HTTP port for keosd has changed to 8900. This resolves an inconstancy where when you launched keosd manually with no options it would listen on 8888, but if you had cleos launch keosd for you it would listen on 8900. Be aware that this default port change won’t take affect for current users — your config.ini will still contain the original 8888 default which will be treated as user specified and thus overrule the new default.

Using the socket with cleos

By default cleos will still try to access the wallet on http://127.0.0.1:8900. To have it access via UNIX socket pass something like --wallet-url unix://${HOME}/eosio-wallet/keosd.sock

Using the socket with nodeos producer_plugin

If you would like to use keosd via UNIX socket as a signature provider the syntax is of the form
signature-provider = EOS6MRyAjQq8ud7hVNYcfnVPJqcVpscN5So8BhtHuGYqET5GDW5CV=KEOSD:unix:///home/username/eosio-wallet/keosd.sock/v1/wallet/sign_digest
Nodeos will discover that /home/username/eosio-wallet/keosd.sock is the path to the socket, and /v1/wallet/sign_digest is the URL path.

Implementation/PR notes

I had to pretty much copy pasta the asio_endpoint.hpp code from websocketpp in to a new local_endpoint.hpp and change a few items. It seemed like tcp was baked deep in to how the original code worked.

Add the ablity to run a unix socket HTTP server in http_plugin. The local_endpoint.hpp is a bit of a copy paste job from the asio endpoint in websocketpp. It may be possible to strip this down further with more investigation. unix socket server is currently disabled pending proper configuration items for it
Also add customization method for defaults based on nodeos vs keosd
Regress a few changes from this branch until unix socket support is fully embraced —
* Maintain current keosd *-server-address config names
* Have keosd start HTTP server by default still (though this time on 8900)
* Change cleos default URL back to http://127.0.0.1:8900
@@ -148,6 +149,22 @@ FC_DECLARE_EXCEPTION( localized_exception, 10000000, "an error occured" );
FC_MULTILINE_MACRO_END \
)

//copy pasta from keosd's main.cpp
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the changes in this file are left over cruft from when unix socket was the default; probably could remove all this

if(current_http_plugin_defaults.default_http_port)
cfg.add_options()
(my->http_server_address_option_name.c_str(), bpo::value<string>()->default_value("127.0.0.1:" + std::to_string(current_http_plugin_defaults.default_http_port)),
"The local IP and port to listen for incoming http connections; set blank to disable.");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leaving blank doesn't actually seem to be possible. It generates a parse error.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That works for me.

$ programs/nodeos/nodeos 2>&1 | grep listen
2018-08-29T23:42:50.438 thread-0   http_plugin.cpp:344           plugin_initialize    ] configured http to listen on 127.0.0.1:8888
2018-08-29T23:42:50.440 thread-0   http_plugin.cpp:401           plugin_startup       ] start listening for http requests
2018-08-29T23:42:50.440 thread-0   net_plugin.cpp:3010           plugin_startup       ] starting listener, max clients is 25
$ programs/nodeos/nodeos --http-server-address '' 2>&1 | grep listen
2018-08-29T23:43:46.528 thread-0   net_plugin.cpp:3010           plugin_startup       ] starting listener, max clients is 25

I also double checked and the config.ini works being blank too.

}

std::string get_remote_endpoint(lib::error_code & ec) const {
return "fixme";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If not implemented, set an error code in ec?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The iostream transport implementation just returns a static "iostream transport" response w/o setting error. I'll change this to something other than fixme,

my->mangle_option_names();
if(current_http_plugin_defaults.default_unix_socket_path.length())
cfg.add_options()
(my->unix_socket_path_option_name.c_str(), bpo::value<boost::filesystem::path>()->default_value(current_http_plugin_defaults.default_unix_socket_path),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs to be std::string with manual parsing into bfs path, or paths with spaces are spuriously rejected. (Ancient boost defect.)

boost::filesystem::path sock_path = options.at(my->unix_socket_path_option_name).as<boost::filesystem::path>();
if (sock_path.is_relative())
sock_path = app().data_dir() / sock_path;
my->unix_endpoint = asio::local::stream_protocol::endpoint(sock_path.string());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does boost do something sane with permissions on the socket file?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It just will get the default via whatever current umask is. You'll find in the local_endpoint.hpp file I monkey with the umask to ensure the file is created 0660

m_acceptor->open(ep.protocol(),bec);
}
if (!bec) {
mode_t old_mask = umask(S_IXUSR|S_IXGRP|S_IRWXO);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this honor an existing more restrictive umask? In particular 077.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's a valid point

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am thinking now, maybe should not be touching this at all

This function is never called, but return something less sloppy
Workaround quirk when filenames have spaces
Just leave this up to the environment
m_acceptor->close();
}
log_err(log::elevel::info,"asio listen",bec);
ec = bec;//make_error_code(error::pass_through);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commented code.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks; bad copy pasta is bad

@spoonincode spoonincode merged commit ad33a01 into develop Sep 1, 2018
@spoonincode spoonincode deleted the unix_socket_wallet branch September 1, 2018 00:28
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.

None yet

2 participants