Skip to content
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

Log host information from websocket requests #134

Merged
merged 25 commits into from
May 2, 2020
Merged

Log host information from websocket requests #134

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
82935ac
Add host information to log messages
jmjatlanta May 23, 2019
8b4ef5a
Fix bad_alloc
jmjatlanta May 23, 2019
d821e86
Add configurable header
jmjatlanta May 24, 2019
4906d8b
Allow configurable proxy header for logging
jmjatlanta May 24, 2019
0df5f7e
Use an api call instead of constructor
jmjatlanta May 24, 2019
94200c1
clarify method name, cache value
jmjatlanta May 27, 2019
99d7e63
revert adjustment to capture
jmjatlanta May 27, 2019
550e092
Merge branch 'master' into jmj_844b
oxarbitrage Jun 21, 2019
5d8e9e1
fix remove appender()
oxarbitrage Jun 21, 2019
e907429
fix tests
oxarbitrage Jun 21, 2019
7ec8ead
chanmge comment from hostname to IP
oxarbitrage Jun 21, 2019
e336649
lambda capture only whats required
oxarbitrage Jun 21, 2019
ba394e0
Refactor code about logging remote host info
abitmore May 1, 2020
9c506d0
Fix test cases
abitmore May 1, 2020
e50067d
Tweak code slightly
abitmore May 1, 2020
8ffe08f
Merge 'master' branch
abitmore May 1, 2020
e778cf1
Revert to simple WS connection for clients
abitmore May 1, 2020
8ddcc87
Add todo
abitmore May 1, 2020
c6df78b
Rename a function to avoid accidental misuse
abitmore May 2, 2020
3e07edf
Remove unused header inclusion
abitmore May 2, 2020
c8fcefb
Update RPC logging format
abitmore May 2, 2020
c58ae55
Update test cases about RPC logging level
abitmore May 2, 2020
ffbda4e
Add test cases about on_http for websocket servers
abitmore May 2, 2020
aa54e39
Remove unused websocket_tls_client class
abitmore May 2, 2020
ea96a5a
Fix websocket_client::secure_connect()
abitmore May 2, 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
6 changes: 5 additions & 1 deletion include/fc/network/http/websocket.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ namespace fc { namespace http {
boost::any& get_session_data() { return _session_data; }

virtual std::string get_request_header(const std::string& key) = 0;
virtual std::string get_remote_ip(const std::string& forward_header_key) = 0;

fc::signal<void()> closed;
private:
Expand All @@ -53,6 +54,7 @@ namespace fc { namespace http {
void listen( const fc::ip::endpoint& ep );
uint16_t get_listening_port();
void start_accept();
void set_forward_header_key(const std::string& key);

void stop_listening();
void close();
Expand All @@ -74,7 +76,7 @@ namespace fc { namespace http {
void listen( uint16_t port );
void listen( const fc::ip::endpoint& ep );
void start_accept();

void set_forward_header_key(const std::string& key);
private:
friend class detail::websocket_tls_server_impl;
std::unique_ptr<detail::websocket_tls_server_impl> my;
Expand All @@ -91,9 +93,11 @@ namespace fc { namespace http {

void close();
void synchronous_close();
void append_header(const std::string& key, const std::string& value);
private:
std::unique_ptr<detail::websocket_client_impl> my;
std::unique_ptr<detail::websocket_tls_client_impl> smy;
std::vector<std::pair<std::string,std::string>> headers;
};
class websocket_tls_client
{
Expand Down
7 changes: 7 additions & 0 deletions src/log/logger.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,13 @@ namespace fc {
void logger::add_appender( const appender::ptr& a )
{ my->_appenders.push_back(a); }

void logger::remove_appender( const appender::ptr& a )
{
auto item = std::find(my->_appenders.begin(), my->_appenders.end(), a);
if (item != my->_appenders.end())
my->_appenders.erase(item);
}

std::vector<appender::ptr> logger::get_appenders()const
{
return my->_appenders;
Expand Down
102 changes: 54 additions & 48 deletions src/network/http/websocket.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,6 @@ namespace fc { namespace http {
typedef base::con_msg_manager_type con_msg_manager_type;
typedef base::endpoint_msg_manager_type endpoint_msg_manager_type;

/// Custom Logging policies
/*typedef websocketpp::log::syslog<concurrency_type,
websocketpp::log::elevel> elog_type;
typedef websocketpp::log::syslog<concurrency_type,
websocketpp::log::alevel> alog_type;
*/
//typedef base::alog_type alog_type;
//typedef base::elog_type elog_type;
typedef websocketpp::log::stub elog_type;
typedef websocketpp::log::stub alog_type;

Expand Down Expand Up @@ -94,14 +86,6 @@ namespace fc { namespace http {
typedef base::con_msg_manager_type con_msg_manager_type;
typedef base::endpoint_msg_manager_type endpoint_msg_manager_type;

/// Custom Logging policies
/*typedef websocketpp::log::syslog<concurrency_type,
websocketpp::log::elevel> elog_type;
typedef websocketpp::log::syslog<concurrency_type,
websocketpp::log::alevel> alog_type;
*/
//typedef base::alog_type alog_type;
//typedef base::elog_type elog_type;
typedef websocketpp::log::stub elog_type;
typedef websocketpp::log::stub alog_type;

Expand Down Expand Up @@ -134,8 +118,6 @@ namespace fc { namespace http {
typedef base::con_msg_manager_type con_msg_manager_type;
typedef base::endpoint_msg_manager_type endpoint_msg_manager_type;

//typedef base::alog_type alog_type;
//typedef base::elog_type elog_type;
typedef websocketpp::log::stub elog_type;
typedef websocketpp::log::stub alog_type;

Expand All @@ -154,10 +136,6 @@ namespace fc { namespace http {
transport_type;
};





using websocketpp::connection_hdl;
typedef websocketpp::server<asio_with_stub_log> websocket_server_type;
typedef websocketpp::server<asio_tls_stub_log> websocket_tls_server_type;
Expand All @@ -177,7 +155,6 @@ namespace fc { namespace http {
virtual void send_message( const std::string& message )override
{
idump((message));
//std::cerr<<"send: "<<message<<"\n";
auto ec = _ws_connection->send( message );
FC_ASSERT( !ec, "websocket send failed: ${msg}", ("msg",ec.message() ) );
}
Expand All @@ -191,7 +168,30 @@ namespace fc { namespace http {
return _ws_connection->get_request_header(key);
}

/****
* @brief retrieves the remote IP address
*
* @param forward_header_key the key to look at in the request header
* @returns the value in the header, otherwise the remote endpoint
*/
virtual std::string get_remote_ip(const std::string& forward_header_key) override
{
if (last_ip.empty() || last_forward_header_key != forward_header_key)
{
// refresh the cache
last_forward_header_key = forward_header_key;
if (!forward_header_key.empty())
last_ip = get_request_header(forward_header_key);
if (last_ip.empty())
last_ip = _ws_connection->get_remote_endpoint();
}
return last_ip;
}

T _ws_connection;
// cache the value of the remote IP
std::string last_forward_header_key;
Copy link
Member

Choose a reason for hiding this comment

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

I didn't get why we need this. Will the key change after a while? Will the header change after the connection is established? Will the connection be reopened after closed? Will the object be reused for a new connection?

Copy link

@pmconrad pmconrad May 28, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

I agree this is not necessary. The header key should be set in the websocket_server before any connections are established, and it shouldn't change after that, and even if it is changed the change need not apply to existing connections. KISS, IMO.

Copy link
Member

Choose a reason for hiding this comment

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

Do we need to handle HTTP connections with connection: keep-alive header?

Copy link

Choose a reason for hiding this comment

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

No. http_handler always closes the connection after sending a response, and for a connection that has been upgraded to websocket protocol there will be no more http requests either.

Copy link
Member

Choose a reason for hiding this comment

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

So it means the connection: keep-alive header is ignored. Are we going to support it in the future?

Copy link

Choose a reason for hiding this comment

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

If I understand this correctly, the http_handler is called for every http request. So we create and close a websocket_connection for each request, not for each actual connection. (This also means I was wrong above - we don't close the http connection after sending the response, but only our own websocket_connection.)

So even in the presence of a keep-alive http connection, the code in its current form should retrieve the remote address separately for each request.

Copy link
Member

Choose a reason for hiding this comment

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

According to page 21 of RFC 6455, to establish a websocket connection, the request must include a connection header with the upgrade keyword inside, however, the RFC doesn't mention "keep-alive", so I think it's implementation-dependent.

The client's opening handshake consists of the following parts.
...
   3.   An |Upgrade| header field containing the value "websocket",
        treated as an ASCII case-insensitive value.

   4.   A |Connection| header field that includes the token "Upgrade",
        treated as an ASCII case-insensitive value.
...

When the connection is upgraded to a websocket connection, there is only one set of HTTP request headers attached to the connection, all following request messages must have the same original IP address (neither IP address nor forward-key would change), so it's best to get the address in set_open_handler() and use it in set_message_handler(), but not put all code in set_message_handler().

When the connection is a pure HTTP connection, there may be several requests received in the connection, we may probably need to handle connection: keep-alive header. If a new websocket_connection object is created for each request, then the header can be ignored. Anyway, we want to extract real IP addresses of the clients and log them, so we need to update set_http_handler().

std::string last_ip;
};

typedef websocketpp::lib::shared_ptr<boost::asio::ssl::context> context_ptr;
Expand All @@ -200,25 +200,26 @@ namespace fc { namespace http {
{
public:
websocket_server_impl()
:_server_thread( fc::thread::current() )
:_server_thread( fc::thread::current() ), fwd_header_key("")
{

_server.clear_access_channels( websocketpp::log::alevel::all );
_server.init_asio(&fc::asio::default_io_service());
_server.set_reuse_addr(true);
_server.set_open_handler( [&]( connection_hdl hdl ){
_server_thread.async( [&](){
auto new_con = std::make_shared<websocket_connection_impl<websocket_server_type::connection_ptr>>( _server.get_con_from_hdl(hdl) );
_on_connection( _connections[hdl] = new_con );
}).wait();
_server_thread.async( [this, hdl](){
auto new_con = std::make_shared<websocket_connection_impl<
websocket_server_type::connection_ptr>>( _server.get_con_from_hdl(hdl) );
_on_connection( _connections[hdl] = new_con );
}).wait();
});
_server.set_message_handler( [&]( connection_hdl hdl, websocket_server_type::message_ptr msg ){
_server_thread.async( [&](){
auto current_con = _connections.find(hdl);
assert( current_con != _connections.end() );
wdump(("server")(msg->get_payload()));
auto payload = msg->get_payload();
std::shared_ptr<websocket_connection> con = current_con->second;
wlog( "Websocket Server Remote: ${host} Payload: ${body}",
("host", con->get_remote_ip(fwd_header_key)) ("body", msg->get_payload()));
++_pending_messages;
auto f = fc::async([this,con,payload](){ if( _pending_messages ) --_pending_messages; con->on_message( payload ); });
if( _pending_messages > 100 )
Expand Down Expand Up @@ -302,6 +303,8 @@ namespace fc { namespace http {
if( _closed ) _closed->wait();
}

void set_forward_header_key(const std::string& key) { fwd_header_key = key; }

typedef std::map<connection_hdl, websocket_connection_ptr,std::owner_less<connection_hdl> > con_map;

con_map _connections;
Expand All @@ -310,6 +313,7 @@ namespace fc { namespace http {
on_connection_handler _on_connection;
fc::promise<void>::ptr _closed;
uint32_t _pending_messages = 0;
std::string fwd_header_key;
};

class websocket_tls_server_impl
Expand All @@ -318,7 +322,6 @@ namespace fc { namespace http {
websocket_tls_server_impl( const string& server_pem, const string& ssl_password )
:_server_thread( fc::thread::current() )
{
//if( server_pem.size() )
{
_server.set_tls_init_handler( [=]( websocketpp::connection_hdl hdl ) -> context_ptr {
context_ptr ctx = websocketpp::lib::make_shared<boost::asio::ssl::context>(boost::asio::ssl::context::tlsv1);
Expand Down Expand Up @@ -352,6 +355,8 @@ namespace fc { namespace http {
assert( current_con != _connections.end() );
auto received = msg->get_payload();
std::shared_ptr<websocket_connection> con = current_con->second;
wlog( "Websocket TLS Server Remote: ${host} Payload: ${body}",
("host", con->get_remote_ip(fwd_header_key)) ("body", msg->get_payload()));
fc::async([con,received](){ con->on_message( received ); });
}).wait();
});
Expand Down Expand Up @@ -398,6 +403,7 @@ namespace fc { namespace http {
}
});
}

~websocket_tls_server_impl()
{
if( _server.is_listening() )
Expand All @@ -407,27 +413,18 @@ namespace fc { namespace http {
_server.close( item.first, 0, "server exit" );
}

void set_forward_header_key( const std::string& key ) { fwd_header_key = key; }

typedef std::map<connection_hdl, websocket_connection_ptr,std::owner_less<connection_hdl> > con_map;

con_map _connections;
fc::thread& _server_thread;
websocket_tls_server_type _server;
on_connection_handler _on_connection;
fc::promise<void>::ptr _closed;
std::string fwd_header_key;
};













typedef websocketpp::client<asio_with_stub_log> websocket_client_type;
typedef websocketpp::client<asio_tls_stub_log> websocket_tls_client_type;

Expand All @@ -447,7 +444,6 @@ namespace fc { namespace http {
typename websocketpp::client<T>::message_ptr msg ){
_client_thread.async( [&](){
wdump((msg->get_payload()));
//std::cerr<<"recv: "<<msg->get_payload()<<"\n";
auto received = msg->get_payload();
fc::async( [=](){
if( _connection )
Expand Down Expand Up @@ -562,7 +558,8 @@ namespace fc { namespace http {

} // namespace detail

websocket_server::websocket_server():my( new detail::websocket_server_impl() ) {}
websocket_server::websocket_server()
:my( new detail::websocket_server_impl() ) {}
websocket_server::~websocket_server(){}

void websocket_server::on_connection( const on_connection_handler& handler )
Expand Down Expand Up @@ -600,7 +597,10 @@ namespace fc { namespace http {
my->_server.close(connection.first, websocketpp::close::status::normal, "Goodbye");
}


void websocket_server::set_forward_header_key( const std::string& key )
{
my->set_forward_header_key( key );
}

websocket_tls_server::websocket_tls_server( const string& server_pem, const string& ssl_password ):my( new detail::websocket_tls_server_impl(server_pem, ssl_password) ) {}
websocket_tls_server::~websocket_tls_server(){}
Expand Down Expand Up @@ -638,7 +638,6 @@ namespace fc { namespace http {
return secure_connect(uri);
FC_ASSERT( uri.substr(0,3) == "ws:" );

// wlog( "connecting to ${uri}", ("uri",uri));
websocketpp::lib::error_code ec;

my->_uri = uri;
Expand All @@ -654,6 +653,9 @@ namespace fc { namespace http {

auto con = my->_client.get_connection( uri, ec );

std::for_each(headers.begin(), headers.end(), [con](std::pair<std::string, std::string> in) {
con->append_header(in.first, in.second);
});
if( ec ) FC_ASSERT( !ec, "error: ${e}", ("e",ec.message()) );

my->_client.connect(con);
Expand All @@ -666,7 +668,6 @@ namespace fc { namespace http {
if( uri.substr(0,3) == "ws:" )
return connect(uri);
FC_ASSERT( uri.substr(0,4) == "wss:" );
// wlog( "connecting to ${uri}", ("uri",uri));
websocketpp::lib::error_code ec;

smy->_uri = uri;
Expand Down Expand Up @@ -700,6 +701,11 @@ namespace fc { namespace http {
my->_closed->wait();
}

void websocket_client::append_header(const std::string& key, const std::string& value)
{
headers.push_back( std::pair<std::string,std::string>(key, value));
}

websocket_connection_ptr websocket_tls_client::connect( const std::string& uri )
{ try {
// wlog( "connecting to ${uri}", ("uri",uri));
Expand Down
6 changes: 6 additions & 0 deletions tests/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,12 @@ target_link_libraries( hmac_test fc )
add_executable( ecc_test crypto/ecc_test.cpp )
target_link_libraries( ecc_test fc )

add_executable( ws_test_server ws_test_server.cpp )
target_link_libraries( ws_test_server fc )

add_executable( ws_test_client ws_test_client.cpp )
target_link_libraries( ws_test_client fc )

#add_executable( test_aes aes_test.cpp )
#target_link_libraries( test_aes fc ${rt_library} ${pthread_library} )
#add_executable( test_sleep sleep.cpp )
Expand Down
Loading