Skip to content

Commit

Permalink
Update header parser in Secure Cell master key API (#592)
Browse files Browse the repository at this point in the history
* Describe master key header format

Just like with passphrase API, introduce structures, serializers and
parsers for header used by master key API. It's mostly identical but
lacks KDF context.

* Update SCell master key decryption code path

Make it read like the current passphrase API using the same parsing
techniques. This resolves a bunch of possible vulnerabilities found
by AFL fuzzer which could be triggered by specific corruptions in
the header.

We reuse some utilities from passphrase API files so move them into
common internal header.

Note how we treat the compatibility path. Themis 0.9.6 used to use
incompatible format for one of the context fields used for Soter KDF
which resulted in encrypted data that cannot be decrypted by modern
implementation. We still support decryption of these 'broken' cells
by trying compatibility KDF if the initial decryption fails.

* Update SCell master key encryption code path

Similarly to decryption, unify the code with passphrase implementation.

* Drop unused old declarations

Since we use new definitions now, those are no longer necessary.

* Use new accessors in passphrase API

We have added a new accessor to KDF in Soter algorithm ID, use it.

* Validate key length on decryption

Just like with passphrase API, we need to pay close attention to the key
length field which we use to determine the length of the derived key.
Accept only formats currently produced by Themis. It's unlikely that we
update them, and Soter cannot handle other formats anyway right now.

* Avoid unsigned overflow in length computations

Similar to passphrase API, master key header can encode messages with
total length exceeding UINT32_MAX which can cause overflows on 32-bit
platforms (with 32-bit size_t).

Avoid overflows in computations by judiciously using uint64_t and
breaking up intermediate computations to ensure uint64_t is used.

* Update expected error codes in tests

Changes in master key implementations unify error reporting with
passphrase API as well. In particular, we consistently report detected
data corruption with THEMIS_FAIL error code now.

THEMIS_INVALID_PARAMETER is used to indicate programming errors which
could have been prevented by a better type system. Attempting to decrypt
corrupted data is not such an issue.

Some tests that verify corrupted data processing expect particular error
codes to be reported. Update them to expect THEMIS_FAIL now.

* Reduce code duplication in decryption path

We still need to maintain compatibility KDF implementation in order to
be able to decrypt data encrypted by Themis 0.9.6 which used incorrect
Soter KDF context computation, incompatible with 32-bit platforms.

Refactor the compatibility shim code to extact the context computation
and avoid duplicating the entire KDF processing, with all its length and
algorithm verification.
  • Loading branch information
ilammy committed Mar 5, 2020
1 parent 9b5f4c1 commit 2f19dfe
Show file tree
Hide file tree
Showing 7 changed files with 501 additions and 265 deletions.
17 changes: 17 additions & 0 deletions src/themis/secure_cell_alg.h
Original file line number Diff line number Diff line change
Expand Up @@ -88,4 +88,21 @@
#define THEMIS_SYM_IV_LENGTH 16
#endif

static inline bool soter_alg_reserved_bits_valid(uint32_t alg)
{
static const uint32_t used_bits = SOTER_SYM_KEY_LENGTH_MASK | SOTER_SYM_PADDING_MASK
| SOTER_SYM_KDF_MASK | SOTER_SYM_ALG_MASK;
return (alg & ~used_bits) == 0;
}

static inline size_t soter_alg_key_length(uint32_t alg)
{
return (alg & SOTER_SYM_KEY_LENGTH_MASK) / 8;
}

static inline uint32_t soter_alg_kdf(uint32_t alg)
{
return alg & SOTER_SYM_KDF_MASK;
}

#endif /* THEMIS_SECURE_CELL_ALG_H */
18 changes: 3 additions & 15 deletions src/themis/secure_cell_seal_passphrase.c
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ static inline uint32_t soter_alg_without_kdf(uint32_t alg)

static inline size_t soter_alg_kdf_context_length(uint32_t alg)
{
switch (alg & SOTER_SYM_KDF_MASK) {
switch (soter_alg_kdf(alg)) {
case SOTER_SYM_PBKDF2:
return themis_scell_pbkdf2_context_min_size + THEMIS_AUTH_SYM_PBKDF2_SALT_LENGTH;
case SOTER_SYM_NOKDF:
Expand All @@ -47,18 +47,6 @@ static inline size_t soter_alg_kdf_context_length(uint32_t alg)
}
}

static inline size_t soter_alg_key_length(uint32_t alg)
{
return (alg & SOTER_SYM_KEY_LENGTH_MASK) / 8;
}

static inline bool soter_alg_reserved_bits_valid(uint32_t alg)
{
static const uint32_t used_bits = SOTER_SYM_KEY_LENGTH_MASK | SOTER_SYM_PADDING_MASK
| SOTER_SYM_KDF_MASK | SOTER_SYM_ALG_MASK;
return (alg & ~used_bits) == 0;
}

static inline size_t default_auth_token_size(void)
{
return themis_scell_auth_token_passphrase_min_size + THEMIS_AUTH_SYM_IV_LENGTH
Expand Down Expand Up @@ -137,7 +125,7 @@ static themis_status_t themis_auth_sym_derive_encryption_key(struct themis_scell
}
*derived_key_length = required_length;

switch (hdr->alg & SOTER_SYM_KDF_MASK) {
switch (soter_alg_kdf(hdr->alg)) {
case SOTER_SYM_PBKDF2:
return themis_auth_sym_derive_encryption_key_pbkdf2(hdr,
passphrase,
Expand Down Expand Up @@ -317,7 +305,7 @@ static themis_status_t themis_auth_sym_derive_decryption_key(
}
*derived_key_length = required_length;

switch (hdr->alg & SOTER_SYM_KDF_MASK) {
switch (soter_alg_kdf(hdr->alg)) {
case SOTER_SYM_PBKDF2:
return themis_auth_sym_derive_decryption_key_pbkdf2(hdr,
passphrase,
Expand Down
Loading

0 comments on commit 2f19dfe

Please sign in to comment.