Symmetric keygen: high-level wrappers #561

ilammy · 2019-12-04T15:32:43Z

Implement symmetric key generation utilities described in RFC 1 (not available publicly at the moment). This is new API introduced in #560, now distributed to high-level wrappers.

This PR contains ‘easy’ wrappers that do not require anything special and with more or less straightforward and compact implementation. I placed them in one PR in order to have fewer merge conflicts in the changelog later.

Node.js (JsThemis) and Java (Android) are hard. They will be added in their own PRs.

Language API

With exception of C++ there is only one entry point for key generation. High-level wrappers generally do not allow the user to customize key length and generate default keys (32 bytes).

Where applicable, new types are introduced along with conversion API.

C++

#include <themispp/secure_keygen.hpp>

std::vector<uint8_t> key = themispp::gen_sym_key();

// Write by reference, mostly for C++03 compatibility:
themispp::gen_sym_key(key);

Go

import "github.com/cossacklabs/themis/gothemis/keys"

key, err := keys.NewSymmetricKey() // returns keys.SymmetricKey

key.Value // access []byte slice

JavaScript (WebAssembly)

const themis = required('wasm-themis')

let key = new themis.SymmetricKey()
// inherits from Uint8Array to access key bytes

// Can also wrap existing buffers
let buffer = new Uint8Array([1, 2, 3, 4, 5, 6])
let anotherKey = new themis.SymmetricKey(buffer)

Objective-C

#import <objcthemis/objcthemis.h>

NSData *key = TSGenerateSymmetricKey();

PHP

// returns string with key bytes
$key = phpthemis_gen_sym_key();

Python

from pythemis.skeygen import GenerateSymmetricKey

# bytes for Python 3, str for Python2
key = GenerateSymmetricKey()

Ruby

require 'rbthemis'

# returns byte string
key = Themis::gen_sym_key()

Rust

use themis::keys::SymmetricKey;

let key = SymmetricKey::new();
// implements AsRef<[u8]> to access key bytes

// Can also wrap existing buffers
let another_key = SymmetricKey::from_slice(&[1, 2, 3, 4])?;

Swift

import Themis

let key: Data? = TSGenerateSymmetricKey();

Checklist

Change is covered by automated tests
~~Benchmark results are attached~~ (not interesting)
The coding guidelines are followed
Public API has proper documentation
~~Example projects and code samples are updated~~ (later)
Changelog is updated if needed

C++ provides both convenient and in-place key generation routines. In-place key generation may be useful for C++03 code that does not want to have an extra copy of std::vector when it's being returned. ThemisPP does not provide separate types for keys, it works with plain std::vector.

Straightforward implementation. The only interestring thing is that size_t is mapped to i32 with Emscripten, as noted in all other APIs.

Straightforward implementation. GoThemis provides wrapper types for private and public keys so we add another one with the same interface for symmetric keys. Currently GoThemis does not return Themis error codes to Go, so CGo code returns "bool" for simplicity and consistency.

Straightforward implementation. Objective-C function is automatically bridged to Swift, we don't have to do anything for that. Keypair generation API for ObjCThemis does not report exact errors to the user and simply returns "nil", We do the same for symmetric key generation for consistency.

Straightforward implementation. PHPThemis exports its functionality as PHP functions so just add a new one. Byte arrays are returned as simply strings, no new types here. Just don't forget that we have to support both PHP 5 and PHP 7 wrappers.

Straightforward implementation. string_at() returns "str" with Python 2 and "bytes" with Python 3.

Straightforward implementation. Probably the most concise one :)

Straigtforward implementation (albeit verbose). RustThemis has a relatively high bar for API documentation quality. Don't let it drop and provide detailed descriptions. themis_gen_sym_key() is added to libthemis-sys by bindgen automatically, we don't have to remember to do that manually.

ilammy · 2019-12-06T09:18:21Z

I’ve rebased the changeset onto master which now has proper Core support. I’ve also added some updates in tests based on feedback in other PRs. This one is ready for review.

JavaScript vOv

Resolve a warning about "this function declaration is not a prototype". Objective-C is still C, not C++, so if a function does not take any arguments then it should have "(void)" argument list, not "()". Latter is considered to take any number of any arguments (for legacy reasons).

vixentael · 2019-12-06T11:26:25Z

src/wrappers/themis/Obj-C/objcthemis/skeygen.m

