Skip to content

Commit

Permalink
Move public Themis headers to "include" directory (#759)
Browse files Browse the repository at this point in the history
* Use new trait order in generated code

Recent release of bindgen 0.56.0 resulted in a change in the order of
the derived traits in the generated code. The changelog does not say
anything about that, but I doubt this is considered a notable change.
However, it breaks our build system which meticulously checks for any
changes in the generated code compared to checked in version. It does
this so that we don't miss any changes.

Well, the CI is going to be using bindgen 0.56.0 from now on, let's use
the new trait order as well, whatever it is.

* Move public Themis headers to "include" directory

As the subject line says, let's move all public Themis and Soter headers
into the new top-level directory "include". This is a common structure
for C projects to keep public headers separate. That way public API is
much more cleanly delineated.

Currently, Themis technically exports some private headers with private
API which might change in the future. With this new structure it will be
more obvious which files you can change without risking to introduce API
or ABI incompatibility.

* Add "include" directory to header search paths

Update Themis Core, AndroidThemis, and Carthage build files to include
the new "include" directory into the header search path. Since its
structure is the same as the previous "src" directory, no other changes
are necessary for compiler to be able to resolve "<themis/themis.h>".

* Update CocoaPods spec with header changes

Since CocoaPods is special needs package manager, a separate commit
takes care of it.

First, just like with other build systems, add the new "include"
directory into header search paths. Instruct CocoaPods to tell Xcode.

Then add new headers to the "source_files" list so that CocoaPods knows
that there are some new project headers and does not delete them
(because it deletes everything not explicitly mentioned there).

Also add the new headers to the "private_header_files" so that they
won't get treated as public headers by default. We don't need to export
Themis Core headers in ObjCThemis, but we do need them in the project.

With this directory split we don't really need the "header_dir" setting,
but we very much do need the "header_mappings_dir" so that CocoaPods
does not flatten the header structure. It's vitally important for
include path resolution since it's <soter/foo.h> and <themis/bar.h>
everywhere in the source code. Note that "header_mappings_dir" must be
a common directory of all headers. Well, with the "include" directory
being on the top-level, the only common directory we have left is the
repository root. So use it.

Repeat this for all three subspecs that we have. Reformat as necessary.
Don't forget that BoringSSL uses its own source set.

* Install only public headers

Update the installation rules to use public headers from "include", not
the ones from "src". However, include both in the header lists used by
code formatting and static analysis rules.

* Avoid using private headers in public API

EC keygen interface does not export EC key structure. RSA code should do
the same. The header structure for EC and RSA is a bit different due to
historical reasons, but both implementation details should be hidden.

The <soter/soter_rsa_key.h> header is a private one, it should not be
used in public headers. Remove it from <soter/soter_rsa_key_pair_gen.h>
and then manually add it in places which expected this header to be
transitively included.

* Move Secure Session details to private headers

The TODO says to "probably move this to private headers" and this
is exactly what we're doing here. All of these definitions are
implementation details of Secure Session and do not belong to the
public Themis API. Move them to appropriate internal headers.
Also, drop all the swathes of commented unused code from there.

* Update "libthemis-sys" definitions

We need to tell bindgen about new header location in "include". After
that, regenerate FFI bindings to clean up all the private API which got
finally hidden from public headers.

* Detailed eviction notice in CHANGELOG
* Tweak CHANGELOG entry formatting
  • Loading branch information
ilammy committed Nov 29, 2020
1 parent 3f8f9b8 commit 39cd5ce
Show file tree
Hide file tree
Showing 35 changed files with 252 additions and 180 deletions.
52 changes: 52 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,58 @@ _Code:_
- `make deb` and `make rpm` with `ENGINE=boringssl` will now produce `libthemis-boringssl` packages with embedded BoringSSL ([#683](https://github.com/cossacklabs/themis/pull/683), [#686](https://github.com/cossacklabs/themis/pull/686)).
- `secure_session_create()` now allows only EC keys, returning an error for RSA ([#693](https://github.com/cossacklabs/themis/pull/693)).
- Cleaned up unused private API. Thanks to [**@luismerino**](https://github.com/luismerino) for pointing this out ([#714](https://github.com/cossacklabs/themis/pull/714)).
- Cleaned up public header files and API of Themis and Soter ([#759](https://github.com/cossacklabs/themis/pull/759)).

Private header files are no longer installed. Private APIs which have been unintentially exported are no longer available. This might be a **breaking change** for those who have used them. Please refrain from using private API and include only public API:

```c
#include <themis/themis.h>
```

Users of official high-level wrappers are not affected. However, this might affect developers of third-party wrappers. Refer to the detailed description below for a list of removed headers.

<details>

The following Soter headers are no longer available:

- `<soter/soter_container.h>`
- `<soter/soter_crc32.h>`
- `<soter/soter_ec_key.h>`
- `<soter/soter_portable_endian.h>`
- `<soter/soter_rsa_key.h>`
- `<soter/soter_sign_ecdsa.h>`
- `<soter/soter_sign_rsa.h>`
- `<soter/soter_t.h>`

All APIs previously exported by them are no longer available as well.

The following Themis headers are no longer available:

- `<themis/secure_cell_alg.h>`
- `<themis/secure_cell_seal_passphrase.h>`
- `<themis/secure_comparator_t.h>`
- `<themis/secure_message_wrapper.h>`
- `<themis/secure_session_peer.h>`
- `<themis/secure_session_t.h>`
- `<themis/secure_session_utils.h>`
- `<themis/sym_enc_message.h>`
- `<themis/themis_portable_endian.h>`

All APIs previously exported by them are no longer available as well.

In addition to that, the following private symbols and definitions previously exported by `<themis/secure_session.h>` have been hidden:

- `THEMIS_SESSION_ID_TAG`
- `THEMIS_SESSION_PROTO_TAG`
- `SESSION_MASTER_KEY_LENGTH`
- `SESSION_MESSAGE_KEY_LENGTH`
- `struct secure_session_peer_type`
- `typedef secure_session_peer_t`
- `typedef secure_session_handler`
- `secure_session_peer_init()`
- `secure_session_peer_cleanup()`

</details>

- **Go**

Expand Down
3 changes: 2 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ INSTALL_DATA ?= $(INSTALL) -m 644

#----- Build directories -------------------------------------------------------

INC_PATH = include
SRC_PATH = src
BIN_PATH = $(BUILD_PATH)
OBJ_PATH = $(BIN_PATH)/obj
Expand All @@ -85,7 +86,7 @@ pkgconfigdir ?= $(libdir)/pkgconfig
#----- Basic compiler flags ----------------------------------------------------

# Add Themis source directory to search paths
CFLAGS += -I$(SRC_PATH) -I$(SRC_PATH)/wrappers/themis/
CFLAGS += -I$(INC_PATH) -I$(SRC_PATH) -I$(SRC_PATH)/wrappers/themis/
LDFLAGS += -L$(BIN_PATH)
# Not all platforms include /usr/local in default search path
CFLAGS += -I/usr/local/include
Expand Down
104 changes: 62 additions & 42 deletions Themis.xcodeproj/project.pbxproj

Large diffs are not rendered by default.

File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@

#include <soter/soter_api.h>
#include <soter/soter_error.h>
#include <soter/soter_rsa_key.h>

typedef struct soter_rsa_key_pair_gen_type soter_rsa_key_pair_gen_t;

Expand Down
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
66 changes: 0 additions & 66 deletions src/themis/secure_session.h → include/themis/secure_session.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,6 @@ extern "C" {
* @{
*/

/** @brief id tag */
#define THEMIS_SESSION_ID_TAG "TSID"
/** @brief protocol tag */
#define THEMIS_SESSION_PROTO_TAG "TSPM"

/** @brief idle state define */
#define STATE_IDLE 0
/** @brief negotiating state define */
Expand Down Expand Up @@ -89,69 +84,8 @@ struct secure_session_user_callbacks_type {

typedef struct secure_session_user_callbacks_type secure_session_user_callbacks_t;

/* TODO: probably move this to private headers */
//#include <soter/soter_t.h>
#include <themis/secure_session_peer.h>
/* TODO: move to separate header */
struct secure_session_peer_type {
uint8_t* id;
size_t id_length;

uint8_t* ecdh_key;
size_t ecdh_key_length;

uint8_t* sign_key;
size_t sign_key_length;
};

typedef struct secure_session_peer_type secure_session_peer_t;

themis_status_t secure_session_peer_init(secure_session_peer_t* peer,
const void* id,
size_t id_len,
const void* ecdh_key,
size_t ecdh_key_len,
const void* sign_key,
size_t sign_key_len);
void secure_session_peer_cleanup(secure_session_peer_t* peer);

#define SESSION_MASTER_KEY_LENGTH 32
/* TODO: for now session keys are same length as master key */
#define SESSION_MESSAGE_KEY_LENGTH SESSION_MASTER_KEY_LENGTH

typedef struct secure_session_type secure_session_t;
typedef themis_status_t (*secure_session_handler)(secure_session_t* session_ctx,
const void* data,
size_t data_length,
void* output,
size_t* output_length);

/*struct secure_session_type
{
soter_asym_ka_t ecdh_ctx;
const secure_session_user_callbacks_t *user_callbacks;
secure_session_handler state_handler;
struct secure_session_peer_type we;
struct secure_session_peer_type peer;
uint32_t session_id;
uint8_t session_master_key[SESSION_MASTER_KEY_LENGTH];
uint8_t out_cipher_key[SESSION_MESSAGE_KEY_LENGTH];
uint8_t in_cipher_key[SESSION_MESSAGE_KEY_LENGTH];
uint32_t out_seq;
uint32_t in_seq;
bool is_client;
};

themis_status_t secure_session_init(secure_session_t *session_ctx, const void *id, size_t id_length,
const void *sign_key, size_t sign_key_length, const secure_session_user_callbacks_t
*user_callbacks); themis_status_t secure_session_cleanup(secure_session_t *session_ctx);
*/
THEMIS_API
secure_session_t* secure_session_create(const void* id,
size_t id_length,
Expand Down
File renamed without changes.
File renamed without changes.
File renamed without changes.
13 changes: 9 additions & 4 deletions jni/Android.mk
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ LOCAL_SRC_FILES += $(patsubst jni/%,%, $(wildcard $(LOCAL_PATH)/../src/soter/ed2

LOCAL_CFLAGS := -DBORINGSSL -DCRYPTO_ENGINE_PATH=boringssl
LOCAL_EXPORT_CFLAGS := -DBORINGSSL
LOCAL_C_INCLUDES := $(LOCAL_PATH)/../src
LOCAL_C_INCLUDES := $(LOCAL_PATH)/../include
LOCAL_C_INCLUDES += $(LOCAL_PATH)/../src
LOCAL_C_INCLUDES += $(LOCAL_PATH)/../third_party/boringssl/src/include

include $(BUILD_STATIC_LIBRARY)
Expand All @@ -43,7 +44,8 @@ LOCAL_MODULE := libthemis

LOCAL_SRC_FILES := $(patsubst jni/%,%, $(wildcard $(LOCAL_PATH)/../src/themis/*.c))
LOCAL_CFLAGS := -DBORINGSSL -DCRYPTO_ENGINE_PATH=boringssl
LOCAL_C_INCLUDES := $(LOCAL_PATH)/../src
LOCAL_C_INCLUDES := $(LOCAL_PATH)/../include
LOCAL_C_INCLUDES += $(LOCAL_PATH)/../src
LOCAL_C_INCLUDES += $(LOCAL_PATH)/../third_party/boringssl/src/include

include $(BUILD_STATIC_LIBRARY)
Expand All @@ -55,7 +57,8 @@ LOCAL_MODULE := libthemis_jni
LOCAL_SRC_FILES := themis_jni.c themis_message.c themis_keygen.c themis_cell.c themis_session.c
LOCAL_SRC_FILES += themis_compare.c
LOCAL_CFLAGS := -DBORINGSSL -DCRYPTO_ENGINE_PATH=boringssl
LOCAL_C_INCLUDES := $(LOCAL_PATH)/../src
LOCAL_C_INCLUDES := $(LOCAL_PATH)/../include
LOCAL_C_INCLUDES += $(LOCAL_PATH)/../src
LOCAL_C_INCLUDES += $(LOCAL_PATH)/../third_party/boringssl/src/include
LOCAL_STATIC_LIBRARIES := libthemis libsoter libcrypto libdecrepit

Expand Down Expand Up @@ -85,7 +88,9 @@ LOCAL_SRC_FILES += $(patsubst jni/%,%, $(wildcard $(LOCAL_PATH)/../tests/common/

LOCAL_SHARED_LIBRARIES := libthemis_jni

LOCAL_C_INCLUDES := $(LOCAL_PATH)/../src $(LOCAL_PATH)/../tests
LOCAL_C_INCLUDES := $(LOCAL_PATH)/../include
LOCAL_C_INCLUDES += $(LOCAL_PATH)/../src
LOCAL_C_INCLUDES += $(LOCAL_PATH)/../tests

include $(BUILD_EXECUTABLE)

Expand Down
1 change: 1 addition & 0 deletions src/soter/openssl/soter_rsa_key_pair_gen.c
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include <openssl/rsa.h>

#include "soter/openssl/soter_engine.h"
#include "soter/soter_rsa_key.h"

static int rsa_key_length(unsigned size)
{
Expand Down
5 changes: 3 additions & 2 deletions src/soter/soter.mk
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ LIBSOTER_SO_LDFLAGS = -Wl,-out-implib,$(BIN_PATH)/$(LIBSOTER_IMPORT)
endif

SOTER_SOURCES = $(wildcard $(SRC_PATH)/soter/*.c)
SOTER_HEADERS = $(wildcard $(SRC_PATH)/soter/*.h)
SOTER_HEADERS += $(wildcard $(INC_PATH)/soter/*.h)
SOTER_HEADERS += $(wildcard $(SRC_PATH)/soter/*.h)
ED25519_SOURCES = $(wildcard $(SRC_PATH)/soter/ed25519/*.c)
ED25519_HEADERS = $(wildcard $(SRC_PATH)/soter/ed25519/*.h)

Expand Down Expand Up @@ -103,7 +104,7 @@ ifdef IS_MSYS
@mkdir -p $(DESTDIR)$(bindir)
endif
@mkdir -p $(DESTDIR)$(libdir)
@$(INSTALL_DATA) $(SRC_PATH)/soter/*.h $(DESTDIR)$(includedir)/soter
@$(INSTALL_DATA) $(INC_PATH)/soter/*.h $(DESTDIR)$(includedir)/soter
@$(INSTALL_DATA) $(BIN_PATH)/libsoter.pc $(DESTDIR)$(pkgconfigdir)
@$(INSTALL_DATA) $(BIN_PATH)/$(LIBSOTER_A) $(DESTDIR)$(libdir)
ifdef IS_MSYS
Expand Down
23 changes: 23 additions & 0 deletions src/themis/secure_session_peer.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,27 @@

#include <themis/secure_session.h>

struct secure_session_peer_type {
uint8_t* id;
size_t id_length;

uint8_t* ecdh_key;
size_t ecdh_key_length;

uint8_t* sign_key;
size_t sign_key_length;
};

typedef struct secure_session_peer_type secure_session_peer_t;

themis_status_t secure_session_peer_init(secure_session_peer_t* peer,
const void* id,
size_t id_len,
const void* ecdh_key,
size_t ecdh_key_len,
const void* sign_key,
size_t sign_key_len);

void secure_session_peer_cleanup(secure_session_peer_t* peer);

#endif /* THEMIS_SECURE_SESSION_PEER_H */
13 changes: 13 additions & 0 deletions src/themis/secure_session_t.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,19 @@
#include <soter/soter_t.h>

#include <themis/secure_session.h>
#include <themis/secure_session_peer.h>
#include <themis/secure_session_utils.h>

/** @brief id tag */
#define THEMIS_SESSION_ID_TAG "TSID"
/** @brief protocol tag */
#define THEMIS_SESSION_PROTO_TAG "TSPM"

typedef themis_status_t (*secure_session_handler)(secure_session_t* session_ctx,
const void* data,
size_t data_length,
void* output,
size_t* output_length);

struct secure_session_type {
soter_asym_ka_t ecdh_ctx;
Expand Down
4 changes: 4 additions & 0 deletions src/themis/secure_session_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@
#define CIPHER_MAX_BLOCK_SIZE 16
#define CIPHER_AUTH_TAG_SIZE 16

#define SESSION_MASTER_KEY_LENGTH 32
/* TODO: for now session keys are same length as master key */
#define SESSION_MESSAGE_KEY_LENGTH SESSION_MASTER_KEY_LENGTH

soter_sign_alg_t get_key_sign_type(const void* sign_key, size_t sign_key_length);
soter_sign_alg_t get_peer_key_sign_type(const void* sign_key, size_t sign_key_length);
themis_status_t compute_signature(const void* sign_key,
Expand Down
5 changes: 3 additions & 2 deletions src/themis/themis.mk
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ LIBTHEMIS_SO_LDFLAGS = -Wl,-out-implib,$(BIN_PATH)/$(LIBTHEMIS_IMPORT)
endif

THEMIS_SOURCES = $(wildcard $(SRC_PATH)/themis/*.c)
THEMIS_HEADERS = $(wildcard $(SRC_PATH)/themis/*.h)
THEMIS_HEADERS += $(wildcard $(INC_PATH)/themis/*.h)
THEMIS_HEADERS += $(wildcard $(SRC_PATH)/themis/*.h)

THEMIS_SRC = $(THEMIS_SOURCES)
THEMIS_AUD_SRC = $(THEMIS_SOURCES) $(THEMIS_HEADERS)
Expand Down Expand Up @@ -86,7 +87,7 @@ ifdef IS_MSYS
@mkdir -p $(DESTDIR)$(bindir)
endif
@mkdir -p $(DESTDIR)$(libdir)
@$(INSTALL_DATA) $(SRC_PATH)/themis/*.h $(DESTDIR)$(includedir)/themis
@$(INSTALL_DATA) $(INC_PATH)/themis/*.h $(DESTDIR)$(includedir)/themis
@$(INSTALL_DATA) $(BIN_PATH)/libthemis.pc $(DESTDIR)$(pkgconfigdir)
@$(INSTALL_DATA) $(BIN_PATH)/$(LIBTHEMIS_A) $(DESTDIR)$(libdir)
ifdef IS_MSYS
Expand Down
3 changes: 2 additions & 1 deletion src/wrappers/themis/rust/libthemis-sys/bindgen.sh
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ bindgen bindgen.h \
--whitelist-var "$WHITELIST" \
--output src/lib.rs \
-- \
-I ../../../.. # ${repository_root}/src
-I ../../../../../include \
-I ../../../../../src

TMP="$(mktemp)"

Expand Down
36 changes: 0 additions & 36 deletions src/wrappers/themis/rust/libthemis-sys/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,6 @@ pub const THEMIS_SCOMPARE_SEND_OUTPUT_TO_PEER: u32 = 1;
pub const THEMIS_SCOMPARE_MATCH: u32 = 21;
pub const THEMIS_SCOMPARE_NO_MATCH: u32 = 22;
pub const THEMIS_SCOMPARE_NOT_READY: u32 = 0;
pub const THEMIS_SESSION_ID_TAG: &'static [u8; 5usize] = b"TSID\0";
pub const THEMIS_SESSION_PROTO_TAG: &'static [u8; 5usize] = b"TSPM\0";
pub const STATE_IDLE: u32 = 0;
pub const STATE_NEGOTIATING: u32 = 1;
pub const STATE_ESTABLISHED: u32 = 2;
Expand Down Expand Up @@ -332,44 +330,10 @@ pub struct secure_session_user_callbacks_type {
pub type secure_session_user_callbacks_t = secure_session_user_callbacks_type;
#[repr(C)]
#[derive(Debug, Copy, Clone)]
pub struct secure_session_peer_type {
pub id: *mut u8,
pub id_length: usize,
pub ecdh_key: *mut u8,
pub ecdh_key_length: usize,
pub sign_key: *mut u8,
pub sign_key_length: usize,
}
pub type secure_session_peer_t = secure_session_peer_type;
extern "C" {
pub fn secure_session_peer_init(
peer: *mut secure_session_peer_t,
id: *const ::std::os::raw::c_void,
id_len: usize,
ecdh_key: *const ::std::os::raw::c_void,
ecdh_key_len: usize,
sign_key: *const ::std::os::raw::c_void,
sign_key_len: usize,
) -> themis_status_t;
}
extern "C" {
pub fn secure_session_peer_cleanup(peer: *mut secure_session_peer_t);
}
#[repr(C)]
#[derive(Debug, Copy, Clone)]
pub struct secure_session_type {
_unused: [u8; 0],
}
pub type secure_session_t = secure_session_type;
pub type secure_session_handler = ::std::option::Option<
unsafe extern "C" fn(
session_ctx: *mut secure_session_t,
data: *const ::std::os::raw::c_void,
data_length: usize,
output: *mut ::std::os::raw::c_void,
output_length: *mut usize,
) -> themis_status_t,
>;
extern "C" {
pub fn secure_session_create(
id: *const ::std::os::raw::c_void,
Expand Down
1 change: 1 addition & 0 deletions tests/soter/soter_test.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#define SOTER_TEST_H

#include <soter/soter.h>
#include <soter/soter_rsa_key.h>
#include <soter/soter_t.h>

#include <common/test_utils.h>
Expand Down
Loading

0 comments on commit 39cd5ce

Please sign in to comment.