Skip to content

Commit

Permalink
Fuzz passphrase API of Secure Cell (#583)
Browse files Browse the repository at this point in the history
* Fuzzers for passphrase API of Secure Cell

Currently passphrase API supports only Seal mode so we add tools for
that only (just like master key API is right now). In fact, the tools
are more or less copypasta of master key tools:

- Encryption code path is verified by "roundtrip" tool. It encrypts
  arbitrary counts of zeros and sees whether it fails to encrypt, or
  decrypt later, or decrypts incorrectly. Fuzzed values here are the
  parameter lengths used.

- Decryption code path is much more interesting and is verified by
  "decrypt" tool. Here we accept fuzzable input and try to decrypt it.
  This is probably the main attack surface of the new API, and boy
  are we vulnerable! AFL immediately found one crash and two more
  came after the first one was fixed. Stay tuned...

This commit also includes test data for the tools and a meta-tool used
to generate that data. Since GoThemis does not support passphrase API
yet, we use CGo directly here.

* Validate key length on decryption

This is the first crash found by AFL. Decryption code fails to validate
key length encoded in "algorithm" field of Secure Cell header and just
proceeds to use it, assuming that it's one of 128, 192, 256 bits.

Maximum value allowed by data format is 4096 bits (512 bytes), but the
code allocates only 32 bytes for the derived key. Longer keys will cause
a buffer overflow which allows an attacker to overwrite return address
stored on the stack and to theoretically execute arbitrary code.
Practically, this can be used for denial of service by exploiting the
much more likely outcome -- crash due to a segmentation fault.

Fix the issue by validating the key length and allowing only values
produced by Themis: 128, 192, or 256 bits.

* Avoid unsigned overflow in length computations

Additions like

    hdr->iv_length + hdr->auth_tag_length + hdr->kdf_context_length

and

    length += sizeof(hdr->auth_tag_length) + hdr->auth_tag_length

can cause unsigned overflow during itermediate computations. This can
lead to incorrect buffer lengths being computed, causing further pointer
computations to use invalid memory addresses, resuling in arbitrary
memory writes controlled by an attacker -- since these lengths come
from message data structure that we parse.

This is further complicated by the fact that "size_t" is typically
32-bit on 32-bit systems, causing more possible oveflows.

Bad news here is that unsigned integer overflow is *not* undefined
behavior in C -- it is defined to be two's complement. Therefore most
tools like UBSan and static analyzers don't catch issues due to them
not being "undefined behavior", so they are not reported. This issue
has been caught only due to its 'side effect' in parsing.

The only way to avoid issues is to either check for overflows or avoid
them. Both options require careful coding of each arithmetic operation,
so we opt for avoiding overflows without significant runtime cost. We
update the code to use "uint64_t" where the maximum size can exceed
uint32_t range (we know that it won't overflow uint64_t because there
are only a handful of values added), and similarly use "uint32_t" for
cases where the value cannot overflow "uint32_t".

Hopefully this is it... By the way, this is *the* reason why Rust
does not have implicit casts between integer types.

* Configurable PBKDF2 iteration count

KDF makes computations slow but fuzzing requires a lot of encryption and
decryption. Let's make the default iteration count to be configurable at
compile time and use a lower number for fuzzer builds. This drastically
increases efficiency of AFL search.

This feature is intended to be used mostly for debugging, and maybe to
provide custom builds on demand, so it's not documented anywhere except
for the code.

* Prevent false positive failures on 32-bit systems

Some sanitizers are built in 32-bit mode because of AFL's limitations.
We should avoid attempts to allocate over 2 GB on 32-bit systems with
Address Sanitizer enabled because this usually causes it to abort.
  • Loading branch information
ilammy committed Mar 5, 2020
1 parent e33f4e4 commit 7edf2df
Show file tree
Hide file tree
Showing 12 changed files with 543 additions and 21 deletions.
1 change: 1 addition & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ jobs:
- image: cossacklabs/build:ubuntu-bionic
environment:
FUZZ_TIMEOUT: 30s
THEMIS_DEFAULT_PBKDF2_ITERATIONS: 10
WITH_FATAL_WARNINGS: yes
WITH_FATAL_SANITIZERS: yes
steps:
Expand Down
12 changes: 8 additions & 4 deletions src/themis/secure_cell_alg.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,18 @@

#include <soter/soter_sym.h>

#ifndef THEMIS_DEFAULT_PBKDF2_ITERATIONS
#define THEMIS_DEFAULT_PBKDF2_ITERATIONS 200000
#endif

#ifdef THEMIS_AUTH_SYM_ALG_AES_256_GCM
#define THEMIS_AUTH_SYM_KEY_LENGTH SOTER_SYM_256_KEY_LENGTH
#define THEMIS_AUTH_SYM_ALG (SOTER_SYM_AES_GCM | THEMIS_AUTH_SYM_KEY_LENGTH)
#define THEMIS_AUTH_SYM_IV_LENGTH 12
#define THEMIS_AUTH_SYM_AUTH_TAG_LENGTH 16
#define THEMIS_AUTH_SYM_PASSPHRASE_ALG (THEMIS_AUTH_SYM_ALG | SOTER_SYM_PBKDF2)
#define THEMIS_AUTH_SYM_PBKDF2_SALT_LENGTH 16
#define THEMIS_AUTH_SYM_PBKDF2_ITERATIONS 200000
#define THEMIS_AUTH_SYM_PBKDF2_ITERATIONS (THEMIS_DEFAULT_PBKDF2_ITERATIONS)
#endif

#ifdef THEMIS_AUTH_SYM_ALG_AES_128_GCM
Expand All @@ -36,7 +40,7 @@
#define THEMIS_AUTH_SYM_AUTH_TAG_LENGTH 16
#define THEMIS_AUTH_SYM_PASSPHRASE_ALG (THEMIS_AUTH_SYM_ALG | SOTER_SYM_PBKDF2)
#define THEMIS_AUTH_SYM_PBKDF2_SALT_LENGTH 16
#define THEMIS_AUTH_SYM_PBKDF2_ITERATIONS 200000
#define THEMIS_AUTH_SYM_PBKDF2_ITERATIONS (THEMIS_DEFAULT_PBKDF2_ITERATIONS)
#endif

#ifdef THEMIS_AUTH_SYM_ALG_AES_192_GCM
Expand All @@ -46,7 +50,7 @@
#define THEMIS_AUTH_SYM_AUTH_TAG_LENGTH 16
#define THEMIS_AUTH_SYM_PASSPHRASE_ALG (THEMIS_AUTH_SYM_ALG | SOTER_SYM_PBKDF2)
#define THEMIS_AUTH_SYM_PBKDF2_SALT_LENGTH 16
#define THEMIS_AUTH_SYM_PBKDF2_ITERATIONS 200000
#define THEMIS_AUTH_SYM_PBKDF2_ITERATIONS (THEMIS_DEFAULT_PBKDF2_ITERATIONS)
#endif

/*default values*/
Expand All @@ -57,7 +61,7 @@
#define THEMIS_AUTH_SYM_AUTH_TAG_LENGTH 16
#define THEMIS_AUTH_SYM_PASSPHRASE_ALG (THEMIS_AUTH_SYM_ALG | SOTER_SYM_PBKDF2)
#define THEMIS_AUTH_SYM_PBKDF2_SALT_LENGTH 16
#define THEMIS_AUTH_SYM_PBKDF2_ITERATIONS 200000
#define THEMIS_AUTH_SYM_PBKDF2_ITERATIONS (THEMIS_DEFAULT_PBKDF2_ITERATIONS)
#endif

#define THEMIS_AUTH_SYM_MAX_KEY_LENGTH SOTER_SYM_256_KEY_LENGTH
Expand Down
15 changes: 14 additions & 1 deletion src/themis/secure_cell_seal_passphrase.c
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ static themis_status_t themis_auth_sym_derive_encryption_key_pbkdf2(

/* KDF pointer is ignored but size is important */
hdr->kdf_context = NULL;
hdr->kdf_context_length = (uint32_t)themis_scell_pbkdf2_context_size(&kdf);
hdr->kdf_context_length = themis_scell_pbkdf2_context_size(&kdf);

res = themis_write_scell_pbkdf2_context(hdr, &kdf, auth_token, *auth_token_length);
if (res != THEMIS_SUCCESS) {
Expand Down Expand Up @@ -299,6 +299,19 @@ static themis_status_t themis_auth_sym_derive_decryption_key(
size_t* derived_key_length)
{
size_t required_length = soter_alg_key_length(hdr->alg);
/*
* Accept only expected values that we know that we can handle. Robustness
* is more important here than future-proofing. AES is unlikely to support
* more key lengths anyway.
*/
switch (required_length) {
case SOTER_SYM_256_KEY_LENGTH / 8:
case SOTER_SYM_192_KEY_LENGTH / 8:
case SOTER_SYM_128_KEY_LENGTH / 8:
break;
default:
return THEMIS_FAIL;
}
/*
* This is our internal buffer so it cannot be too small. If it is, either
* the data is corrupted, or THEMIS_AUTH_SYM_MAX_KEY_LENGTH is incorrect.
Expand Down
40 changes: 26 additions & 14 deletions src/themis/secure_cell_seal_passphrase.h
Original file line number Diff line number Diff line change
Expand Up @@ -128,17 +128,21 @@ struct themis_scell_auth_token_passphrase {
uint32_t kdf_context_length;
};

static const size_t themis_scell_auth_token_passphrase_min_size = 5 * sizeof(uint32_t);
static const uint64_t themis_scell_auth_token_passphrase_min_size = 5 * sizeof(uint32_t);

static inline size_t themis_scell_auth_token_passphrase_size(
static inline uint64_t themis_scell_auth_token_passphrase_size(
const struct themis_scell_auth_token_passphrase* hdr)
{
size_t total_size = 0;
uint64_t total_size = 0;
/* Add separately to avoid overflows in intermediade calculations */
total_size += sizeof(hdr->alg);
total_size += sizeof(hdr->iv_length) + hdr->iv_length;
total_size += sizeof(hdr->auth_tag_length) + hdr->auth_tag_length;
total_size += sizeof(hdr->iv_length);
total_size += hdr->iv_length;
total_size += sizeof(hdr->auth_tag_length);
total_size += hdr->auth_tag_length;
total_size += sizeof(hdr->message_length);
total_size += sizeof(hdr->kdf_context_length) + hdr->kdf_context_length;
total_size += sizeof(hdr->kdf_context_length);
total_size += hdr->kdf_context_length;
return total_size;
}

Expand Down Expand Up @@ -170,7 +174,7 @@ static inline themis_status_t themis_write_scell_auth_token_passphrase(
static inline themis_status_t themis_read_scell_auth_token_passphrase(
const uint8_t* buffer, size_t buffer_length, struct themis_scell_auth_token_passphrase* hdr)
{
size_t need_length = themis_scell_auth_token_passphrase_min_size;
uint64_t need_length = themis_scell_auth_token_passphrase_min_size;
if (buffer_length < need_length) {
return THEMIS_FAIL;
}
Expand All @@ -179,7 +183,10 @@ static inline themis_status_t themis_read_scell_auth_token_passphrase(
buffer = stream_read_uint32LE(buffer, &hdr->auth_tag_length);
buffer = stream_read_uint32LE(buffer, &hdr->message_length);
buffer = stream_read_uint32LE(buffer, &hdr->kdf_context_length);
need_length += hdr->iv_length + hdr->auth_tag_length + hdr->kdf_context_length;
/* Add separately to avoid overflows in intermediade calculations */
need_length += hdr->iv_length;
need_length += hdr->auth_tag_length;
need_length += hdr->kdf_context_length;
if (buffer_length < need_length) {
return THEMIS_FAIL;
}
Expand Down Expand Up @@ -214,13 +221,15 @@ struct themis_scell_pbkdf2_context {
uint16_t salt_length;
};

static const size_t themis_scell_pbkdf2_context_min_size = sizeof(uint32_t) + sizeof(uint16_t);
static const uint32_t themis_scell_pbkdf2_context_min_size = sizeof(uint32_t) + sizeof(uint16_t);

static inline size_t themis_scell_pbkdf2_context_size(const struct themis_scell_pbkdf2_context* ctx)
static inline uint32_t themis_scell_pbkdf2_context_size(const struct themis_scell_pbkdf2_context* ctx)
{
size_t total_size = 0;
uint32_t total_size = 0;
/* Add separately to avoid overflows in intermediade calculations */
total_size += sizeof(ctx->iteration_count);
total_size += sizeof(ctx->salt_length) + ctx->salt_length;
total_size += sizeof(ctx->salt_length);
total_size += ctx->salt_length;
return total_size;
}

Expand All @@ -236,9 +245,12 @@ static inline themis_status_t themis_write_scell_pbkdf2_context(
if (hdr->kdf_context_length != themis_scell_pbkdf2_context_size(ctx)) {
return THEMIS_FAIL;
}
/* Add separately to avoid overflows in intermediade calculations */
buffer += sizeof(hdr->alg);
buffer += sizeof(hdr->iv_length) + hdr->iv_length;
buffer += sizeof(hdr->auth_tag_length) + hdr->auth_tag_length;
buffer += sizeof(hdr->iv_length);
buffer += hdr->iv_length;
buffer += sizeof(hdr->auth_tag_length);
buffer += hdr->auth_tag_length;
buffer += sizeof(hdr->message_length);
buffer += sizeof(hdr->kdf_context_length);
buffer = stream_write_uint32LE(buffer, ctx->iteration_count);
Expand Down
4 changes: 4 additions & 0 deletions src/themis/themis.mk
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ THEMIS_STATIC = $(BIN_PATH)/$(LIBTHEMIS_A) $(SOTER_STATIC)

$(THEMIS_OBJ): CFLAGS += -DTHEMIS_EXPORT

ifneq ($(THEMIS_DEFAULT_PBKDF2_ITERATIONS),)
$(THEMIS_OBJ): CFLAGS += -DTHEMIS_DEFAULT_PBKDF2_ITERATIONS=$(THEMIS_DEFAULT_PBKDF2_ITERATIONS)
endif

$(BIN_PATH)/$(LIBTHEMIS_A): CMD = $(AR) rcs $@ $(filter %.o, $^)

$(BIN_PATH)/$(LIBTHEMIS_A): $(THEMIS_OBJ)
Expand Down
132 changes: 132 additions & 0 deletions tools/afl/generate/scell_seal_decrypt_pw.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
/*
* Copyright (c) 2020 Cossack Labs Limited
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package main

// Test input generator for "scell_seal_decrypt_pw" fuzzer
//
// Run this tool to generate new test input for fuzzer:
//
// cd tools/afl
// go run generate/scell_seal_decrypt_pw.go "secret" "message" "context" \
// > input/scell_seal_decrypt_pw/example.dat
//
// "context" may be omitted. Output is binary so you want to redirect it.
//
// Themis must be installed. It is a good idea to build it with a lower PBKDF2
// iteration count (e.g., THEMIS_DEFAULT_PBKDF2_ITERATIONS=10) to get fuzzing
// results faster.

import (
"encoding/binary"
"fmt"
"io"
"os"
"unsafe"
)

/*
#cgo LDFLAGS: -lthemis
#include <stdbool.h>
#include <themis/themis.h>
static bool themis_scell_encrypt_seal_with_passphrase(
const void* passphrase,
size_t passphrase_length,
const void* user_context,
size_t user_context_length,
const void* message,
size_t message_length,
void* encrypted_message,
size_t* encrypted_message_length)
{
themis_status_t res = themis_secure_cell_encrypt_seal_with_passphrase(
passphrase,
passphrase_length,
user_context,
user_context_length,
message,
message_length,
encrypted_message,
encrypted_message_length
);
return (res == THEMIS_SUCCESS) || (res == THEMIS_BUFFER_TOO_SMALL);
}
*/
import "C"

// TODO: use GoThemis once passphrase API is available there
func encrypt(passphrase, context, message []byte) ([]byte, error) {
var encryptedSize C.size_t
if !C.themis_scell_encrypt_seal_with_passphrase(unsafe.Pointer(&passphrase[0]),
C.size_t(len(passphrase)),
unsafe.Pointer(&context[0]),
C.size_t(len(context)),
unsafe.Pointer(&message[0]),
C.size_t(len(message)),
nil,
&encryptedSize,
) {
return nil, fmt.Errorf("failed to compute output size")
}
encrypted := make([]byte, encryptedSize)
if !C.themis_scell_encrypt_seal_with_passphrase(unsafe.Pointer(&passphrase[0]),
C.size_t(len(passphrase)),
unsafe.Pointer(&context[0]),
C.size_t(len(context)),
unsafe.Pointer(&message[0]),
C.size_t(len(message)),
unsafe.Pointer(&encrypted[0]),
&encryptedSize,
) {
return nil, fmt.Errorf("failed to encrypt data")
}
return encrypted, nil
}

func writeByteString(w io.Writer, str []byte) {
binary.Write(w, binary.BigEndian, uint32(len(str)))
w.Write(str)
}

func main() {
args := os.Args

if (len(args) != 3) && (len(args) != 4) {
fmt.Printf("usage:\n\t%s <passphrase> <message> [<context>]\n", args[0])
os.Exit(1)
}

passphrase := []byte(args[1])
message := []byte(args[2])

var context []byte
if len(args) == 4 {
context = []byte(args[3])
}

encrypted, err := encrypt(passphrase, context, message)
if err != nil {
fmt.Fprintf(os.Stderr, "failed to encrypt: %s\n", err)
os.Exit(1)
}

writeByteString(os.Stdout, passphrase)
writeByteString(os.Stdout, context)
writeByteString(os.Stdout, encrypted)
}
Binary file not shown.
Binary file not shown.
27 changes: 25 additions & 2 deletions tools/afl/src/readline.c
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@
#include <errno.h>
#include <stdlib.h>

#define MAX_SANE_LENGTH (50 * 1024 * 1024)

int read_line_binary(FILE* input, uint8_t** out_bytes, size_t* out_size)
{
uint8_t length_bytes[4] = {0};
Expand Down Expand Up @@ -75,3 +73,28 @@ int read_line_binary(FILE* input, uint8_t** out_bytes, size_t* out_size)

return 0;
}

int read_u32be(FILE* input, uint32_t* out_value)
{
uint8_t value_bytes[4] = {0};

if (!input || !out_value) {
errno = EINVAL;
return -1;
}

if (fread(value_bytes, 1, 4, input) != 4) {
if (!errno) {
errno = EFAULT;
}
return -1;
}

*out_value = 0;
*out_value = (*out_value << 8) | value_bytes[0];
*out_value = (*out_value << 8) | value_bytes[1];
*out_value = (*out_value << 8) | value_bytes[2];
*out_value = (*out_value << 8) | value_bytes[3];

return 0;
}
13 changes: 13 additions & 0 deletions tools/afl/src/readline.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
#include <stdint.h>
#include <stdio.h>

#define MAX_SANE_LENGTH (50 * 1024 * 1024)

/**
* Read a byte string for a file, prefixed with 4-byte length (big-endian).
*
Expand All @@ -34,4 +36,15 @@
*/
int read_line_binary(FILE* input, uint8_t** out_bytes, size_t* out_size);

/**
* Read a big-endian unsinged 32-bit integer for a file.
*
* @param [in] input file to read, must be non-NULL
* @param [out] out_value resulting host-endian value will be put here,
* must be non-NULL
*
* @returns zero on success, negative value on error.
*/
int read_u32be(FILE* input, uint32_t* out_value);

#endif /* READ_LINE_H */
Loading

0 comments on commit 7edf2df

Please sign in to comment.