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.

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

Secure Cell passphrase API: WasmThemis #616

Merged
merged 9 commits into from
Apr 8, 2020
Merged

Conversation

ilammy
Copy link
Collaborator

@ilammy ilammy commented Mar 30, 2020

Add support of Secure Cell passphrase API to WasmThemis. The API is described in RFC 3.6.

User notes

Passphrase-based interface of Secure Cell allows you to use short and memorable passphrases to secure your data. While symmetric keys are more secure, they are also longer and much harder for humans to remember.

Here is how you can use passphrases with Secure Cell in JavaScript:

const themis = require('wasm-themis')

let cell = themis.SecureCellSeal.withPassphrase('secret')

let message = Buffer.from('precious message')

let encrypted = cell.encrypt(message)
let decrypted = cell.decrypt(encrypted)

assert.equal(decrypted, message)

Passphrase API accepts passphrases as relatively short strings, suitable for human memory. Master key API uses randomly generated, long binary keys, which are more suitable for machines to remember. However, they are also much more efficient and generally more secure due to considerable length. You should prefer to use keys over passphrases if there are no humans involved. The interface is almost the same:

// Generate a new key if you don't have one:
let masterKey = new themis.SymmetricKey()
// Or use an existing value that you store somewhere:
let masterKey = Buffer.from('b0gyNlM4LTFKRDI5anFIRGJ4SmQyLGE7MXN5YWUzR2U=', 'base64')

let cell = themis.SecureCellSeal.withKey(masterKey)

let message = Buffer.from('precious message')

let encrypted = cell.encrypt(message)
let decrypted = cell.decrypt(encrypted)

assert.equal(decrypted, message)

API deprecations

new constructor of all Secure Cell classes is now (soft-)deprecated. We do not recommend its use due to ambiguity of whether you work with master keys or passphrases. Please use new factory methods instead:

// PLEASE DO
let cell = themis.SecureCellTokenProtect.withKey(masterKey)

// don't:
let cell = new themis.SecureCellTokenProtect(masterKey)

There are no plans to remove the constructors, they are still supported and will work as before, accepting only master keys.

Technical notes

There are no significant deviations from RFC 3.6.

Just like with other wrappers, the new subclass SecureCellSealWithPassphrase is kept private for now. It can be made public if it becomes necessary to subclass it, but for now it is expected that the users would not want to do that.

WebAssembly is still an VM which has certain overheads. Passphrase API seems to be ~4 times slower in WebAssembly than native code (~450 ms per call, compared to ~125 ms native). I have considered lowering PBKDF2 iterations for Wasm to improve performance, but I guess we should not lower the security because of that. If this becomes an issue for someone, the preferred way is to use master keys instead. If that is not possible, we'll consider expanding the API to allow advanced and informed users to tweak the PBKDF2 iteration count.

Checklist

  • Change is covered by automated tests
  • Benchmark results are attached (kinda, see CI test output)
  • The coding guidelines are followed
  • Public API has proper documentation
  • Example projects and code samples are up-to-date
  • Changelog is updated

Add new static factory methods "withKey()" to all Secure Cell classes,
with SecureCellSeal getting additional "withPasshrase()" to support
passphrase API.

The new API allows to disambiguate between master keys and passphrases
better then the current new constructor.

Passphrase API is implemented in a subclass of SecureCellSeal so that
all typing tests still work in JavaScript. The class itself is not
exported, the only way to construct it is through the withPassword()
method. That might change in the future.

There's quite a lot of copy-pasta involved in the implementation but for
now we did not find any convenient way to avoid it. Note that like
ObjCThemis, the "passphrase bytes" are kept in the "masterKey" property
of Secure Cell object. Themis Core does not really care, as long as they
are in an Uint8Array.

Also note TextEncoder usage to convert strings into UTF-8. JavaScript
strings are normally in UTF-16. TextEncoder is supported by modern Web
browsers but only for UTF-8. Node.js allows other encodings, but we're
not interested in them right now. Being limited with UTF-8 is the reason
for why we do not provide an out-of-the-box support for other encodings
like some other Themis wrappers do.
Update the tests to use the new construction syntax.

As for

> It's a really nice idea to accept strings as 'master keys'

this is not entirely true. This comment has been written at the time
when we believed that Secure Cell can be safely used with passphrases
via its master key API. Remove these comments, master key API is not
going to support strings anymore.
Add tests for passphrase API, taking into account its peculiarities and
incompatibility with the master key API.
Due to KDF usage, passphrase API is intentionally slow. However, Mocha
testing framework has too low default threshold for slow tests. Bump it
up a bit so that we're not risking to fail the test.

Also, ensure that passphrase API *is* slow enough, just in case it gets
swapped with master key API, or some technical progress will make
WebAssembly to run really fast.

Currently, bare-metal benchmarks of Themis Core suggest that default
iteration count for PBKDF2 results in 125 ms median time for encryption
and decryption. WebAssembly shows about 400-500 ms median. Well, not bad
but not amazing either.
@ilammy ilammy added the W-WasmThemis 🌐 Wrapper: WasmThemis, JavaScript API, npm packages label Mar 30, 2020
@ilammy
Copy link
Collaborator Author

ilammy commented Mar 30, 2020

Node.js v10:

ReferenceError: TextEncoder is not defined

*sigh* It's polyfill time!

O, JavaScript, language of a thousand incompatibilities! You managed
to bring Lisp to the Web, with all its merits and vices!

Long story short, Node.js v10 and some outdated browsers do not support
TextEncoder API. Add a polyfill to provide TextEncoder API on those
platforms.

We don't want to write yet another UTF-8 encoder ourselves to let's use
a library. This is our first external dependency in WasmThemis. Here are
some obligatory links:

  - https://github.com/anonyco/FastestSmallestTextEncoderDecoder
  - https://developer.mozilla.org/en-US/docs/Web/API/TextEncoder

