From 3e26224d86e24161e9dedb3671e287564615c9e4 Mon Sep 17 00:00:00 2001 From: Roker Date: Mon, 6 Jul 2020 13:04:35 +0200 Subject: [PATCH 2/7] update README.md --- README.md | 63 ++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 39 insertions(+), 24 deletions(-) diff --git a/README.md b/README.md index 3575e62..6b4874c 100644 --- a/README.md +++ b/README.md @@ -189,22 +189,44 @@ the adapter and its functions. The JSON Server Adapter can be started on demand. It checks automatically whether an instance for the same user on the machine -is already running and if yes it ends itself gracefully. +is already running and if yes it ends itself gracefully. (TODO!) If there is no running server found the newly started server creates the server token file and forks itself into background (if not prevented via "-d" commandline switch). +### Multi-Client handling -### Session handling +The p≡p JSON server adapter supports multiple clients, communicating with the +server at the same time. Each client instance is identified by a client ID, +that the clients put into each JSON RPC request in the field "clientid". -When using the p≡p engine, a session is needed to which any adapter can -connect. The p≡p JSON Server Adapter automatically creates one session per +The client ID is a UUID Version 4, created by the client at startup and has to +be stable while the client application runs. When the client restarts, a new +client ID should be created to avoid interferene with data from the old client +session. + +The p≡p JSON server adapter stores data (e.g., a so called "config cache", see +next section) associated with each client ID. After a timeout period with no +JSON RPC calls and no open client connections these data are removed +automatically. Run the mini adapter with -h to see the compiled-in default +timeout value. + +### PEP_SESSION handling + +When using the p≡p engine, a `PEP_SESSION` is needed as parameter to many API +functions. The p≡p JSON Server Adapter automatically creates one session per HTTP client connection (and also closes that session automatically when the client connections is closed). Therefore, the client does not need to take -care of the session management. However, the client has to set up a [HTTP +care of the session management. However, the client should set up a [HTTP persistent -connection](https://en.wikipedia.org/wiki/HTTP_persistent_connection). +connection](https://en.wikipedia.org/wiki/HTTP_persistent_connection) to +minify session creation and destruction. + +There is a configuration cache, that stores all `config_*()` calls and its +configured values. Whenever a new PEP_SESSION is needed for this client +(identified via its client ID, see previous section), all config values +are applied to this new session, too, before the session is used. ### API Principles @@ -268,13 +290,16 @@ a certain C function). An complete overview with all functions that are callable from the client can be found in the [API Reference](pEp JSON Server Adapter/API Reference). -That API reference is a generated file that shows the current API briefly. +That API reference is a generated file (at irregular intervals) that shows the current API briefly. There is also a (currently manually written) file that holts a copy of the documentation from the Engine's header files: [API reference detail.md] +BEWARE: Because this file is not auto-generated, yet, it might be even more outdated! + Most of the callable functions are functions from the C API of the p≡p Engine. They are described in detail, incl. pre- and post-conditions in -the appropriate C header files of the Engine. +the appropriate C header files of the Engine, which are the authoritative source +of documentation in cases of doubt. ### Authentication @@ -301,7 +326,7 @@ file that has user-only read permissions. token". -### Callbacks / Reverse connection +### Callbacks / Event delivery p≡p applications must register callback handlers at the Engine. At the moment there are these callbacks: @@ -312,23 +337,13 @@ there are these callbacks: The JSON adapter register its own functions at the Engine which propagate these events to all connected clients. -The event propagation to the clients are also done via JSON RPC calls. Here -the JSON Adapter acts as "client" which connects to a host:port that was told -to by the Client to the JSON Adapter via `registerEventListener()` call, where -Client also sends a security token to the JSON Adapter for the reverse JSON RPC -connection. - -Note: It is planned to change the way how events are sent to the Client, when -the Client is unable to open listen sockets (it is rumored that future versions -of Mozilla Thunderbird's plugin API will no longer allow this). +The event propagation to the clients are done via long polling: Clients +call the function `pollForEvents()` that blocks until an event +from the Engine arrives. TODO: remove create_session(), use client ID instead? -Idea 1: Use long polling: The Client calls a function that blocks until an event -from the Engine arrives. This approach requires only a few changes in the JSON Adapter. -Idea 2: Use WebSockets: In fact this is also a type of "long polling" and an open -TCP connection, opened by the Client. But it requires additional code in the JSON -Adapter for the WebSockets protocol and a mechanism how to transfer the underlaying -TCP socket and buffer from the libevent library to the WebSockets implementation. +It is planned to switch to use WebSockets: In fact this is also a type of +"long polling" and an open TCP connection, opened by the Client. See: https://pep.foundation/jira/browse/JSON-128 From 7f231095447c9714c7a1ae825636e7c9b1fff539 Mon Sep 17 00:00:00 2001 From: Roker Date: Thu, 20 Aug 2020 17:41:36 +0200 Subject: [PATCH 3/7] remove Context::augment() because it is no longer necessary. --- server/context.hh | 1 - server/function_map.hh | 1 - server/json-adapter.cc | 7 ------- server/json-adapter.hh | 6 ++---- server/unittest_rpc.cc | 1 - 5 files changed, 2 insertions(+), 14 deletions(-) diff --git a/server/context.hh b/server/context.hh index ebb6953..88d99de 100644 --- a/server/context.hh +++ b/server/context.hh @@ -12,7 +12,6 @@ public: virtual ~Context() = default; virtual bool verify_security_token(const std::string& token) const = 0; - virtual void augment(json_spirit::Object& returnObject) = 0; // Cache a certain function call. See JSON-155. virtual void cache(const std::string& func_name, const std::function& fn) = 0; diff --git a/server/function_map.hh b/server/function_map.hh index c32366f..4d5858d 100644 --- a/server/function_map.hh +++ b/server/function_map.hh @@ -209,7 +209,6 @@ public: rs.emplace_back("outParams", std::move(out_params)); rs.emplace_back("return", std::move(ret)); - context->augment(rs); // used e.g. add some debug infos to the status return value context->clear(); // clear all stored values, if any. return rs; diff --git a/server/json-adapter.cc b/server/json-adapter.cc index 3f7b825..b2b4db6 100644 --- a/server/json-adapter.cc +++ b/server/json-adapter.cc @@ -341,13 +341,6 @@ bool JsonAdapter::verify_security_token(const std::string& s) const } -void JsonAdapter::augment(json_spirit::Object& returnObject) -{ - check_guard(); - // nothing to do anymore. -} - - void JsonAdapter::cache(const std::string& fn_name, const std::function& func) { i->session_registry->add_to_cache(fn_name, func); diff --git a/server/json-adapter.hh b/server/json-adapter.hh index 32a8b1b..b574bc1 100644 --- a/server/json-adapter.hh +++ b/server/json-adapter.hh @@ -75,8 +75,6 @@ public: // returns 'true' if 's' is the security token created by the function above. virtual bool verify_security_token(const std::string& s) const override; - virtual void augment(json_spirit::Object& returnObject) override; - virtual void cache(const std::string& fn_name, const std::function& func) override; // returns the version of the JsonAdapter @@ -96,8 +94,8 @@ public: static JsonAdapter& getInstance(); - static SessionRegistry& getSessionRegistry(); - + static SessionRegistry& getSessionRegistry(); + JsonAdapter& startup(::messageToSend_t messageToSend); protected: diff --git a/server/unittest_rpc.cc b/server/unittest_rpc.cc index 013e8ac..dd204dd 100644 --- a/server/unittest_rpc.cc +++ b/server/unittest_rpc.cc @@ -28,7 +28,6 @@ class DummyContext : public Context { public: virtual bool verify_security_token(const std::string& token) const override { return true; } - virtual void augment(js::Object&) override { /* do nothing */ } virtual void cache(const std::string& func_name, const std::function& fn) override { From b687c4d5312f045bf6db69a085e29490e0a24de8 Mon Sep 17 00:00:00 2001 From: Roker Date: Thu, 20 Aug 2020 17:41:56 +0200 Subject: [PATCH 4/7] fetch client_id from JSON-RPC request. --- server/json_rpc.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/server/json_rpc.cc b/server/json_rpc.cc index 529fa18..3703e19 100644 --- a/server/json_rpc.cc +++ b/server/json_rpc.cc @@ -91,6 +91,9 @@ js::Object call(const FunctionMap& fm, const js::Object& request, Context* conte return make_error(JSON_RPC::INVALID_REQUEST, "Invalid request: Wrong security token.", request, request_id); } + const auto client_id = find_value(request, "client_id"); + const std::string client_id_s = (client_id.type()==js::str_type ? client_id.get_str() : std::string() ); // missing or non-string "client_id" --> empty string. + const auto method = find_value(request, "method"); if(method.type()!=js::str_type) { From f7043856c3b537915c318d8c961573df02772bff Mon Sep 17 00:00:00 2001 From: Roker Date: Tue, 25 Aug 2020 12:44:45 +0200 Subject: [PATCH 5/7] compare security token via constant_time_equal() to avoid timing attacks on the security token. NEEDS NEW LIBPEPADAPTER! --- server/json-adapter.cc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/server/json-adapter.cc b/server/json-adapter.cc index b2b4db6..75c3dd2 100644 --- a/server/json-adapter.cc +++ b/server/json-adapter.cc @@ -26,6 +26,7 @@ #include #include +#include #include // from libpEpAdapter. #include @@ -333,11 +334,12 @@ void JsonAdapter::shutdown(timeval* t) bool JsonAdapter::verify_security_token(const std::string& s) const { check_guard(); - if(s!=i->token) + const bool eq = pEp::constant_time_equal(s, i->token); + if( eq==false ) { Log(Logger::Notice) << "sec_token=\"" << i->token << "\" (len=" << i->token.size() << ") is unequal to \"" << s << "\" (len=" << s.size() << ")!"; } - return s == i->token; + return eq; } From 9b3a0bd4ffe129969ec3eec6c091c39de268fa0e Mon Sep 17 00:00:00 2001 From: Roker Date: Tue, 25 Aug 2020 12:45:48 +0200 Subject: [PATCH 6/7] restructure call context, so cached/generated values are now thread-safe "per call" and not globally stored in the JsonAdapter class! --- server/context.cc | 21 +++++++++++++++------ server/context.hh | 23 +++++++++++++++-------- server/function_map.hh | 2 -- server/json-adapter.hh | 16 ++++++++++++++-- server/json_rpc.cc | 8 +++++--- server/json_rpc.hh | 5 +++-- server/unittest_rpc.cc | 7 ++++--- 7 files changed, 56 insertions(+), 26 deletions(-) diff --git a/server/context.cc b/server/context.cc index 1e43d76..41514f0 100644 --- a/server/context.cc +++ b/server/context.cc @@ -1,5 +1,7 @@ #include "context.hh" #include "logger.hh" +#include "json-adapter.hh" + namespace { @@ -11,6 +13,19 @@ namespace } +bool Context::verify_security_token(const std::string& token) const +{ + return ja->verify_security_token(token); +} + + +// Cache a certain function call. See JSON-155. +void Context::cache(const std::string& func_name, const std::function& fn) +{ + ja->cache(func_name, fn); +} + + void Context::store(int position, size_t value) { DEBUG_OUT( Log(), "Store value %zu for position %d.", value, position); @@ -29,9 +44,3 @@ size_t Context::retrieve(int position) throw; } } - - -void Context::clear() -{ - obj_store.clear(); -} diff --git a/server/context.hh b/server/context.hh index 88d99de..a9bd3c6 100644 --- a/server/context.hh +++ b/server/context.hh @@ -1,30 +1,37 @@ #ifndef JSON_ADAPTER_CONTEXT_HH #define JSON_ADAPTER_CONTEXT_HH -#include -#include "json_spirit/json_spirit_value.h" #include #include + +class JsonAdapterBase; + class Context { public: - virtual ~Context() = default; + Context(JsonAdapterBase* _ja) : ja{_ja} {} + + Context(const Context&) = delete; + void operator=(const Context&) = delete; - virtual bool verify_security_token(const std::string& token) const = 0; + // delegate call to the 'ja' member + bool verify_security_token(const std::string& token) const ; // Cache a certain function call. See JSON-155. - virtual void cache(const std::string& func_name, const std::function& fn) = 0; + // delegate call to the 'ja' member + void cache(const std::string& func_name, const std::function& fn); // store and retrieve other parameters into the context. // that allows semantic actions based on other function parameters // KISS: at the moment only "size_t" objects are supported. - virtual void store(int position, size_t value); - virtual size_t retrieve(int position); - virtual void clear(); + void store(int position, size_t value); + size_t retrieve(int position); + void clear(); private: std::map obj_store; + JsonAdapterBase* ja; }; #endif // JSON_ADAPTER_CONTEXT_HH diff --git a/server/function_map.hh b/server/function_map.hh index 4d5858d..a30f82f 100644 --- a/server/function_map.hh +++ b/server/function_map.hh @@ -209,8 +209,6 @@ public: rs.emplace_back("outParams", std::move(out_params)); rs.emplace_back("return", std::move(ret)); - context->clear(); // clear all stored values, if any. - return rs; } diff --git a/server/json-adapter.hh b/server/json-adapter.hh index b574bc1..acfe3a3 100644 --- a/server/json-adapter.hh +++ b/server/json-adapter.hh @@ -5,15 +5,27 @@ #include #include #include "registry.hh" -#include "context.hh" #include "logger.hh" #include "server_version.hh" +#include "json_spirit/json_spirit_value.h" class SessionRegistry; -class JsonAdapter : public Context +// allow mocking the JsonAdapter in unittest_rpc +class JsonAdapterBase +{ +public: + ~JsonAdapterBase() = default; + virtual bool verify_security_token(const std::string& s) const = 0; + + // Cache a certain function call. See JSON-155. + virtual void cache(const std::string& fn_name, const std::function& func) = 0; +}; + + +class JsonAdapter : public JsonAdapterBase { public: diff --git a/server/json_rpc.cc b/server/json_rpc.cc index 3703e19..4eeaecc 100644 --- a/server/json_rpc.cc +++ b/server/json_rpc.cc @@ -1,3 +1,4 @@ +#include "context.hh" #include "json_rpc.hh" #include "json_spirit/json_spirit_utils.h" #include "json_spirit/json_spirit_writer.h" @@ -72,7 +73,7 @@ js::Object make_request(const std::string& functionName, const js::Array& parame using json_spirit::find_value; -js::Object call(const FunctionMap& fm, const js::Object& request, Context* context) +js::Object call(const FunctionMap& fm, const js::Object& request, JsonAdapterBase* ja) { Logger L("jrpc:call"); int request_id = -1; @@ -86,7 +87,7 @@ js::Object call(const FunctionMap& fm, const js::Object& request, Context* conte const auto sec_token = find_value(request, "security_token"); const std::string sec_token_s = (sec_token.type()==js::str_type ? sec_token.get_str() : std::string() ); // missing or non-string "security_token" --> empty string. - if( context->verify_security_token(sec_token_s)==false ) + if( ja->verify_security_token(sec_token_s)==false ) { return make_error(JSON_RPC::INVALID_REQUEST, "Invalid request: Wrong security token.", request, request_id); } @@ -130,7 +131,8 @@ js::Object call(const FunctionMap& fm, const js::Object& request, Context* conte DEBUG_OUT(L, "method_name=\"" + method_name + "\"\n" "params=" + js::write(params) ); - const js::Value result = fn->second->call(p, context); + Context context{ja}; + const js::Value result = fn->second->call(p, &context); DEBUG_OUT(L, "result=" + js::write(result, js::raw_utf8) ); return make_result(result, request_id); diff --git a/server/json_rpc.hh b/server/json_rpc.hh index 0fdf8a7..9d5cb7b 100644 --- a/server/json_rpc.hh +++ b/server/json_rpc.hh @@ -2,7 +2,6 @@ #define JSON_RPC_HH #include "json_spirit/json_spirit_value.h" -#include "context.hh" #include "function_map.hh" namespace js = json_spirit; @@ -17,11 +16,13 @@ enum class JSON_RPC INTERNAL_ERROR = -32603, }; +class JsonAdapterBase; + // Server side: // parse the JSON-RPC 2.0 compatible "request", call the C function // and create an appropiate "response" object (containing a result or an error) -js::Object call(const FunctionMap& fm, const js::Object& request, Context* context); +js::Object call(const FunctionMap& fm, const js::Object& request, JsonAdapterBase* ja); // create a JSON-RPC 2.0 compatible result response object //js::Object make_result(const js::Value& result, int id); diff --git a/server/unittest_rpc.cc b/server/unittest_rpc.cc index dd204dd..9df517e 100644 --- a/server/unittest_rpc.cc +++ b/server/unittest_rpc.cc @@ -1,5 +1,6 @@ #include #include "json_rpc.hh" +#include "json-adapter.hh" #include "function_map.hh" #include "c_string.hh" #include "json_spirit/json_spirit_reader.h" @@ -24,7 +25,7 @@ std::ostream& operator<<(std::ostream& os, const Value& value) namespace { -class DummyContext : public Context +class DummyAdapter : public JsonAdapterBase { public: virtual bool verify_security_token(const std::string& token) const override { return true; } @@ -36,7 +37,7 @@ public: }; -DummyContext dummyContext; +DummyAdapter dummyAdapter; // some example & test functions: @@ -162,7 +163,7 @@ TEST_P( RpcTest, Meh ) js::read_or_throw(v.result, expected_result); auto r = request; - const js::Value actual_result = call( test_functions, request.get_obj(), &dummyContext); + const js::Value actual_result = call( test_functions, request.get_obj(), &dummyAdapter); js::Object result_obj = actual_result.get_obj(); js::Object::iterator q = std::find_if(result_obj.begin(), result_obj.end(), [](const js::Pair& v){ return js::Config::get_name(v) == "thread_id"; } ); result_obj.erase( q ); From 3f9a4c93b0853e2a5019a930010752aabdf7ba0c Mon Sep 17 00:00:00 2001 From: Roker Date: Wed, 26 Aug 2020 10:04:19 +0200 Subject: [PATCH 7/7] new version name "(45) Kreuz Erfurt" to release the bugfix for context-saved function parameters. --- server/server_version.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/server_version.cc b/server/server_version.cc index ca0af92..d215d3c 100644 --- a/server/server_version.cc +++ b/server/server_version.cc @@ -75,7 +75,8 @@ static const std::string VersionName = // 41a,b were skipped, intentionally // "(42) Gotha"; // JSON-152: 2-parameter version of pollForEvents(). // "(43) Wandersleben"; // JSON-153 passphrase support. *sigh* - "(44) Neudietendorf"; // replace my own sync thread code by libpEpAdapter's implementation. +// "(44) Neudietendorf"; // replace my own sync thread code by libpEpAdapter's implementation. + "(45) Kreuz Erfurt"; // fix of context-saved function parameters that would cause trouble when >1 request is processed in parallel. } // end of anonymous namespace ////////////////////////////////////////////////////////////////////////////