+    key = [NSMutableData dataWithLength:keyLength];
+
+    result = (TSErrorType) themis_gen_sym_key(key.mutableBytes, &keyLength);
+    if (result != TSErrorTypeSuccess) {


Suggested change

if (result != TSErrorTypeSuccess) {

if (result != TSErrorTypeSuccess) {

[key resetBytesInRange:NSMakeRange(0, [key length])];

~~Also, let's add tests here: https://github.com/cossacklabs/themis/tree/master/tests/objcthemis/objthemis~~

Found tests :)

[key resetBytesInRange:NSMakeRange(0, [key length])];

I'd think this will be excessive when these suggestion is implemented in Themis Core. It will ensure that if themis_gen_sym_key() fails then output buffer does not contain anything useful or sensitive. That way we will not need to add additional wiping to every single wrapper individually.

agree.

another reason I suggested it for iOS only – I don't know how to zeroing bytes in every other language :D

vixentael · 2019-12-06T11:36:09Z

That's a lot of going on here!

ilammy · 2019-12-06T11:56:33Z

That's a lot of going on here!

I have a faint feeling that this PR should have been nine separate PRs...

Also, @vixentael, by chance, do you have a guess why Bitrise build keeps failing? Because it's certainly fine on the master branch, the changes do not seem to have anything to do with OpenSSL includes, not to say that the framework and tests build fine on my machine.

Lagovas · 2019-12-06T12:21:33Z

tests/phpthemis/scell_test.php

+    public function testKeyGeneration() {
+        $defaultLength = 32;
+        $key = phpthemis_gen_sym_key();
+        $this->assertEquals(strlen($key), $defaultLength);


Will be better if for php we will have more checks, like in themis core, where we check that several calls of function return different keys. because php has a more complicated wrapper and own memory management.

Just like with JsThemis that has nontrivial wrapper, let's make sure that key generation returns distinct objects for every call and we don't do anything silly in the implementation.

ilammy · 2019-12-06T13:51:49Z

why Bitrise build keeps failing

I have a guess:

themis/tests/objcthemis/Podfile

Lines 24 to 25 in 5f886e1

    
           # example should work with head 
        
           pod 'themis', :git => "https://github.com/cossacklabs/themis.git"

This pulls in ObjCThemis code from master branch, not from a pull request, that's probably why the tests don't see the new API, and... that causes an error about <openssl/crypto.h> being not found to be displayed? Weird but possible. It's Apple after all, you should think different™

For local testing I have always been using an edited Podfile that refers to this branch. However, if I use it without changes then the errors are different, like more appropriate:

Use of unresolved identifier 'TSGenerateSymmetricKey'

ilammy · 2019-12-06T14:17:24Z

Oh, I get it now. Carthage build fails because carthage bootstrap step did not run as it’s not marked “run if previous steps fail”, and that causes an error with OpenSSL: of course we don’t have headers if the dependencies are not fetched. CocoaPods build fails for the reason I described above.

I’ve tweaked Bitrise build steps to avoid these issues:

Added a new Hack Podfile to use PR branch step before pod install so that we install the code from the pull request, not from the master branch.
Marked Carthage bootstrap step to “Run if previous Step failed”.

Let’s see if this is all we need.

vixentael · 2019-12-11T13:05:47Z

Looks like you fixed a problem with Bitrise

vixentael

Merge 'em all!

ilammy mentioned this pull request Dec 4, 2019

Symmetric keygen: JsThemis #562

Merged

6 tasks

ilammy added 10 commits December 6, 2019 11:16

Symmetric keygen: WasmThemis

2a7d3a8

Straightforward implementation. The only interestring thing is that size_t is mapped to i32 with Emscripten, as noted in all other APIs.

Symmetric keygen: PHPThemis

f72c0e1

Straightforward implementation. PHPThemis exports its functionality as PHP functions so just add a new one. Byte arrays are returned as simply strings, no new types here. Just don't forget that we have to support both PHP 5 and PHP 7 wrappers.

Symmetric keygen: PyThemis

c0cc339

Straightforward implementation. string_at() returns "str" with Python 2 and "bytes" with Python 3.

Symmetric keygen: RbThemis

971c2b2

Straightforward implementation. Probably the most concise one :)

Stricter length tests

87ea19a

More empty buffer checks

a00dbf0

ilammy force-pushed the themis_gen_sym_key/wrappers branch from 0c9e696 to a00dbf0 Compare December 6, 2019 09:16

ilammy marked this pull request as ready for review December 6, 2019 09:17

ilammy requested review from Lagovas and vixentael as code owners December 6, 2019 09:17

ilammy added 2 commits December 6, 2019 13:13

fixup! Stricter length tests

62ade93

JavaScript vOv

vixentael reviewed Dec 6, 2019

View reviewed changes

Lagovas reviewed Dec 6, 2019

View reviewed changes

More instance tests for PHP

5922a4f

Just like with JsThemis that has nontrivial wrapper, let's make sure that key generation returns distinct objects for every call and we don't do anything silly in the implementation.

vixentael requested review from vixentael and Lagovas December 11, 2019 13:07

vixentael approved these changes Dec 11, 2019

View reviewed changes

ilammy merged commit 3ff3cca into cossacklabs:master Dec 11, 2019

ilammy deleted the themis_gen_sym_key/wrappers branch December 11, 2019 18:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Symmetric keygen: high-level wrappers #561

Symmetric keygen: high-level wrappers #561

ilammy commented Dec 4, 2019 •

edited

Loading

ilammy commented Dec 6, 2019

vixentael Dec 6, 2019

vixentael Dec 6, 2019

ilammy Dec 6, 2019

vixentael Dec 6, 2019 •

edited

Loading

vixentael commented Dec 6, 2019

ilammy commented Dec 6, 2019

Lagovas Dec 6, 2019

ilammy commented Dec 6, 2019

ilammy commented Dec 6, 2019

vixentael commented Dec 11, 2019

vixentael left a comment

	if (result != TSErrorTypeSuccess) {
	if (result != TSErrorTypeSuccess) {
	[key resetBytesInRange:NSMakeRange(0, [key length])];

Symmetric keygen: high-level wrappers #561

Symmetric keygen: high-level wrappers #561

Conversation

ilammy commented Dec 4, 2019 • edited Loading

Language API

C++

Go

JavaScript (WebAssembly)

Objective-C

PHP

Python

Ruby

Rust

Swift

Checklist

ilammy commented Dec 6, 2019

vixentael Dec 6, 2019

Choose a reason for hiding this comment

vixentael Dec 6, 2019

Choose a reason for hiding this comment

ilammy Dec 6, 2019

Choose a reason for hiding this comment

vixentael Dec 6, 2019 • edited Loading

Choose a reason for hiding this comment

vixentael commented Dec 6, 2019

ilammy commented Dec 6, 2019

Lagovas Dec 6, 2019

Choose a reason for hiding this comment

ilammy commented Dec 6, 2019

ilammy commented Dec 6, 2019

vixentael commented Dec 11, 2019

vixentael left a comment

Choose a reason for hiding this comment

ilammy commented Dec 4, 2019 •

edited

Loading

vixentael Dec 6, 2019 •

edited

Loading