[WIP] Implement per database encryption for primary data #2685

eiri · 2020-03-20T08:00:58Z

Overview

This PR adds an option for enabling encryption for primary data. The encryption done with aes-256-gcm algorithm per document with document keys (Data Encryption Keys) derived from an individual database's key (Key Encryption Key). KEK supplied at time of the database creation and stored along with the rest of its configuration in encrypted "wrapped" form.

The encoding and decoding of documents handled in a dedicated module that also acts as a cache of "unwrapped" KEKs. The general management of KEKs implemented through epi plugin interface to allow a possibility for implementation of interfaces to alternative Key Management Services, e.g. Hashicorps Vault or SecretHub.

The default epi plugin generates KEK at a databae creation time and encrypts it with a Master Encryption Key specified in CouchDB configuration.

Testing recommendations

Make target make check-fdb should pass

eiri · 2020-03-20T08:09:00Z

@davisp @rnewson This PR is still work in progress, but can you please take a look if general shape of things seems fine to you?

rnewson · 2020-03-23T13:38:53Z

rel/overlay/etc/default.ini

+;
+; Keep both files as read-only and owned by couch process.
+; key_file = /var/secured/mount/location/key.dat
+; iv_file = /var/secured/mount/location/iv.dat


not sure why these are external and not inline given the other files are readable by the same user.

storing an IV here makes no immediate sense to me so I worry we have a crypto problem to come.

Config is writable, my idea is that key will be kept in read-only file with an option to have that file on a separate encrypted mount point.

How do you suggest to store IV?

there shouldn't be one.

rnewson · 2020-03-23T13:43:00Z

src/fabric/src/fabric2_fdb.erl

 AddSize = sum_add_rev_sizes([NewWinner | ToUpdate]),
 RemSize = sum_rem_rev_sizes(ToRemove),
 incr_stat(Db, <<"sizes">>, <<"external">>, AddSize - RemSize),
+ incr_stat(Db, <<"update_count">>, 1),


This increment will only happen if the transaction succeeds.

rnewson · 2020-03-23T13:43:19Z

src/fabric/src/fabric2_fdb.erl

+ _ ->
+ UpdateCounter = get_stat(Db, <<"update_count">>),
+ BinBody = term_to_binary(Body,
+ [{compressed, 0}, {minor_version, 1}]),


please compress the body.

rnewson · 2020-03-23T13:43:43Z

src/fabric/src/fabric2_fdb.erl

+ UpdateCounter = get_stat(Db, <<"update_count">>),
+ BinBody = term_to_binary(Body,
+ [{compressed, 0}, {minor_version, 1}]),
+ {ok, Encoded} = fabric2_encryption:encode(


encode is not an acceptable synonym for encrypt.

rnewson · 2020-03-23T13:44:54Z

src/fabric/src/fabric2_fdb.erl

+ false ->
+ {Body, DiskAtts, Deleted};
+ _ ->
+ UpdateCounter = get_stat(Db, <<"update_count">>),


that we fetch this stat and use it without guaranteeing it is unique is a serious problem. the update happens elsewhere.

stats are using atomic operations, if the counter got updated since the start of this transaction it'll get aborted here and retried, so counter guaranteed to be unique in a single transaction.

I have a question though: using read-write on atomic ops kills its performance benefits - can we use uuid instead of update counter, is it cryptographically solid enough? that would allow us to avoid "external" monitonic dependancy all together

That would not have a guarantee of uniqueness, which is the only reason we have "update counter" instead of "rev" from the original proposal.

rnewson · 2020-03-23T13:51:18Z

src/fabric/src/fabric2_encryption.erl

+ is_binary(DocId),
+ is_integer(UpdateCounter), UpdateCounter > 0,
+ is_binary(DocBody) ->
+ gen_server:call(?MODULE,


doing all the crypto work in a single gen_server sounds like a recipe for performance issues, even if we do spawn off worker processes there.

could we instead a) set the sensitive flag on self(), b) fetch the unwrapped key c) perform the operation in process d) unset sensitive flag?

This is because of KEK cache. Its erts table set to protected, so only gen server can read it, fetching unwrapped key from worker would require to change ets to private, but then any proc could read from it, there are no option to limit this, unfortunally.

you mean private there. a protected table can be read by all processes.

in the current code you hand the unwrapped key to another process (spawn_monitor). It doesn't appear to be more protected in that process than if we did the work in the calling process.