The library does not have any dependencies itself, is licensed under
the Unlicense (effectively public domain) which is compatible with our
Apache 2.0, has seen some testing, and is quite minimal so we can
maintain it ourselves if necessary. However, I'd consider adding
explicit "npm audit" steps to build jobs from now on...
@ilammy
Copy link
Collaborator Author

ilammy commented Mar 30, 2020

O, JavaScript, language of a thousand incompatibilities! You managed to bring Lisp to the Web, with all its merits and vices!

Long story short, Node.js v10 and some outdated browsers do not support TextEncoder API. Add a polyfill to provide TextEncoder API on those platforms.

We don't want to write yet another UTF-8 encoder ourselves to let's use a library. This is our first external dependency in WasmThemis. Here are some obligatory links:

The library does not have any dependencies itself, is licensed under the Unlicense (effectively public domain) which is compatible with our Apache 2.0, has seen some testing, and is quite minimal so we can maintain it ourselves if necessary. However, I'd consider adding explicit npm audit steps to build jobs from now on...

Add a tool to check integration of passphrase API with other platforms.
describe('Seal mode (passphrase)', function() {
// Passphrase API uses KDF so it can be quite slow.
// Mocha uses default threshold of 75 ms which is not enough.
this.slow(1500) // milliseconds
Copy link
Collaborator

Choose a reason for hiding this comment

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

mocha stop test if it has execution time > 75ms and this.slow up this value?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's a bit more complex.

The default timeout is 2 seconds. Tests taking more time than that are considered failed.

The default slow test threshold – we adjust it here – is 75 milliseconds. Tests taking more time than that are reported as slow. They still pass but are indicated with warnings in test reports. Since KDF being slow is normal, I want to avoid having these warnings interpreted as false positives.

CHANGELOG.md Outdated

You can safely and securely use human-readable passphrases as text strings with this new API.

Existing master key API (available as `themis.SecureCellSeal.withKey(...)`) is not secure when used with passphrases. You should use it with symmetric encryption keys, such as generated by `SymmetricKey` ([#561](https://github.com/cossacklabs/themis/pull/561)).
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Existing master key API (available as `themis.SecureCellSeal.withKey(...)`) is not secure when used with passphrases. You should use it with symmetric encryption keys, such as generated by `SymmetricKey` ([#561](https://github.com/cossacklabs/themis/pull/561)).
Existing master key API (available as `themis.SecureCellSeal.withKey(...)`) should not be used with passphrases (short human-friendly passwords).
Use master key API with symmetric encryption keys, such as generated by `SymmetricKey` ([#561](https://github.com/cossacklabs/themis/pull/561)).
Use passphrase API with human-readable passphrases.

Also, if we have similar texts for other languages, I suggest to change to this formulating.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Also, if we have similar texts for other languages, I suggest to change to this formulating.

We do, of course. I've updated them as well.

Thank you. Your input on those things is always valuable.

@@ -132,81 +140,81 @@ describe('wasm-themis', function() {
})
let masterKey1 = new Uint8Array([1, 2, 3, 4])
let masterKey2 = new Uint8Array([5, 6, 7, 8, 9])
let passphrase1 = 'open sesame'
Copy link
Contributor

Choose a reason for hiding this comment

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

😂

@vixentael
Copy link
Contributor

Long story short, Node.js v10

shouldn't we drop v10 eventually? do you have some date in mind? v10 EOL will happen in a year (https://nodejs.org/en/about/releases/)

@ilammy
Copy link
Collaborator Author

ilammy commented Apr 2, 2020

shouldn't we drop v10 eventually? do you have some date in mind?

Probably. I recall our general approach to supporting platforms is to leave them running if we can support them without much active effort, even if they are EOL.

For example, we still build with Node.js v8 and will continue doing it until, say, we are no longer able to install it as easily as modern versions. However, we won't add workarounds to continue supporting it specifically if some new additions break with v8 and there is no straightforward and convenient way to support it. We also won't say "Node.js v8 is too old, please upgrade, I know it works perfectly fine but we won't test with it because you should feel bad for using old software".

Lack of security maintenance is a problem, but IMO it's not as important for a runtime as lack of maintenance in the cryptographic backend library. If our immediate dependency OpenSSL 1.0.2 is not supported since 2020, it is a reason to say that Themis does not support it, effective immediately. If application runtime is not supported since 2020, it is not as critical as far as our library is concerned. If you consider the entire application then yes, you should fear the possible security issues and upgrade, but I don't feel that a single library is in a position to enforce it for the application, even if it's a data security library.

To sum it up: no, I don't have a particular scheduled date for removal of Node.js v10 support. It will happen after it's EOL, when maintenance becomes a burden. The need to keep a dependency to maintain compatibility is a reason to drop it earlier, but hardly a defining one.

@vixentael
Copy link
Contributor

To sum it up: no, I don't have a particular scheduled date for removal of Node.js v10 support. It will happen after it's EOL, when maintenance becomes a burden.

Makes sense, thank you

Give more specific instructions on when to use master key API vs
passphrase API. Also, avoid phrasing about master key API being not
secure with passphrases. It *is* secure, given the passphrase of
sufficient length.

Update other languages to follow the style as well.

Co-Authored-By: vixentael <[email protected]>
@ilammy ilammy merged commit a2d50d3 into cossacklabs:master Apr 8, 2020
@ilammy ilammy deleted the kdf/wasm branch April 8, 2020 13:57
@ilammy ilammy mentioned this pull request Apr 9, 2020
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
W-WasmThemis 🌐 Wrapper: WasmThemis, JavaScript API, npm packages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants