Skip to content

Commit

Permalink
Update JsThemis tests (#620)
Browse files Browse the repository at this point in the history
* Secure Cell: reuse WasmThemis tests

Update Secure Cell test suite with the tests used in WasmThemis. There
are some minor tweaks here and there to make them compatible, but it's
all the same tests otherwise.

* Secure Cell: report TypeError correctly

Instead of throwing a generic error with INVALID_PARAMETER status code,
throw a proper TypeError when we notice an issue with input types.

This commit updates all cryptosystems, not only Secure Cell.

* Secure Cell: allow "null" for context values

In addition to allowing empty buffers and omitted arguments, accept
explicit "null" values when the context is not used. This improves
API compatibility with WasmThemis.

* Secure Message: reuse WasmThemis tests

In JsThemis there is no SecureMessageSign and SecureMessageVerify
objects, empty keys should be used instead.

* Secure Message: allow "null" keys for sign/verify

Allow omitted keys to be specified with "null" values in addition to
empty buffers. This improves compatibility with WasmThemis API and is
consistent with other places where we allow optional values.

* Secure Message: disallow both empty keys

Check that the user has provided either public or private key, do not
allow both keys to be omitted.

* Secure Comparator: reuse WasmThemis tests

Note that JsThemis uses a bit different naming and does not have a
"destroy()" method. The Secure Comparator object is freed when Node.js
runtime collects the garbage.

* Secure Session: reuse WasmThemis tests

API is slightly different in JsThemis and WasmThemis, the names are not
the same and JsThemis does not have a dedicated negotiation method,
using just unwrap() for that.

* Secure Session: check private key validity

WasmThemis verifies the private key during Secure Session construction.
Don't defer this to usage in JsThemis either.

* Secure Session: handle exceptions in callback

We should check the call result as it may be empty if the callback
throws an exception. Failure to check for this leads to process abort:

    FATAL ERROR: v8::ToLocalChecked Empty MaybeLocal.
     1: 0x8fa090 node::Abort() [node]
     2: 0x8fa0dc  [node]
     3: 0xb0030a v8::Utils::ReportApiFailure(char const*, char const*) [node]
     4: 0x7ff9182b1785 jsthemis::SecureSession::get_public_key_for_id_callback(void const*, unsigned long, void*, unsigned long, void*)
     [...]
     9: 0xb8e96f  [node]
    10: 0xb8f4d9 v8::internal::Builtin_HandleApiCall(int, v8::internal::Object**, v8::internal::Isolate*) [node]
    11: 0x1762fd45be1d
    Aborted (core dumped)

We don't want this to happen.

* Retroactively apply copyright to tests

Since we're publishing test code on npm now, we should properly
attribute this file. The tests were added in 944b9e0 ("Add JSTHEMIS
test", 2016-01-17).

* Simplify sign/verify test case

We know that JsThemis returns results as instances of Node.js Buffer so
we can verify that signed message includes the plaintext by using the
"includes" method. We don't need to write it ourselves.

* Reformat code with "make fmt"
  • Loading branch information
ilammy committed Apr 9, 2020
1 parent e3a90c1 commit 49d8389
Show file tree
Hide file tree
Showing 11 changed files with 849 additions and 392 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,9 @@ _Code:_

- New class `SymmetricKey` can be used to generate symmetric keys for Secure Cell ([#562](https://github.com/cossacklabs/themis/pull/562)).
- New makefile target `make jsthemis` can be used to build JsThemis from source ([#618](https://github.com/cossacklabs/themis/pull/618)).
- `SecureCell` now allows `null` to explicitly specify omitted encryption context ([#620](https://github.com/cossacklabs/themis/pull/620)).
- `SecureMessage` now allows `null` for omitted keys in sign/verify mode ([#620](https://github.com/cossacklabs/themis/pull/620)).
- Fixed a crash when an exception is thrown from `SecureSession` callback ([#620](https://github.com/cossacklabs/themis/pull/620)).

- **PHP**

Expand Down
10 changes: 10 additions & 0 deletions src/wrappers/themis/jsthemis/errors.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,16 @@ void ThrowError(const char* domain, themis_status_t status)
Nan::ThrowError(WithStatus(Nan::Error(message.c_str()), status));
}

void ThrowTypeError(const char* domain, const char* description)
{
std::string message;
message += "themis.";
message += domain;
message += ": ";
message += description;
Nan::ThrowError(Nan::TypeError(message.c_str()));
}

void ThrowParameterError(const char* domain, const char* description)
{
std::string message;
Expand Down
2 changes: 2 additions & 0 deletions src/wrappers/themis/jsthemis/errors.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ void Init(v8::Local<v8::Object> exports);

void ThrowError(const char* domain, themis_status_t status);

void ThrowTypeError(const char* domain, const char* description);

void ThrowParameterError(const char* domain, const char* description);

void ThrowSecureSessionError(const char* domain, themis_status_t status);
Expand Down
19 changes: 9 additions & 10 deletions src/wrappers/themis/jsthemis/secure_cell_context_imprint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,7 @@ void SecureCellContextImprint::New(const Nan::FunctionCallbackInfo<v8::Value>& a
return;
}
if (!args[0]->IsUint8Array()) {
ThrowParameterError("Secure Cell (Context Imprint) constructor",
"master key is not a byte buffer");
ThrowTypeError("SecureCellContextImprint", "master key is not a byte buffer");
args.GetReturnValue().SetUndefined();
return;
}
Expand Down Expand Up @@ -96,8 +95,8 @@ void SecureCellContextImprint::encrypt(const Nan::FunctionCallbackInfo<v8::Value
return;
}
if (!args[0]->IsUint8Array()) {
ThrowParameterError("Secure Cell (Context Imprint) failed to encrypt",
"message is not a byte buffer, use ByteBuffer or Uint8Array");
ThrowTypeError("SecureCellContextImprint",
"message is not a byte buffer, use ByteBuffer or Uint8Array");
args.GetReturnValue().SetUndefined();
return;
}
Expand All @@ -107,8 +106,8 @@ void SecureCellContextImprint::encrypt(const Nan::FunctionCallbackInfo<v8::Value
return;
}
if (!args[1]->IsUint8Array()) {
ThrowParameterError("Secure Cell (Context Imprint) failed to encrypt",
"context is not a byte buffer, use ByteBuffer or Uint8Array");
ThrowTypeError("SecureCellContextImprint",
"context is not a byte buffer, use ByteBuffer or Uint8Array");
args.GetReturnValue().SetUndefined();
return;
}
Expand Down Expand Up @@ -162,8 +161,8 @@ void SecureCellContextImprint::decrypt(const Nan::FunctionCallbackInfo<v8::Value
return;
}
if (!args[0]->IsUint8Array()) {
ThrowParameterError("Secure Cell (Context Imprint) failed to decrypt",
"message is not a byte buffer, use ByteBuffer or Uint8Array");
ThrowTypeError("SecureCellContextImprint",
"message is not a byte buffer, use ByteBuffer or Uint8Array");
args.GetReturnValue().SetUndefined();
return;
}
Expand All @@ -173,8 +172,8 @@ void SecureCellContextImprint::decrypt(const Nan::FunctionCallbackInfo<v8::Value
return;
}
if (!args[1]->IsUint8Array()) {
ThrowParameterError("Secure Cell (Context Imprint) failed to decrypt",
"context is not a byte buffer, use ByteBuffer or Uint8Array");
ThrowTypeError("SecureCellContextImprint",
"context is not a byte buffer, use ByteBuffer or Uint8Array");
args.GetReturnValue().SetUndefined();
return;
}
Expand Down
22 changes: 10 additions & 12 deletions src/wrappers/themis/jsthemis/secure_cell_seal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ void SecureCellSeal::New(const Nan::FunctionCallbackInfo<v8::Value>& args)
return;
}
if (!args[0]->IsUint8Array()) {
ThrowParameterError("Secure Cell (Seal) constructor",
"master key is not a byte buffer, use ByteBuffer or Uint8Array");
ThrowTypeError("SecureCellSeal",
"master key is not a byte buffer, use ByteBuffer or Uint8Array");
args.GetReturnValue().SetUndefined();
return;
}
Expand Down Expand Up @@ -96,8 +96,7 @@ void SecureCellSeal::encrypt(const Nan::FunctionCallbackInfo<v8::Value>& args)
return;
}
if (!args[0]->IsUint8Array()) {
ThrowParameterError("Secure Cell (Seal) failed to encrypt",
"message is not a byte buffer, use ByteBuffer or Uint8Array");
ThrowTypeError("SecureCellSeal", "message is not a byte buffer, use ByteBuffer or Uint8Array");
args.GetReturnValue().SetUndefined();
return;
}
Expand All @@ -109,10 +108,10 @@ void SecureCellSeal::encrypt(const Nan::FunctionCallbackInfo<v8::Value>& args)
size_t length = 0;
const uint8_t* context = NULL;
size_t context_length = 0;
if (args.Length() == 2) {
if (args.Length() == 2 && !args[1]->IsNull()) {
if (!args[1]->IsUint8Array()) {
ThrowParameterError("Secure Cell (Seal) failed to encrypt",
"context is not a byte buffer, use ByteBuffer or Uint8Array");
ThrowTypeError("SecureCellSeal",
"context is not a byte buffer, use ByteBuffer or Uint8Array");
args.GetReturnValue().SetUndefined();
return;
}
Expand Down Expand Up @@ -161,8 +160,7 @@ void SecureCellSeal::decrypt(const Nan::FunctionCallbackInfo<v8::Value>& args)
return;
}
if (!args[0]->IsUint8Array()) {
ThrowParameterError("Secure Cell (Seal) failed to decrypt",
"message is not a byte buffer, use ByteBuffer or Uint8Array");
ThrowTypeError("SecureCellSeal", "message is not a byte buffer, use ByteBuffer or Uint8Array");
args.GetReturnValue().SetUndefined();
return;
}
Expand All @@ -174,10 +172,10 @@ void SecureCellSeal::decrypt(const Nan::FunctionCallbackInfo<v8::Value>& args)
size_t length = 0;
const uint8_t* context = NULL;
size_t context_length = 0;
if (args.Length() == 2) {
if (args.Length() == 2 && !args[1]->IsNull()) {
if (!args[1]->IsUint8Array()) {
ThrowParameterError("Secure Cell (Seal) failed to decrypt",
"context is not a byte buffer, use ByteBuffer or Uint8Array");
ThrowTypeError("SecureCellSeal",
"context is not a byte buffer, use ByteBuffer or Uint8Array");
args.GetReturnValue().SetUndefined();
return;
}
Expand Down
28 changes: 14 additions & 14 deletions src/wrappers/themis/jsthemis/secure_cell_token_protect.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ void SecureCellTokenProtect::New(const Nan::FunctionCallbackInfo<v8::Value>& arg
return;
}
if (!args[0]->IsUint8Array()) {
ThrowParameterError("Secure Cell (Token Protect) constructor",
"master key is not a byte buffer, use ByteBuffer or Uint8Array");
ThrowTypeError("SecureCellTokenProtect",
"master key is not a byte buffer, use ByteBuffer or Uint8Array");
args.GetReturnValue().SetUndefined();
return;
}
Expand Down Expand Up @@ -96,8 +96,8 @@ void SecureCellTokenProtect::encrypt(const Nan::FunctionCallbackInfo<v8::Value>&
return;
}
if (!args[0]->IsUint8Array()) {
ThrowParameterError("Secure Cell (Token Protect) failed to encrypt",
"message is not a byte buffer, use ByteBuffer or Uint8Array");
ThrowTypeError("SecureCellTokenProtect",
"message is not a byte buffer, use ByteBuffer or Uint8Array");
args.GetReturnValue().SetUndefined();
return;
}
Expand All @@ -110,10 +110,10 @@ void SecureCellTokenProtect::encrypt(const Nan::FunctionCallbackInfo<v8::Value>&
size_t token_length = 0;
const uint8_t* context = NULL;
size_t context_length = 0;
if (args.Length() == 2) {
if (args.Length() == 2 && !args[1]->IsNull()) {
if (!args[1]->IsUint8Array()) {
ThrowParameterError("Secure Cell (Token Protect) failed to encrypt",
"context is not a byte buffer, use ByteBuffer or Uint8Array");
ThrowTypeError("SecureCellTokenProtect",
"context is not a byte buffer, use ByteBuffer or Uint8Array");
args.GetReturnValue().SetUndefined();
return;
}
Expand Down Expand Up @@ -175,8 +175,8 @@ void SecureCellTokenProtect::decrypt(const Nan::FunctionCallbackInfo<v8::Value>&
return;
}
if (!args[0]->IsUint8Array()) {
ThrowParameterError("Secure Cell (Token Protect) failed to decrypt",
"message is not a byte buffer, use ByteBuffer or Uint8Array");
ThrowTypeError("SecureCellTokenProtect",
"message is not a byte buffer, use ByteBuffer or Uint8Array");
args.GetReturnValue().SetUndefined();
return;
}
Expand All @@ -186,8 +186,8 @@ void SecureCellTokenProtect::decrypt(const Nan::FunctionCallbackInfo<v8::Value>&
return;
}
if (!args[1]->IsUint8Array()) {
ThrowParameterError("Secure Cell (Token Protect) failed to decrypt",
"token is not a byte buffer, use ByteBuffer or Uint8Array");
ThrowTypeError("SecureCellTokenProtect",
"token is not a byte buffer, use ByteBuffer or Uint8Array");
args.GetReturnValue().SetUndefined();
return;
}
Expand All @@ -199,10 +199,10 @@ void SecureCellTokenProtect::decrypt(const Nan::FunctionCallbackInfo<v8::Value>&
size_t length = 0;
const uint8_t* context = NULL;
size_t context_length = 0;
if (args.Length() == 3) {
if (args.Length() == 3 && !args[2]->IsNull()) {
if (!args[2]->IsUint8Array()) {
ThrowParameterError("Secure Cell (Token Protect) failed to decrypt",
"context is not a byte buffer, use ByteBuffer or Uint8Array");
ThrowTypeError("SecureCellTokenProtect",
"context is not a byte buffer, use ByteBuffer or Uint8Array");
args.GetReturnValue().SetUndefined();
return;
}
Expand Down
7 changes: 3 additions & 4 deletions src/wrappers/themis/jsthemis/secure_comparator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,8 @@ void SecureComparator::New(const Nan::FunctionCallbackInfo<v8::Value>& args)
return;
}
if (!args[0]->IsUint8Array()) {
ThrowParameterError("Secure Comparator constructor",
"secret is not a byte buffer, use ByteBuffer or Uint8Array");
ThrowTypeError("SecureComparator",
"secret is not a byte buffer, use ByteBuffer or Uint8Array");
return;
}
if (node::Buffer::Length(args[0]) == 0) {
Expand Down Expand Up @@ -133,8 +133,7 @@ void SecureComparator::proceedCompare(const Nan::FunctionCallbackInfo<v8::Value>
return;
}
if (!args[0]->IsUint8Array()) {
ThrowParameterError("Secure Comparator failed to proceed comparison",
"message is not a byte buffer, use ByteBuffer or Uint8Array");
ThrowTypeError("SecureComparator", "message is not a byte buffer, use ByteBuffer or Uint8Array");
return;
}
if (node::Buffer::Length(args[0]) == 0) {
Expand Down
12 changes: 5 additions & 7 deletions src/wrappers/themis/jsthemis/secure_keygen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,17 +59,17 @@ void KeyPair::New(const Nan::FunctionCallbackInfo<v8::Value>& args)
if (args.IsConstructCall()) {
if (args.Length() == 2) {
if (!args[0]->IsUint8Array()) {
ThrowParameterError("Key Pair constructor",
"private key is not a byte buffer, use ByteBuffer or Uint8Array");
ThrowTypeError("KeyPair",
"private key is not a byte buffer, use ByteBuffer or Uint8Array");
return;
}
if (node::Buffer::Length(args[0]) == 0) {
ThrowParameterError("Key Pair constructor", "private key is empty");
return;
}
if (!args[1]->IsUint8Array()) {
ThrowParameterError("Key Pair constructor",
"public key is not a byte buffer, use ByteBuffer or Uint8Array");
ThrowTypeError("KeyPair",
"public key is not a byte buffer, use ByteBuffer or Uint8Array");
return;
}
if (node::Buffer::Length(args[1]) == 0) {
Expand Down Expand Up @@ -228,8 +228,7 @@ void SymmetricKey::New(const Nan::FunctionCallbackInfo<v8::Value>& args)
// a byte buffer that we copy.
v8::Local<v8::Value> value = args[0];
if (!value->IsUint8Array()) {
ThrowParameterError("Themis SymmetricKey",
"key is not a byte buffer (use Buffer or Uint8Array)");
ThrowTypeError("SymmetricKey", "key is not a byte buffer (use Buffer or Uint8Array)");
args.GetReturnValue().SetUndefined();
return;
}
Expand All @@ -240,7 +239,6 @@ void SymmetricKey::New(const Nan::FunctionCallbackInfo<v8::Value>& args)
}

args.GetReturnValue().Set(CopyIntoBuffer(value));
return;
}

// TODO: return properly inherited instances of SymmetricKey
Expand Down
58 changes: 34 additions & 24 deletions src/wrappers/themis/jsthemis/secure_message.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,14 @@ void SecureMessage::Init(v8::Local<v8::Object> exports)
Nan::Set(exports, className, function);
}

static inline void assign_buffer_to_vector(std::vector<uint8_t>& vector,
const v8::Local<v8::Value>& buffer)
{
const uint8_t* data = reinterpret_cast<const uint8_t*>(node::Buffer::Data(buffer));
size_t length = node::Buffer::Length(buffer);
vector.assign(data, data + length);
}

void SecureMessage::New(const Nan::FunctionCallbackInfo<v8::Value>& args)
{
if (args.IsConstructCall()) {
Expand All @@ -66,24 +74,26 @@ void SecureMessage::New(const Nan::FunctionCallbackInfo<v8::Value>& args)
args.GetReturnValue().SetUndefined();
return;
}
if (!args[0]->IsUint8Array()) {
ThrowParameterError("Secure Message constructor",
"private key is not a byte buffer, use ByteBuffer or Uint8Array");
args.GetReturnValue().SetUndefined();
return;
std::vector<uint8_t> private_key;
if (!args[0]->IsNull()) {
if (!args[0]->IsUint8Array()) {
ThrowTypeError("SecureMessage",
"private key is not a byte buffer, use ByteBuffer or Uint8Array");
args.GetReturnValue().SetUndefined();
return;
}
assign_buffer_to_vector(private_key, args[0]);
}
if (!args[1]->IsUint8Array()) {
ThrowParameterError("Secure Message constructor",
"public key is not a byte buffer, use ByteBuffer or Uint8Array");
args.GetReturnValue().SetUndefined();
return;
std::vector<uint8_t> public_key;
if (!args[1]->IsNull()) {
if (!args[1]->IsUint8Array()) {
ThrowTypeError("SecureMessage",
"public key is not a byte buffer, use ByteBuffer or Uint8Array");
args.GetReturnValue().SetUndefined();
return;
}
assign_buffer_to_vector(public_key, args[1]);
}
std::vector<uint8_t> private_key((uint8_t*)(node::Buffer::Data(args[0])),
(uint8_t*)(node::Buffer::Data(args[0])
+ node::Buffer::Length(args[0])));
std::vector<uint8_t> public_key((uint8_t*)(node::Buffer::Data(args[1])),
(uint8_t*)(node::Buffer::Data(args[1])
+ node::Buffer::Length(args[1])));
if (!ValidateKeys(private_key, public_key)) {
args.GetReturnValue().SetUndefined();
return;
Expand All @@ -102,6 +112,10 @@ void SecureMessage::New(const Nan::FunctionCallbackInfo<v8::Value>& args)
bool SecureMessage::ValidateKeys(const std::vector<uint8_t>& private_key,
const std::vector<uint8_t>& public_key)
{
if (private_key.empty() && public_key.empty()) {
ThrowParameterError("SecureMessage", "private and public key cannot be both empty");
return false;
}
if (!private_key.empty()) {
if (!IsValidKey(private_key)) {
ThrowParameterError("Secure Message constructor", "invalid private key");
Expand Down Expand Up @@ -146,8 +160,7 @@ void SecureMessage::encrypt(const Nan::FunctionCallbackInfo<v8::Value>& args)
return;
}
if (!args[0]->IsUint8Array()) {
ThrowParameterError("Secure Message failed to encrypt message",
"message is not a byte buffer, use ByteBuffer or Uint8Array");
ThrowTypeError("SecureMessage", "message is not a byte buffer, use ByteBuffer or Uint8Array");
args.GetReturnValue().SetUndefined();
return;
}
Expand Down Expand Up @@ -209,8 +222,7 @@ void SecureMessage::decrypt(const Nan::FunctionCallbackInfo<v8::Value>& args)
return;
}
if (!args[0]->IsUint8Array()) {
ThrowParameterError("Secure Message failed to decrypt message",
"message is not a byte buffer, use ByteBuffer or Uint8Array");
ThrowTypeError("SecureMessage", "message is not a byte buffer, use ByteBuffer or Uint8Array");
args.GetReturnValue().SetUndefined();
return;
}
Expand Down Expand Up @@ -267,8 +279,7 @@ void SecureMessage::sign(const Nan::FunctionCallbackInfo<v8::Value>& args)
return;
}
if (!args[0]->IsUint8Array()) {
ThrowParameterError("Secure Message failed to sign message",
"message is not a byte buffer, use ByteBuffer or Uint8Array");
ThrowTypeError("SecureMessage", "message is not a byte buffer, use ByteBuffer or Uint8Array");
args.GetReturnValue().SetUndefined();
return;
}
Expand Down Expand Up @@ -321,8 +332,7 @@ void SecureMessage::verify(const Nan::FunctionCallbackInfo<v8::Value>& args)
return;
}
if (!args[0]->IsUint8Array()) {
ThrowParameterError("Secure Message failed to verify signature",
"message is not byte buffer, use ByteBuffer or Uint8Array");
ThrowTypeError("SecureMessage", "message is not byte buffer, use ByteBuffer or Uint8Array");
args.GetReturnValue().SetUndefined();
return;
}
Expand Down
Loading

0 comments on commit 49d8389

Please sign in to comment.