yes, private, thank you. it's more protected in sense that I can't remsh and read unwrapped key from ets. if you think this is not a concern then I'll make worker to do the unwrap fetch.

rnewson · 2020-03-23T13:51:42Z

src/fabric/src/fabric2_encryption.erl

+ end.
+
+
+encode(WrappedKEK, DbName, DocId, UpdateCounter, DocBody)


this function performs a cryptographic encryption not a mere encoding. rename to encrypt for clarity.

rnewson · 2020-03-23T13:51:49Z

src/fabric/src/fabric2_encryption.erl

+ {encode, WrappedKEK, DbName, DocId, UpdateCounter, DocBody}).
+
+
+decode(WrappedKEK, DbName, DocId, UpdateCounter, DocBody)


rnewson · 2020-03-23T13:52:21Z

src/fabric/src/fabric2_encryption.erl

+
+
+handle_call({get_wrapped_kek, DbName}, _From, #{cache := Cache} = St) ->
+ {ok, KEK, WrappedKEK} = fabric2_encryption_plugin:get_wrapped_kek(DbName),


can we avoid doing slow external work within the gen_server loop?

rnewson · 2020-03-23T13:53:07Z

src/fabric/src/fabric2_encryption.erl

+
+init(_) ->
+ process_flag(sensitive, true),
+ process_flag(trap_exit, true),


let's try to avoid building another supervisor of our own.

rnewson · 2020-03-23T13:56:16Z

src/fabric/src/fabric2_encryption_provider.erl

+
+
+unwrap_kek(WrappedKEK) ->
+ case get_mek_iv() of


reusing the same Key and IV for different data is a cryptographic no-no.

(we can use the key wrapping and unwrapping code from aegis instead)

This is Master key though, not databases' Key encryption key, it's not touching data, it only used to encrypt generated per db KEKs. I can change aes_ctr encryption of KEK I'm using here for aegis wrapping, but it'll still be the same key for wrap/unwrap operation.

we must use a standard key wrapping technique, not one of your own invention.

davisp · 2020-03-23T17:13:37Z

I'm gonna leave all the encryption related stuff to @rnewson and others that know better. My only concern was already called out about having a single gen_server handling all of that work. Though looks like there's been changes there already.

rnewson

This PR raised several key management issues that were left unspecified in the original IBM proposal for encryption at rest within CouchDB 4.0. That proposal uses a proprietary key management system (IBM Key Protect) that the Apache CouchDB project will not accept being coupled to.

The key management scheme for Apache CouchDB will be as follows;

Master Key

The master key is needed whenever CouchDB starts and is cached in memory for as long as CouchDB remains running. CouchDB will call an external script (using os:cmd()), if present, at a location configured in the .ini config file, and the script is required to print the master key to standard out (and nothing else). CouchDB will ship with a few example implementations of this script;

printing the contents of a local file.
fetching the contents of a url and printing the response body.

Per-Database Key's

These will be generated by CouchDB by calling crypto:strong_rand_bytes(32) and using the resultant value directly as the encryption key for a specific database. Each database will have its own independently generated key.
This value will be wrapped by the master key using the AES Key Wrap algorithm described in NIST Special Publication 800-38F (https://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-38F.pdf) so that administrators can, if they choose to, run in a FIPS 140-2 compliant manner (if they provide a FIPS 140-2 validated module, of course).
The wrapped value will be stored in the database where other database metadata is stored.

Per-Document Key's

These keys are derived from the per-database key using the key derivation function specified in FIPS 140-2 Annex D, Recommendation for Key Derivation Using Pseudorandom Functions, Section 5.1 (KDF in Counter Mode).

The key derivation function will be passed;

K_I: The per-database key
Label: The string "couchdb-aes256-gcm-encryption-key"
Context: The document id and document revision concatenated together, separated by a single `0x00` octet
h: 256
r: 16

When encryption provided registred in epi's provider chain it intercepts the call chain preventing alternative plugins from working. This changes it to be just a default module, so it'll be used when no other encryption plugins registered.

This reverts commit 8f9a988.

Instead of serializing and encrypting doc body separately this changes approach to encrypt the whole stored term prior chunkifying. This is now possible because we not depend on a stored in term update counter to derive a key anymore and saves a term/bin convertion per doc write/read.

- Set sensitive flags on workers - Use longer timeout for all the calls - Don't trap exit - No superfluous init_st - Properly drain waiters on the server termination

If getting KEK requires a database name, then depending on an encryption provider it could be also necessary for unwrapping it too, so pass it to unwrap function just in case.

eiri · 2020-04-17T04:06:33Z

Closing in favour of prototype/fdb-layer...aegis

rnewson requested changes Mar 23, 2020

View reviewed changes

rnewson self-requested a review March 24, 2020 10:22

rnewson reviewed Mar 24, 2020

View reviewed changes

eiri force-pushed the prototype/fdb-encryption branch from 513114c to 865b560 Compare April 3, 2020 20:46

eiri added 23 commits April 7, 2020 02:33

Add basic data encryption

72a3b5f

Don't block encryption server on encode/decode operations

e4af159

Grab KEK in main server loop

bbccfa7

Add basic KEK cache

e2236e9

Store wrapped KEK in db config

e50f63e

Add basic tests

6218abb

Switch to old crypto API for now

5a808bd

Switch from rev to update counter for key derivation

fecdee1

Add on/off config switch for encryption

f6c21be

Implement key management as an epi plugin

061d624

Change encryption provider to be default

8ac9b6b

When encryption provided registred in epi's provider chain it intercepts the call chain preventing alternative plugins from working. This changes it to be just a default module, so it'll be used when no other encryption plugins registered.

Revert from using update counter back to doc revs for key derivation

e609874

This reverts commit 8f9a988.

Rename encode/decode methods to encrypt/decrypt

76d8af8

Improvements in encryption server

1994787

- Set sensitive flags on workers - Use longer timeout for all the calls - Don't trap exit - No superfluous init_st - Properly drain waiters on the server termination

Request wrapped KEK out of encryption server loop

36591db

Delegate AAD generation to encryption plugin

c2b5912

Use a record for cache entries for better clarity

6f29391

Switch to couch_keywrap in encryption provider

34399cd

Make sure to use mocked config call in get_mek_failure_test

1b58f97

Use macro to choose old vs new crypto API depending on OTP version

2cc8d4f

Change interface for plugin's unwrap_kek

9d121e3

If getting KEK requires a database name, then depending on an encryption provider it could be also necessary for unwrapping it too, so pass it to unwrap function just in case.

Cache AAD alongside of KEK

eedae09

Unwrap KEK outside the main loop

086a4e7

eiri force-pushed the prototype/fdb-encryption branch from 865b560 to 086a4e7 Compare April 7, 2020 05:45

eiri closed this Apr 17, 2020

eiri deleted the prototype/fdb-encryption branch April 27, 2020 14:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Implement per database encryption for primary data #2685

[WIP] Implement per database encryption for primary data #2685

eiri commented Mar 20, 2020

eiri commented Mar 20, 2020

rnewson Mar 23, 2020

eiri Mar 23, 2020

rnewson Mar 23, 2020

rnewson Mar 23, 2020

rnewson Mar 23, 2020

rnewson Mar 23, 2020

rnewson Mar 23, 2020

eiri Mar 23, 2020

rnewson Mar 23, 2020

rnewson Mar 23, 2020

eiri Mar 23, 2020

rnewson Mar 23, 2020

eiri Mar 23, 2020

rnewson Mar 23, 2020

rnewson Mar 23, 2020

rnewson Mar 23, 2020

rnewson Mar 23, 2020

rnewson Mar 23, 2020

rnewson Mar 23, 2020

eiri Mar 23, 2020

rnewson Mar 23, 2020

davisp commented Mar 23, 2020

rnewson left a comment •

edited

Loading

eiri commented Apr 17, 2020

		end.


		encode(WrappedKEK, DbName, DocId, UpdateCounter, DocBody)

		{encode, WrappedKEK, DbName, DocId, UpdateCounter, DocBody}).


		decode(WrappedKEK, DbName, DocId, UpdateCounter, DocBody)



		handle_call({get_wrapped_kek, DbName}, _From, #{cache := Cache} = St) ->
		{ok, KEK, WrappedKEK} = fabric2_encryption_plugin:get_wrapped_kek(DbName),

[WIP] Implement per database encryption for primary data #2685

[WIP] Implement per database encryption for primary data #2685

Conversation

eiri commented Mar 20, 2020

Overview

Testing recommendations

eiri commented Mar 20, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davisp commented Mar 23, 2020

rnewson left a comment • edited Loading

Choose a reason for hiding this comment

eiri commented Apr 17, 2020

rnewson left a comment •

edited

Loading