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.

Sign up for GitHub

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

crypto: Add Galois/Counter Mode (GCM) support #22407

Merged
merged 3 commits into from
Feb 8, 2020
Merged

crypto: Add Galois/Counter Mode (GCM) support #22407

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c0b306a
include: crypto: Add Galois/Counter Mode (GCM) support
mrfuchs Feb 2, 2020
c689ec2
drivers: crypto: crypto_mtls_shim: Add AES-GCM support
mrfuchs Feb 2, 2020
86e6989
samples: crypto: Add AES-GCM sample
mrfuchs Feb 2, 2020
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
110 changes: 107 additions & 3 deletions drivers/crypto/crypto_mtls_shim.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@
#endif /* CONFIG_MBEDTLS_CFG_FILE */

#include <mbedtls/ccm.h>
#ifdef CONFIG_MBEDTLS_CIPHER_MODE_GCM_ENABLED
#include <mbedtls/gcm.h>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this header available when mbedTLS is built without gcm support ? Shouldn't you put this include under ifdef ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it's available - but you are right, it's probably better to include gcm.h conditionally. That would mean that the mbedtls_gcm_context in the session would also have to be surrounded by an ifdef, though. Will change it.

#endif
#include <mbedtls/aes.h>

#define MTLS_SUPPORT (CAP_RAW_KEY | CAP_SEPARATE_IO_BUFS | CAP_SYNC_OPS | \
Expand All @@ -33,6 +36,9 @@ LOG_MODULE_REGISTER(mbedtls);
struct mtls_shim_session {
union {
mbedtls_ccm_context mtls_ccm;
#ifdef CONFIG_MBEDTLS_CIPHER_MODE_GCM_ENABLED
mbedtls_gcm_context mtls_gcm;
#endif
mbedtls_aes_context mtls_aes;
};
bool in_use;
Expand Down Expand Up @@ -203,7 +209,12 @@ static int mtls_ccm_decrypt_auth(struct cipher_ctx *ctx,
apkt->pkt->out_buf, apkt->tag,
ctx->mode_params.ccm_info.tag_len);
if (ret) {
LOG_ERR("Could non decrypt/auth (%d)", ret);
if (ret == MBEDTLS_ERR_CCM_AUTH_FAILED) {
LOG_ERR("Message authentication failed");
return -EFAULT;
}

LOG_ERR("Could not decrypt/auth (%d)", ret);

/*ToDo: try to return relevant code depending on ret? */
return -EINVAL;
Expand All @@ -215,6 +226,66 @@ static int mtls_ccm_decrypt_auth(struct cipher_ctx *ctx,
return 0;
}

#ifdef CONFIG_MBEDTLS_CIPHER_MODE_GCM_ENABLED
static int mtls_gcm_encrypt_auth(struct cipher_ctx *ctx,
struct cipher_aead_pkt *apkt,
u8_t *nonce)
{
mbedtls_gcm_context *mtls_ctx = MTLS_GET_CTX(ctx, gcm);
int ret;

ret = mbedtls_gcm_crypt_and_tag(mtls_ctx, MBEDTLS_GCM_ENCRYPT,
apkt->pkt->in_len, nonce,
ctx->mode_params.gcm_info.nonce_len,
apkt->ad, apkt->ad_len,
apkt->pkt->in_buf,
apkt->pkt->out_buf,
ctx->mode_params.gcm_info.tag_len,
apkt->tag);
if (ret) {
LOG_ERR("Could not encrypt/auth (%d)", ret);

return -EINVAL;
}

/* This is equivalent to what is done in mtls_ccm_encrypt_auth(). */
apkt->pkt->out_len = apkt->pkt->in_len;
apkt->pkt->out_len += ctx->mode_params.gcm_info.tag_len;

return 0;
}

static int mtls_gcm_decrypt_auth(struct cipher_ctx *ctx,
struct cipher_aead_pkt *apkt,
u8_t *nonce)
{
mbedtls_gcm_context *mtls_ctx = MTLS_GET_CTX(ctx, gcm);
int ret;

ret = mbedtls_gcm_auth_decrypt(mtls_ctx, apkt->pkt->in_len, nonce,
ctx->mode_params.gcm_info.nonce_len,
apkt->ad, apkt->ad_len,
apkt->tag,
ctx->mode_params.gcm_info.tag_len,
apkt->pkt->in_buf,
apkt->pkt->out_buf);
if (ret) {
if (ret == MBEDTLS_ERR_GCM_AUTH_FAILED) {
LOG_ERR("Message authentication failed");
return -EFAULT;
}

LOG_ERR("Could not decrypt/auth (%d)", ret);
return -EINVAL;
}

apkt->pkt->out_len = apkt->pkt->in_len;
apkt->pkt->out_len += ctx->mode_params.gcm_info.tag_len;

return 0;
}
#endif /* CONFIG_MBEDTLS_CIPHER_MODE_GCM_ENABLED */

static int mtls_get_unused_session_index(void)
{
int i;
Expand All @@ -233,8 +304,11 @@ static int mtls_session_setup(struct device *dev, struct cipher_ctx *ctx,
enum cipher_algo algo, enum cipher_mode mode,
enum cipher_op op_type)
{
mbedtls_ccm_context *ccm_ctx;
mbedtls_aes_context *aes_ctx;
mbedtls_ccm_context *ccm_ctx;
#ifdef CONFIG_MBEDTLS_CIPHER_MODE_GCM_ENABLED
mbedtls_gcm_context *gcm_ctx;
#endif
int ctx_idx;
int ret;

Expand All @@ -250,6 +324,9 @@ static int mtls_session_setup(struct device *dev, struct cipher_ctx *ctx,

if (mode != CRYPTO_CIPHER_MODE_CCM &&
mode != CRYPTO_CIPHER_MODE_CBC &&
#ifdef CONFIG_MBEDTLS_CIPHER_MODE_GCM_ENABLED
mode != CRYPTO_CIPHER_MODE_GCM &&
#endif
mode != CRYPTO_CIPHER_MODE_ECB) {
LOG_ERR("Unsupported mode");
return -EINVAL;
Expand Down Expand Up @@ -314,7 +391,7 @@ static int mtls_session_setup(struct device *dev, struct cipher_ctx *ctx,
ret = mbedtls_ccm_setkey(ccm_ctx, MBEDTLS_CIPHER_ID_AES,
ctx->key.bit_stream, ctx->keylen * 8U);
if (ret) {
LOG_ERR("Could not setup the key (%d)", ret);
LOG_ERR("AES_CCM: failed at setkey (%d)", ret);
mtls_sessions[ctx_idx].in_use = false;

return -EINVAL;
Expand All @@ -325,6 +402,29 @@ static int mtls_session_setup(struct device *dev, struct cipher_ctx *ctx,
ctx->ops.ccm_crypt_hndlr = mtls_ccm_decrypt_auth;
}
break;
#ifdef CONFIG_MBEDTLS_CIPHER_MODE_GCM_ENABLED
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a test for CRYPTO_CIPHER_MODE_GCM is already done i L423, which means if CONFIG_MBEDTLS_CIPHER_MODE_GCM_ENABLED=n then code never reaches here when mode is CRYPTO_CIPHER_MODE_GCM, so any reason not to include the case CRYPTO_CIPHER_MODE_GCM: inside the #ifdef ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay. Should we add a default case to the switch statement then? I don't know, if all compilers and static code analysis tools will accept the previous mode check without complaining about unhandled case values.

Copy link
Contributor Author

@mrfuchs mrfuchs Feb 4, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has been changed. As GCC issued a

warning: enumeration value 'CRYPTO_CIPHER_MODE_GCM' not handled in switch

I decided to add a default case. It also seems to be more future-proof this way when new modes are added to the Crypto API.

case CRYPTO_CIPHER_MODE_GCM:
gcm_ctx = &mtls_sessions[ctx_idx].mtls_gcm;
mbedtls_gcm_init(gcm_ctx);
ret = mbedtls_gcm_setkey(gcm_ctx, MBEDTLS_CIPHER_ID_AES,
ctx->key.bit_stream, ctx->keylen * 8U);
if (ret) {
LOG_ERR("AES_GCM: failed at setkey (%d)", ret);
mtls_sessions[ctx_idx].in_use = false;

return -EINVAL;
}
if (op_type == CRYPTO_CIPHER_OP_ENCRYPT) {
ctx->ops.gcm_crypt_hndlr = mtls_gcm_encrypt_auth;
} else {
ctx->ops.gcm_crypt_hndlr = mtls_gcm_decrypt_auth;
}
break;
#endif /* CONFIG_MBEDTLS_CIPHER_MODE_GCM_ENABLED */
default:
LOG_ERR("Unhandled mode");
mtls_sessions[ctx_idx].in_use = false;
return -EINVAL;
}

mtls_sessions[ctx_idx].mode = mode;
Expand All @@ -340,6 +440,10 @@ static int mtls_session_free(struct device *dev, struct cipher_ctx *ctx)

if (mtls_session->mode == CRYPTO_CIPHER_MODE_CCM) {
mbedtls_ccm_free(&mtls_session->mtls_ccm);
#ifdef CONFIG_MBEDTLS_CIPHER_MODE_GCM_ENABLED
} else if (mtls_session->mode == CRYPTO_CIPHER_MODE_GCM) {
mbedtls_gcm_free(&mtls_session->mtls_gcm);
#endif
} else {
mbedtls_aes_free(&mtls_session->mtls_aes);
}
Expand Down
22 changes: 22 additions & 0 deletions include/crypto/cipher.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,4 +268,26 @@ static inline int cipher_ccm_op(struct cipher_ctx *ctx,
return ctx->ops.ccm_crypt_hndlr(ctx, pkt, nonce);
}

/*
* @brief Perform Galois/Counter Mode (GCM) crypto operation
*
* @param[in] ctx Pointer to the crypto context of this op.
* @param[in/out] pkt Structure holding the input/output, Associated
* Data (AD) and auth tag buffer pointers.
* @param[in] nonce Nonce for the operation. Same nonce value should not
* be reused across multiple operations (within a
* session context) for security.
*
* @return 0 on success, negative errno code on fail.
*/
static inline int cipher_gcm_op(struct cipher_ctx *ctx,
struct cipher_aead_pkt *pkt, u8_t *nonce)
{
__ASSERT(ctx->ops.cipher_mode == CRYPTO_CIPHER_MODE_GCM, "GCM mode "
"session invoking a different mode handler");

pkt->pkt->ctx = ctx;
return ctx->ops.gcm_crypt_hndlr(ctx, pkt, nonce);
}

#endif /* ZEPHYR_INCLUDE_CRYPTO_CIPHER_H_ */
11 changes: 11 additions & 0 deletions include/crypto/cipher_structs.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ enum cipher_mode {
CRYPTO_CIPHER_MODE_CBC = 2,
CRYPTO_CIPHER_MODE_CTR = 3,
CRYPTO_CIPHER_MODE_CCM = 4,
CRYPTO_CIPHER_MODE_GCM = 5,
};

/* Forward declarations */
Expand All @@ -59,6 +60,9 @@ typedef int (*ctr_op_t)(struct cipher_ctx *ctx, struct cipher_pkt *pkt,
typedef int (*ccm_op_t)(struct cipher_ctx *ctx, struct cipher_aead_pkt *pkt,
u8_t *nonce);

typedef int (*gcm_op_t)(struct cipher_ctx *ctx, struct cipher_aead_pkt *pkt,
u8_t *nonce);

struct cipher_ops {

enum cipher_mode cipher_mode;
Expand All @@ -68,6 +72,7 @@ struct cipher_ops {
cbc_op_t cbc_crypt_hndlr;
ctr_op_t ctr_crypt_hndlr;
ccm_op_t ccm_crypt_hndlr;
gcm_op_t gcm_crypt_hndlr;
};
};

Expand All @@ -83,6 +88,11 @@ struct ctr_params {
u32_t ctr_len;
};

struct gcm_params {
u16_t tag_len;
u16_t nonce_len;
};

/* Structure encoding session parameters. Refer to comments for individual
* fields to know the contract in terms of who fills what and when w.r.t
* begin_session() call.
Expand Down Expand Up @@ -132,6 +142,7 @@ struct cipher_ctx {
union {
struct ccm_params ccm_info;
struct ctr_params ctr_info;
struct gcm_params gcm_info;
} mode_params;

/* Cryptographic keylength in bytes. To be populated by the app
Expand Down
1 change: 1 addition & 0 deletions samples/drivers/crypto/prj_mtls_shim.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ CONFIG_MBEDTLS_BUILTIN=y
CONFIG_MBEDTLS_CFG_FILE="config-tls-generic.h"
CONFIG_MBEDTLS_HEAP_SIZE=512
CONFIG_MBEDTLS_CIPHER_CCM_ENABLED=y
CONFIG_MBEDTLS_CIPHER_MODE_GCM_ENABLED=y
CONFIG_MAIN_STACK_SIZE=4096

CONFIG_CRYPTO=y
Expand Down
2 changes: 2 additions & 0 deletions samples/drivers/crypto/sample.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ tests:
- ".*: CBC Mode"
- ".*: CTR Mode"
- ".*: CCM Mode"
- ".*: GCM Mode"
sample.drivers.crypto.mbedtls.micro:
tags: crypto
harness_config:
Expand All @@ -27,6 +28,7 @@ tests:
- ".*: CBC Mode"
- ".*: CTR Mode"
- ".*: CCM Mode"
- ".*: GCM Mode"
sample.drivers.crypto.stm32:
tags: crypto
platform_whitelist: mikroe_mini_m4_for_stm32
Expand Down
Loading