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

Jump to bottom

Smart pointer constructors for Secure Session #378

Merged
merged 4 commits into from
Feb 14, 2019
Merged

Smart pointer constructors for Secure Session #378

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cb91b59
Smart pointer constructors for Secure Session
ilammy Feb 13, 2019
d31862d
Merge branch 'master' into themispp-ownership
ilammy Feb 14, 2019
522decc
Check that Secure Session is unusable after being moved
ilammy Feb 14, 2019
558ed3e
Reword deprecation message
ilammy Feb 14, 2019
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
10 changes: 10 additions & 0 deletions src/soter/soter_error.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,16 @@ typedef int32_t soter_status_t;
#define UNUSED(x) (void)(x)
#endif

#ifndef DEPRECATED
#if __cplusplus >= 201402L
#define DEPRECATED(msg) [[deprecated(msg)]]
#elif defined(__GNUC__) || defined(__clang__)
#define DEPRECATED(msg) __attribute__((deprecated(msg)))
#else
#define DEPRECATED(msg)
#endif
#endif

/**@}*/

/**
Expand Down
51 changes: 38 additions & 13 deletions src/wrappers/themis/themispp/secure_session.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@
#define THEMISPP_SECURE_SESSION_HPP_

#include <cstring>
#if __cplusplus >= 201103L
#include <memory>
#endif
#include <vector>
#include <themis/themis.h>
#include "exception.hpp"
Expand Down Expand Up @@ -67,21 +70,14 @@ namespace themispp{
_callback(NULL){
}

#if __cplusplus >= 201103L
DEPRECATED("use std::shared_ptr variant to transfer callback ownership instead")
ilammy marked this conversation as resolved.
Show resolved Hide resolved
#endif
secure_session_t(const data_t& id, const data_t& priv_key, secure_session_callback_interface_t* callbacks):
_session(NULL),
_callback(NULL),
_res(0){
_callback=new secure_session_user_callbacks_t();
_callback->get_public_key_for_id=themispp::get_public_key_for_id_callback;
_callback->send_data=themispp::send_callback;
_callback->receive_data=themispp::receive_callback;
_callback->state_changed=NULL;
_callback->user_data=callbacks;
_session=secure_session_create(&id[0], id.size(), &priv_key[0], priv_key.size(), _callback);
if(!_session){
delete _callback;
throw themispp::exception_t("Secure Session failed creating");
}
initialize_session(id, priv_key, callbacks);
}

virtual ~secure_session_t(){
Expand All @@ -94,13 +90,22 @@ namespace themispp{
}

#if __cplusplus >= 201103L
secure_session_t(const data_t& id, const data_t& priv_key, std::shared_ptr<secure_session_callback_interface_t> &&callbacks):
_session(nullptr),
_callback(nullptr),
_res(0),
_interface(std::move(callbacks)){
initialize_session(id, priv_key, _interface.get());
}

secure_session_t(const secure_session_t&) = delete;
secure_session_t& operator=(const secure_session_t&) = delete;

secure_session_t(secure_session_t&& other){
_session=other._session;
_callback=other._callback;
_res=other._res;
_res=std::move(other._res);
_interface=std::move(other._interface);
other._session=nullptr;
other._callback=nullptr;
}
Expand All @@ -113,7 +118,8 @@ namespace themispp{
delete _callback;
_session=other._session;
_callback=other._callback;
_res=other._res;
_res=std::move(other._res);
_interface=std::move(other._interface);
other._session=nullptr;
other._callback=nullptr;
}
Expand Down Expand Up @@ -204,10 +210,29 @@ namespace themispp{
if(send_size<=0)
throw themispp::exception_t("Secure Session failed sending");
}

private:
void initialize_session(const data_t& id, const data_t& priv_key, secure_session_callback_interface_t* callbacks){
_callback=new secure_session_user_callbacks_t();
_callback->get_public_key_for_id=themispp::get_public_key_for_id_callback;
_callback->send_data=themispp::send_callback;
_callback->receive_data=themispp::receive_callback;
_callback->state_changed=NULL;
_callback->user_data=callbacks;
_session=secure_session_create(&id[0], id.size(), &priv_key[0], priv_key.size(), _callback);
if(!_session){
delete _callback;
throw themispp::exception_t("Secure Session failed creating");
}
}

private:
::secure_session_t* _session;
::secure_session_user_callbacks_t *_callback;
std::vector<uint8_t> _res;
#if __cplusplus >= 201103L
std::shared_ptr<secure_session_callback_interface_t> _interface;
#endif
};
}// ns themis

Expand Down
48 changes: 48 additions & 0 deletions tests/themispp/secure_session_test.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,12 @@
#include <unordered_map>
#endif

// Allow usage of non-owning Secure Session constructor for testing
#if defined(__GNUC__) || defined(__clang__)
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
#endif

namespace themispp{
namespace secure_session_test{

Expand Down Expand Up @@ -113,19 +119,61 @@ namespace themispp{

sput_fail_if(client.is_established(), "using moved session again", __LINE__);
}

void secure_session_ownership(){
std::string client_id("client");

auto client_callbacks_shared = std::make_shared<callback>();
auto client_callbacks_unique = std::unique_ptr<callback>(new callback());

themispp::secure_session_t client_shared(std::vector<uint8_t>(client_id.c_str(), client_id.c_str()+client_id.length()), std::vector<uint8_t>(client_priv, client_priv+sizeof(client_priv)), std::move(client_callbacks_shared));
themispp::secure_session_t client_unique(std::vector<uint8_t>(client_id.c_str(), client_id.c_str()+client_id.length()), std::vector<uint8_t>(client_priv, client_priv+sizeof(client_priv)), std::move(client_callbacks_unique));

themispp::secure_session_t client_shared_moved = std::move(client_shared);
themispp::secure_session_t client_unique_moved = std::move(client_unique);

sput_fail_if(client_shared_moved.is_established(), "using shared session", __LINE__);
sput_fail_if(client_unique_moved.is_established(), "using unique session", __LINE__);
Copy link
Collaborator

Choose a reason for hiding this comment

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

will be good to check that client_unique is invalid after move. check bad case too)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done. I've added a check that secure session throws after being moved out. (This is true for client_shared as well. Ownership over the transport does not matter here.)


bool shared_throws=false;
bool unique_throws=false;
try{
client_shared.is_established();
}
catch(const themispp::exception_t&){
shared_throws=true;
}
try{
client_unique.is_established();
}
catch(const themispp::exception_t&){
unique_throws=true;
}
sput_fail_unless(shared_throws, "using shared session after move", __LINE__);
sput_fail_unless(unique_throws, "using unique session after move", __LINE__);
}
#else
void secure_session_moved(){
sput_fail_if(false, "move semantics (disabled for C++03)", __LINE__);
}

void secure_session_ownership(){
sput_fail_if(false, "ownership transfer (disabled for C++03)", __LINE__);
}
#endif

int run_secure_session_test(){
sput_enter_suite("ThemisPP secure session test");
sput_run_test(secure_session_test, "secure_session_test", __FILE__);
sput_run_test(secure_session_uninitialized, "secure_session_uninitialized", __FILE__);
sput_run_test(secure_session_moved, "secure_session_moved", __FILE__);
sput_run_test(secure_session_ownership, "secure_session_ownership", __FILE__);
return 0;
}
}
}
#endif

#if defined(__GNUC__) || defined(__clang__)
#pragma GCC diagnostic pop
#endif