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

jared/thomcc/ffi cleanup #2138

Merged
merged 3 commits into from
Nov 1, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,12 @@ bool extern_error_has_error(const ockam_vault_extern_error_t* error) {
return error->code != 0;
}

bool extern_error_check_and_free_error(ockam_vault_extern_error_t* error) {
bool result = extern_error_has_error(error);
ockam_vault_free_error(error);
return result;
}

ERL_NIF_TERM ok_void(ErlNifEnv *env) {
return enif_make_atom(env, "ok");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@
#include <ockam/vault.h>
#include "erl_nif.h"

bool extern_error_has_error(const ockam_vault_extern_error_t* error);
bool extern_error_has_error(const ockam_vault_extern_error_t *error);
bool extern_error_check_and_free_error(ockam_vault_extern_error_t *error);

ERL_NIF_TERM ok_void(ErlNifEnv *env);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ ERL_NIF_TERM default_init(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]) {
ockam_vault_t vault;

ockam_vault_extern_error_t error = ockam_vault_default_init(&vault);
if (extern_error_has_error(&error)) {
if (extern_error_check_and_free_error(&error)) {
return err(env, "failed to create vault connection");
}

Expand Down Expand Up @@ -183,7 +183,7 @@ ERL_NIF_TERM sha256(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]) {
memset(digest, 0, 32);

ockam_vault_extern_error_t error = ockam_vault_sha256(vault, input.data, input.size, digest);
if (extern_error_has_error(&error)) {
if (extern_error_check_and_free_error(&error)) {
return err(env, "failed to compute sha256 digest");
}

Expand All @@ -207,7 +207,7 @@ ERL_NIF_TERM secret_generate(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]

ockam_vault_secret_t secret;
ockam_vault_extern_error_t error = ockam_vault_secret_generate(vault, &secret, attributes);
if (extern_error_has_error(&error)) {
if (extern_error_check_and_free_error(&error)) {
return err(env, "unable to generate the secret");
}

Expand Down Expand Up @@ -238,7 +238,7 @@ ERL_NIF_TERM secret_import(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[])

ockam_vault_secret_t secret;
ockam_vault_extern_error_t error = ockam_vault_secret_import(vault, &secret, attributes, input.data, input.size);
if (extern_error_has_error(&error)) {
if (extern_error_check_and_free_error(&error)) {
return err(env, "unable to import the secret");
}

Expand Down Expand Up @@ -266,7 +266,7 @@ ERL_NIF_TERM secret_export(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[])
uint32_t length = 0;

ockam_vault_extern_error_t error = ockam_vault_secret_export(vault, secret_handle, buffer, MAX_SECRET_EXPORT_SIZE, &length);
if (extern_error_has_error(&error)) {
if (extern_error_check_and_free_error(&error)) {
return err(env, "failed to ockam_vault_secret_export");
}

Expand Down Expand Up @@ -300,7 +300,7 @@ ERL_NIF_TERM secret_publickey_get(ErlNifEnv *env, int argc, const ERL_NIF_TERM a
uint32_t length = 0;

ockam_vault_extern_error_t error = ockam_vault_secret_publickey_get(vault, secret_handle, buffer, MAX_SECRET_EXPORT_SIZE, &length);
if (extern_error_has_error(&error)) {
if (extern_error_check_and_free_error(&error)) {
return err(env, "failed to ockam_vault_secret_publickey_get");
}

Expand Down Expand Up @@ -332,7 +332,7 @@ ERL_NIF_TERM secret_attributes_get(ErlNifEnv *env, int argc, const ERL_NIF_TERM

ockam_vault_secret_attributes_t attributes;
ockam_vault_extern_error_t error = ockam_vault_secret_attributes_get(vault, secret_handle, &attributes);
if (extern_error_has_error(&error)) {
if (extern_error_check_and_free_error(&error)) {
return err(env, "failed to secret_attributes_get");
}

Expand All @@ -355,7 +355,7 @@ ERL_NIF_TERM secret_destroy(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[])
}

ockam_vault_extern_error_t error = ockam_vault_secret_destroy(vault, secret_handle);
if (extern_error_has_error(&error)) {
if (extern_error_check_and_free_error(&error)) {
return err(env, "failed to secret_destroy");
}

Expand Down Expand Up @@ -384,7 +384,7 @@ ERL_NIF_TERM ecdh(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]) {

ockam_vault_secret_t shared_secret;
ockam_vault_extern_error_t error = ockam_vault_ecdh(vault, secret_handle, input.data, input.size, &shared_secret);
if (extern_error_has_error(&error)) {
if (extern_error_check_and_free_error(&error)) {
return err(env, "failed to ecdh");
}

Expand Down Expand Up @@ -449,7 +449,7 @@ ERL_NIF_TERM hkdf_sha256(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]) {

ockam_vault_secret_t shared_secrets[MAX_DERIVED_OUTPUT_COUNT];
ockam_vault_extern_error_t error = ockam_vault_hkdf_sha256(vault, salt_handle, ikm_handle_ptr, attributes, derived_outputs_count, shared_secrets);
if (extern_error_has_error(&error)) {
if (extern_error_check_and_free_error(&error)) {
return err(env, "failed to hkdf_sha256");
}

Expand Down Expand Up @@ -516,7 +516,7 @@ ERL_NIF_TERM aead_aes_gcm_encrypt(ErlNifEnv *env, int argc, const ERL_NIF_TERM a
cipher_text,
size,
&length);
if (extern_error_has_error(&error)) {
if (extern_error_check_and_free_error(&error)) {
return err(env, "failed to aead_aes_gcm_encrypt");
}

Expand Down Expand Up @@ -584,7 +584,7 @@ ERL_NIF_TERM aead_aes_gcm_decrypt(ErlNifEnv *env, int argc, const ERL_NIF_TERM a
plain_text,
size,
&length);
if (extern_error_has_error(&error)) {
if (extern_error_check_and_free_error(&error)) {
return err(env, "failed to aead_aes_gcm_decrypt");
}

Expand All @@ -606,7 +606,7 @@ ERL_NIF_TERM deinit(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]) {
}

ockam_vault_extern_error_t error = ockam_vault_deinit(vault);
if (extern_error_has_error(&error)) {
if (extern_error_check_and_free_error(&error)) {
return err(env, "failed to deinit vault");
}

Expand Down
4 changes: 4 additions & 0 deletions implementations/rust/ockam/ockam_ffi/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ description = """FFI layer for ockam_vault.
[lib]
crate-type = ["staticlib"]

[features]
default = []
bls = ["ockam_vault_core/bls"]

[dependencies]
ockam_core = { path = "../ockam_core", version = "0.36.0" }
ockam_vault_core = { path = "../ockam_vault_core", version = "0.30.0" }
Expand Down
39 changes: 26 additions & 13 deletions implementations/rust/ockam/ockam_ffi/include/vault.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,14 @@ typedef struct {

typedef uint64_t ockam_vault_secret_t;

/**
* @struct ockam_vault_extern_error_t
* @brief Represents an error that occured in one of the `ockam_vault` functions.
*
* In the case of an error, resources associated with this error (the `domain` string)
* must be released using @ref ockam_vault_free_error (which is a no-op if an error did
* not occur) in order to avoid a memory leak.
*/
typedef struct {
int32_t code;
const char *domain;
Expand Down Expand Up @@ -55,7 +63,7 @@ typedef struct {
/**
* @brief Initialize the specified ockam vault object
* @param vault[out] The ockam vault object to initialize with the default vault.
* @return error.
* @return an error, which should be freed using @ref ockam_vault_free_error.
*/
ockam_vault_extern_error_t ockam_vault_default_init(ockam_vault_t* vault);

Expand All @@ -65,7 +73,7 @@ ockam_vault_extern_error_t ockam_vault_default_init(ockam_vault_t* vault);
* @param input[in] Buffer containing data to run through SHA-256.
* @param input_length[in] Length of the data to run through SHA-256.
* @param digest[out] Buffer to place the resulting SHA-256 hash in. Must be 32 bytes.
* @return error.
* @return an error, which should be freed using @ref ockam_vault_free_error.
*/
ockam_vault_extern_error_t ockam_vault_sha256(ockam_vault_t vault,
const uint8_t* input,
Expand All @@ -78,7 +86,7 @@ ockam_vault_extern_error_t ockam_vault_sha256(ockam_vault_t vault,
* @param vault[in] Vault object to use for generating a secret key.
* @param secret[out] Pointer to an ockam secret object to be populated with a handle to the secret
* @param attributes[in] Desired attribtes for the secret to be generated.
* @return error.
* @return an error, which should be freed using @ref ockam_vault_free_error.
*/
ockam_vault_extern_error_t ockam_vault_secret_generate(ockam_vault_t vault,
ockam_vault_secret_t* secret,
Expand All @@ -91,7 +99,7 @@ ockam_vault_extern_error_t ockam_vault_secret_generate(ockam_vault_t
* @param attributes[in] Desired attributes for the secret being imported.
* @param input[in] Data to load into the supplied secret.
* @param input_length[in] Length of data to load into the secret.
* @return error.
* @return an error, which should be freed using @ref ockam_vault_free_error.
*/

ockam_vault_extern_error_t ockam_vault_secret_import(ockam_vault_t vault,
Expand All @@ -107,7 +115,7 @@ ockam_vault_extern_error_t ockam_vault_secret_import(ockam_vault_t
* @param output_buffer[out] Buffer to place the exported secret data in.
* @param output_buffer_size[in] Size of the output buffer.
* @param output_buffer_length[out] Amount of data placed in the output buffer.
* @return error.
* @return an error, which should be freed using @ref ockam_vault_free_error.
*/
ockam_vault_extern_error_t ockam_vault_secret_export(ockam_vault_t vault,
ockam_vault_secret_t secret,
Expand All @@ -122,7 +130,7 @@ ockam_vault_extern_error_t ockam_vault_secret_export(ockam_vault_t vault,
* @param output_buffer[out] Buffer to place the public key in.
* @param output_buffer_size[in] Size of the output buffer.
* @param output_buffer_length[out] Amount of data placed in the output buffer.
* @return error.
* @return an error, which should be freed using @ref ockam_vault_free_error.
*/
ockam_vault_extern_error_t ockam_vault_secret_publickey_get(ockam_vault_t vault,
ockam_vault_secret_t secret,
Expand All @@ -135,7 +143,7 @@ ockam_vault_extern_error_t ockam_vault_secret_publickey_get(ockam_vault_t
* @param vault[in] Vault object to use for retrieving ockam vault secret attributes.
* @param secret[in] Ockam vault secret to get attributes for.
* @param secret_attributes[out] Pointer to the attributes for the specified secret.
* @return error.
* @return an error, which should be freed using @ref ockam_vault_free_error.
*/
ockam_vault_extern_error_t ockam_vault_secret_attributes_get(ockam_vault_t vault,
uint64_t secret,
Expand All @@ -145,7 +153,7 @@ ockam_vault_extern_error_t ockam_vault_secret_attributes_get(ockam_vault_t
* @brief Delete an ockam vault secret.
* @param vault[in] Vault object to use for deleting the ockam vault secret.
* @param secret[in] Ockam vault secret to delete.
* @return error.
* @return an error, which should be freed using @ref ockam_vault_free_error.
*/
ockam_vault_extern_error_t ockam_vault_secret_destroy(ockam_vault_t vault, ockam_vault_secret_t secret);

Expand All @@ -157,7 +165,7 @@ ockam_vault_extern_error_t ockam_vault_secret_destroy(ockam_vault_t vault, ockam
* @param peer_publickey[in] Public key data to use for ECDH.
* @param peer_publickey_length[in] Length of the public key.
* @param shared_secret[out] Resulting shared secret from a sucessful ECDH operation. Invalid if ECDH failed.
* @return error.
* @return an error, which should be freed using @ref ockam_vault_free_error.
*/
ockam_vault_extern_error_t ockam_vault_ecdh(ockam_vault_t vault,
ockam_vault_secret_t privatekey,
Expand All @@ -173,7 +181,7 @@ ockam_vault_extern_error_t ockam_vault_ecdh(ockam_vault_t vault,
* @param derived_outputs_attributes[in] Attibutes of output secrets.
* @param derived_outputs_count[in] Length of outputs attributes array.
* @param derived_outputs[out] Array of ockam vault secrets resulting from HKDF.
* @return error.
* @return an error, which should be freed using @ref ockam_vault_free_error.
*/
ockam_vault_extern_error_t ockam_vault_hkdf_sha256(ockam_vault_t vault,
ockam_vault_secret_t salt,
Expand All @@ -194,7 +202,7 @@ ockam_vault_extern_error_t ockam_vault_hkdf_sha256(ockam_vault_t
* @param ciphertext_and_tag[in] Buffer containing the generated ciphertext and tag data.
* @param ciphertext_and_tag_size[in] Size of the ciphertext + tag buffer. Must be plaintext_size + 16.
* @param ciphertext_and_tag_length[out] Amount of data placed in the ciphertext + tag buffer.
* @return error.
* @return an error, which should be freed using @ref ockam_vault_free_error.
*/
ockam_vault_extern_error_t ockam_vault_aead_aes_gcm_encrypt(ockam_vault_t vault,
ockam_vault_secret_t key,
Expand All @@ -219,7 +227,7 @@ ockam_vault_extern_error_t ockam_vault_aead_aes_gcm_encrypt(ockam_vault_t
* @param plaintext[out] Buffer to place the decrypted data in.
* @param plaintext_size[in] Size of the plaintext buffer. Must be ciphertext_tag_size - 16.
* @param plaintext_length[out] Amount of data placed in the plaintext buffer.
* @return error.
* @return an error, which should be freed using @ref ockam_vault_free_error.
*/
ockam_vault_extern_error_t ockam_vault_aead_aes_gcm_decrypt(ockam_vault_t vault,
ockam_vault_secret_t key,
Expand All @@ -235,10 +243,15 @@ ockam_vault_extern_error_t ockam_vault_aead_aes_gcm_decrypt(ockam_vault_t
/**
* @brief Deinitialize the specified ockam vault object
* @param vault[in] The ockam vault object to deinitialize.
* @return error.
* @return an error, which should be freed using @ref ockam_vault_free_error.
*/
ockam_vault_extern_error_t ockam_vault_deinit(ockam_vault_t vault);

/**
* @brief Free any resources associated with a @ref ockam_vault_extern_error_t.
* @param error[in] the error to free.
*/
void ockam_vault_free_error(ockam_vault_extern_error_t *error);

#ifdef __cplusplus
} // extern "C"
Expand Down
13 changes: 13 additions & 0 deletions implementations/rust/ockam/ockam_ffi/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,9 @@ pub enum FfiError {

/// Ownership error.
OwnershipError,

/// Caught a panic (which would be UB if we let it unwind across the FFI).
UnexpectedPanic,
}

impl FfiError {
Expand All @@ -95,3 +98,13 @@ impl From<FfiError> for FfiOckamError {
Self::from(err)
}
}

/// # Safety
/// frees `FfiOckamError::domain` if it's non-null
#[no_mangle]
pub unsafe extern "C" fn ockam_vault_free_error(context: &mut FfiOckamError) {
if !context.domain.is_null() {
let _ = CString::from_raw(context.domain as *mut _);
context.domain = core::ptr::null();
}
}
4 changes: 2 additions & 2 deletions implementations/rust/ockam/ockam_ffi/src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@
macro_rules! check_buffer {
($buffer:expr) => {
if $buffer.is_null() {
return FfiError::InvalidParam.into();
return Err(FfiError::InvalidParam.into());
}
};
($buffer:expr, $length:expr) => {
if $buffer.is_null() || $length == 0 {
return FfiError::InvalidParam.into();
return Err(FfiError::InvalidParam.into());
}
};
}
Loading