[PATCH RFC v2 3/5] lib: Add support for ECDSA image signing
Simon Glass
sjg at chromium.org
Thu Jan 7 13:35:36 CET 2021
Hi Alexandru,
On Wed, 30 Dec 2020 at 14:00, Alexandru Gagniuc <mr.nuke.me at gmail.com> wrote:
>
> mkimage supports rsa2048, and rsa4096 signatures. With newer silicon
> now supporting hardware-accelerated ECDSA, it makes sense to expand
> signing support to elliptic curves.
>
> Implement host-side ECDSA signing and verification with libcrypto.
> Device-side implementation of signature verification is beyond the
> scope of this patch.
>
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me at gmail.com>
> ---
> common/image-sig.c | 14 +-
> include/u-boot/ecdsa.h | 27 ++++
> lib/ecdsa/ecdsa-libcrypto.c | 300 ++++++++++++++++++++++++++++++++++++
> tools/Makefile | 3 +
> 4 files changed, 342 insertions(+), 2 deletions(-)
> create mode 100644 include/u-boot/ecdsa.h
> create mode 100644 lib/ecdsa/ecdsa-libcrypto.c
>
> diff --git a/common/image-sig.c b/common/image-sig.c
> index 21dafe6b91..2548b3eb0f 100644
> --- a/common/image-sig.c
> +++ b/common/image-sig.c
> @@ -15,6 +15,7 @@
> DECLARE_GLOBAL_DATA_PTR;
> #endif /* !USE_HOSTCC*/
> #include <image.h>
> +#include <u-boot/ecdsa.h>
> #include <u-boot/rsa.h>
> #include <u-boot/hash-checksum.h>
>
> @@ -82,8 +83,17 @@ struct crypto_algo crypto_algos[] = {
> .sign = rsa_sign,
> .add_verify_data = rsa_add_verify_data,
> .verify = rsa_verify,
> - }
> -
> + },
> +#if defined(USE_HOSTCC)
> + /* Currently, only host support exists for ECDSA */
> + {
> + .name = "ecdsa256",
> + .key_len = ECDSA256_BYTES,
> + .sign = ecdsa_sign,
> + .add_verify_data = ecdsa_add_verify_data,
> + .verify = ecdsa_verify,
> + },
> +#endif
> };
>
> struct padding_algo padding_algos[] = {
> diff --git a/include/u-boot/ecdsa.h b/include/u-boot/ecdsa.h
> new file mode 100644
> index 0000000000..a13a7267e1
> --- /dev/null
> +++ b/include/u-boot/ecdsa.h
> @@ -0,0 +1,27 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright (c) 2020, Alexandru Gagniuc <mr.nuke.me at gmail.com>.
> + */
> +
> +#ifndef _ECDSA_H
> +#define _ECDSA_H
> +
> +#include <errno.h>
> +#include <image.h>
> +
> +/**
> + * crypto_algo API impementation for ECDSA;
> + * @see "struct crypto_algo"
> + * @{
> + */
> +int ecdsa_sign(struct image_sign_info *info, const struct image_region region[],
> + int region_count, uint8_t **sigp, uint *sig_len);
> +int ecdsa_verify(struct image_sign_info *info,
> + const struct image_region region[], int region_count,
> + uint8_t *sig, uint sig_len);
> +int ecdsa_add_verify_data(struct image_sign_info *info, void *keydest);
Please always add full comments when you export functions, or have a
non-trivial static function.
> +/** @} */
> +
> +#define ECDSA256_BYTES (256 / 8)
> +
> +#endif
> diff --git a/lib/ecdsa/ecdsa-libcrypto.c b/lib/ecdsa/ecdsa-libcrypto.c
> new file mode 100644
> index 0000000000..ff491411d0
> --- /dev/null
> +++ b/lib/ecdsa/ecdsa-libcrypto.c
> @@ -0,0 +1,300 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * ECDSA image signing implementation using libcrypto backend
> + *
> + * The signature is a binary representation of the (R, S) points, padded to the
> + * key size. The signature will be (2 * key_size_bits) / 8 bytes.
> + *
> + * Deviations from behavior of RSA equivalent:
> + * - Verification uses private key. This is not technically required, but a
> + * limitation on how clumsy the openssl API is to use.
I'm not quite sure what the implications are on this. If this is
public key crypto, the private key is not available, so how can you
verify with it?
> + * - Handling of keys and key paths:
> + * - The '-K' key directory option must contain path to the key file,
> + * instead of the key directory.
If we make this change we should update RSA to do the same.
> + * - No assumptions are made about the file extension of the key
> + * - The 'key-name-hint' property is only used for naming devicetree nodes,
> + * but is not used for looking up keys on the filesystem.
> + *
> + * Copyright (c) 2020, Alexandru Gagniuc <mr.nuke.me at gmail.com>
> + */
> +
> +#include <u-boot/ecdsa.h>
> +#include <u-boot/fdt-libcrypto.h>
> +#include <openssl/ssl.h>
> +#include <openssl/ec.h>
> +#include <openssl/bn.h>
> +
> +struct signer {
> + EVP_PKEY *evp_key;
> + EC_KEY *ecdsa_key;
> + void *hash;
> + void *signature; /* Do not free() !*/
need comments
> +};
> +
> +static int alloc_ctx(struct signer *ctx, const struct image_sign_info *info)
> +{
> + memset(ctx, 0, sizeof(*ctx));
> +
> + if (!OPENSSL_init_ssl(0, NULL)) {
> + fprintf(stderr, "Failure to init SSL library\n");
> + return -1;
> + }
> +
> + ctx->hash = malloc(info->checksum->checksum_len);
> + ctx->signature = malloc(info->crypto->key_len * 2);
> +
> + if (!ctx->hash || !ctx->signature)
> + return -1;
> +
> + return 0;
> +}
> +
> +static void free_ctx(struct signer *ctx)
> +{
> + if (ctx->ecdsa_key)
> + EC_KEY_free(ctx->ecdsa_key);
> +
> + if (ctx->evp_key)
> + EVP_PKEY_free(ctx->evp_key);
> +
> + if (ctx->hash)
> + free(ctx->hash);
> +}
> +
> +/*
> + * Convert an ECDSA signature to raw format
> + *
> + * openssl DER-encodes 'binary' signatures. We want the signature in a raw
> + * (R, S) point pair. So we have to dance a bit.
> + */
> +static void ecdsa_sig_encode_raw(void *buf, const ECDSA_SIG *sig, size_t order)
> +{
> + int point_bytes = order;
> + const BIGNUM *r, *s;
> + uintptr_t s_buf;
> +
> + ECDSA_SIG_get0(sig, &r, &s);
> + s_buf = (uintptr_t)buf + point_bytes;
> + BN_bn2binpad(r, buf, point_bytes);
> + BN_bn2binpad(s, (void *)s_buf, point_bytes);
> +}
> +
> +static ECDSA_SIG *ecdsa_sig_from_raw(void *buf, size_t order)
> +{
> + int point_bytes = order;
> + uintptr_t s_buf;
> + ECDSA_SIG *sig;
> + BIGNUM *r, *s;
> +
> + sig = ECDSA_SIG_new();
> + if (!sig)
> + return NULL;
> +
> + s_buf = (uintptr_t)buf + point_bytes;
> + r = BN_bin2bn(buf, point_bytes, NULL);
> + s = BN_bin2bn((void *)s_buf, point_bytes, NULL);
> + ECDSA_SIG_set0(sig, r, s);
> +
> + return sig;
> +}
> +
> +static size_t ecdsa_key_size_bytes(const EC_KEY *key)
I think all of these functions need a comment.
> +{
> + const EC_GROUP *group;
> +
> + group = EC_KEY_get0_group(key);
> + return EC_GROUP_order_bits(group) / 8;
> +}
> +
> +static int read_key(struct signer *ctx, const char *key_name)
> +{
> + FILE *f = fopen(key_name, "r");
> +
> + if (!f) {
> + fprintf(stderr, "Can not get key file '%s'\n", key_name);
> + return -1;
return -EIO perhaps?
> + }
> +
> + ctx->evp_key = PEM_read_PrivateKey(f, NULL, NULL, NULL);
> + fclose(f);
> + if (!ctx->evp_key) {
> + fprintf(stderr, "Can not read key from '%s'\n", key_name);
> + return -1;
These should return useful error number: -1 is -EPERM which doesn't
seem right here.
> + }
> +
> + if (EVP_PKEY_id(ctx->evp_key) != EVP_PKEY_EC) {
> + fprintf(stderr, "'%s' is not an ECDSA key\n", key_name);
> + return -1;
> + }
> +
> + ctx->ecdsa_key = EVP_PKEY_get1_EC_KEY(ctx->evp_key);
> + if (!ctx->ecdsa_key)
> + fprintf(stderr, "Can not extract ECDSA key\n");
> +
> + return (ctx->ecdsa_key) ? 0 : -1;
> +}
> +
> +static int prepare_ctx(struct signer *ctx, const struct image_sign_info *info)
> +{
> + const char *kname = info->keydir;
> + int key_len_bytes;
> +
> + if (alloc_ctx(ctx, info) < 0)
That function returns 0 on success, so you don't need the < 0. A
comment on the above function would make that clear.
> + return -1;
> +
> + if (read_key(ctx, kname) < 0)
> + return -1;
> +
> + key_len_bytes = ecdsa_key_size_bytes(ctx->ecdsa_key);
> + if (key_len_bytes != info->crypto->key_len) {
> + fprintf(stderr, "Expected a %u-bit key, got %u-bit key\n",
> + info->crypto->key_len * 8, key_len_bytes * 8);
> + return -1;
> + }
> +
> + return 0;
> +}
> +
[..]
Regards,
Simon
More information about the U-Boot
mailing list