On Wed, Jan 16, 2013 at 10:53 AM, Lucas De Marchi
<lucas.demarchi@xxxxxxxxxxxxxx> wrote:
> Hi Michal,
>
> On Wed, Jan 16, 2013 at 7:18 AM, Michal Marek <mmarek@xxxxxxx> wrote:
>> If the module is built with CONFIG_MODULE_SIG, add the the signer's
>> name, hexadecimal key id and hash algorithm to the list returned in
>> kmod_module_get_info(). The modinfo output then looks like this:
>>
>> filename:
>> /home/mmarek/kmod/testsuite/rootfs-pristine/test-modinfo/ext4-x86_64-sha256.ko
>> license: GPL
>> description: Fourth Extended Filesystem
>> author: Remy Card, Stephen Tweedie, Andrew Morton, Andreas Dilger, Theodore Ts'o and others
>> alias: ext3
>> alias: ext2
>> depends: mbcache,jbd2
>> intree: Y
>> vermagic: 3.7.0 SMP mod_unload
>> signer: Magrathea: Glacier signing key
>> sig_key: E3:C8:FC:A7:3F:B3:1D:DE:84:81:EF:38:E3:4C:DE:4B:0C:FD:1B:F9
>> sig_hashalgo: sha256
>>
>> The signature algorithm (RSA) and key identifier type (X509) are not
>> displayed, because they are constant information for every signed
>> module. But it would be trivial to add this. Note: No attempt is made at
>> verifying the signature, I don't think that modinfo is the right tool
>> for this.
>
> Ack. In general the patch looks good, but I have a few comments.
>
>
>> ---
>> Makefile.am | 3 +-
>> libkmod/libkmod-module.c | 40 ++++++++++++
>> libkmod/libkmod-private.h | 9 +++
>> libkmod/libkmod-signature.c | 141 +++++++++++++++++++++++++++++++++++++++++++
>> 4 files changed, 192 insertions(+), 1 deletion(-)
>> create mode 100644 libkmod/libkmod-signature.c
>>
>> diff --git a/Makefile.am b/Makefile.am
>> index 995f2de..9feaf96 100644
>> --- a/Makefile.am
>> +++ b/Makefile.am
>> @@ -62,7 +62,8 @@ libkmod_libkmod_la_SOURCES =\
>> libkmod/libkmod-index.h \
>> libkmod/libkmod-module.c \
>> libkmod/libkmod-file.c \
>> - libkmod/libkmod-elf.c
>> + libkmod/libkmod-elf.c \
>> + libkmod/libkmod-signature.c
>>
>> EXTRA_DIST += libkmod/libkmod.sym
>> EXTRA_DIST += libkmod/README libkmod/COPYING testsuite/COPYING COPYING
>> diff --git a/libkmod/libkmod-module.c b/libkmod/libkmod-module.c
>> index ae0d9c8..cf7b45f 100644
>> --- a/libkmod/libkmod-module.c
>> +++ b/libkmod/libkmod-module.c
>> @@ -2134,6 +2134,7 @@ KMOD_EXPORT int kmod_module_get_info(const struct kmod_module *mod, struct kmod_
>
> You forgot to update the docs. The doc says it gets the info from
> .modinfo section. This is not true for the signature, which is in the
> end of the module.
>
>> struct kmod_elf *elf;
>> char **strings;
>> int i, count, ret = -ENOMEM;
>> + struct kmod_signature_info sig_info;
>>
>> if (mod == NULL || list == NULL)
>> return -ENOENT;
>> @@ -2168,6 +2169,45 @@ KMOD_EXPORT int kmod_module_get_info(const struct kmod_module *mod, struct kmod_
>> if (n == NULL)
>> goto list_error;
>> }
>
> missing blank line here.
>
>> + if (kmod_module_signature_info(mod->file, &sig_info)) {
>> + struct kmod_list *n;
>> + char *key_hex;
>> +
>> + n = kmod_module_info_append(list, "signer", strlen("signer"),
>> + sig_info.signer, sig_info.signer_len);
>> + if (n == NULL)
>> + goto list_error;
>
> and here. Also I didn't notice in the first patch that you are freeing
> and setting *list to NULL on ENOMEM. Please add this after label
> list_error.
>
>> + count++;
>
> missing blank line here, too... so we group each key-val.
>
>> + /* Display the key id as 01:12:DE:AD:BE:EF:... */
>> + key_hex = malloc(sig_info.key_id_len * 3);
>> + if (key_hex == NULL) {
>> + kmod_module_info_free_list(*list);
>> + *list = NULL;
>> + goto list_error;
>
> As said, these lines should be after list_error
>
>> + }
>> + for (i = 0; i < (int)sig_info.key_id_len; i++) {
>> + sprintf(key_hex + i * 3, "%02X",
>> + (unsigned char)sig_info.key_id[i]);
>
> no need for this cast?
>
>> + if (i < (int)sig_info.key_id_len - 1)
>> + key_hex[i * 3 + 2] = ':';
>> + }
>> + n = kmod_module_info_append(list, "sig_key", strlen("sig_key"),
>> + key_hex, sig_info.key_id_len * 3 - 1);
>> + free(key_hex);
>> + if (n == NULL)
>> + goto list_error;
>> + count++;
>> + n = kmod_module_info_append(list,
>> + "sig_hashalgo", strlen("sig_hashalgo"),
>> + sig_info.hash_algo, strlen(sig_info.hash_algo));
>> + if (n == NULL)
>> + goto list_error;
>> + count++;
>> + /*
>> + * Omit sig_info.id_type and sig_info.algo for now, as these
>> + * are currently constant.
>> + */
>> + }
>> ret = count;
>>
>> list_error:
>> diff --git a/libkmod/libkmod-private.h b/libkmod/libkmod-private.h
>> index 4760733..c2f80e7 100644
>> --- a/libkmod/libkmod-private.h
>> +++ b/libkmod/libkmod-private.h
>> @@ -168,5 +168,14 @@ int kmod_elf_strip_vermagic(struct kmod_elf *elf) _must_check_ __attribute__((no
>> */
>> int kmod_elf_get_section(const struct kmod_elf *elf, const char *section, const void **buf, uint64_t *buf_size) _must_check_ __attribute__((nonnull(1,2,3,4)));
>>
>> +/* libkmod-signature.c */
>> +struct kmod_signature_info {
>> + const char *signer;
>> + size_t signer_len;
>> + const char *key_id;
>> + size_t key_id_len;
>> + const char *algo, *hash_algo, *id_type;
>> +};
>> +int kmod_module_signature_info(const struct kmod_file *file, struct kmod_signature_info *sig_info) _must_check_ __attribute__((nonnull(1, 2)));
>> /* util functions */
>> #include "libkmod-util.h"
>> diff --git a/libkmod/libkmod-signature.c b/libkmod/libkmod-signature.c
>> new file mode 100644
>> index 0000000..1ffe4e4
>> --- /dev/null
>> +++ b/libkmod/libkmod-signature.c
>> @@ -0,0 +1,141 @@
>> +/*
>> + * libkmod - module signature display
>> + *
>> + * Copyright (C) 2013 Michal Marek, SUSE
>> + *
>> + * This library is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU Lesser General Public
>> + * License as published by the Free Software Foundation; either
>> + * version 2.1 of the License, or (at your option) any later version.
>> + *
>> + * This library is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>> + * Lesser General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU Lesser General Public
>> + * License along with this library; if not, write to the Free Software
>> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
>> + */
>> +
>> +#include <endian.h>
>> +#include <stdint.h>
>> +#include <stdlib.h>
>> +#include <string.h>
>> +#include <stdio.h>
>> +
>> +#include "libkmod-private.h"
>> +
>> +/* These types and tables were copied from the 3.7 kernel sources.
>> + * As this is just description of the signature format, it should not be
>> + * considered derived work (so libkmod can use the LGPL license).
>> + */
>> +enum pkey_algo {
>> + PKEY_ALGO_DSA,
>> + PKEY_ALGO_RSA,
>> + PKEY_ALGO__LAST
>> +};
>> +
>> +static const char *const pkey_algo[PKEY_ALGO__LAST] = {
>> + [PKEY_ALGO_DSA] = "DSA",
>> + [PKEY_ALGO_RSA] = "RSA",
>> +};
>> +
>> +enum pkey_hash_algo {
>> + PKEY_HASH_MD4,
>> + PKEY_HASH_MD5,
>> + PKEY_HASH_SHA1,
>> + PKEY_HASH_RIPE_MD_160,
>> + PKEY_HASH_SHA256,
>> + PKEY_HASH_SHA384,
>> + PKEY_HASH_SHA512,
>> + PKEY_HASH_SHA224,
>> + PKEY_HASH__LAST
>> +};
>> +
>> +const char *const pkey_hash_algo[PKEY_HASH__LAST] = {
>> + [PKEY_HASH_MD4] = "md4",
>> + [PKEY_HASH_MD5] = "md5",
>> + [PKEY_HASH_SHA1] = "sha1",
>> + [PKEY_HASH_RIPE_MD_160] = "rmd160",
>> + [PKEY_HASH_SHA256] = "sha256",
>> + [PKEY_HASH_SHA384] = "sha384",
>> + [PKEY_HASH_SHA512] = "sha512",
>> + [PKEY_HASH_SHA224] = "sha224",
>> +};
>> +
>> +enum pkey_id_type {
>> + PKEY_ID_PGP, /* OpenPGP generated key ID */
>> + PKEY_ID_X509, /* X.509 arbitrary subjectKeyIdentifier */
>> + PKEY_ID_TYPE__LAST
>> +};
>> +
>> +const char *const pkey_id_type[PKEY_ID_TYPE__LAST] = {
>> + [PKEY_ID_PGP] = "PGP",
>> + [PKEY_ID_X509] = "X509",
>> +};
>> +
>> +/*
>> + * Module signature information block.
>> + *
>> + * The constituents of the signature section are, in order:
>> + *
>> + * - Signer's name
>> + * - Key identifier
>> + * - Signature data
>> + * - Information block
>> + */
>> +struct module_signature {
>> + uint8_t algo; /* Public-key crypto algorithm [enum pkey_algo] */
>> + uint8_t hash; /* Digest algorithm [enum pkey_hash_algo] */
>> + uint8_t id_type; /* Key identifier type [enum pkey_id_type] */
>> + uint8_t signer_len; /* Length of signer's name */
>> + uint8_t key_id_len; /* Length of key identifier */
>> + uint8_t __pad[3];
>> + uint32_t sig_len; /* Length of signature data (big endian) */
>> +};
>> +
>> +#define SIG_MAGIC "~Module signature appended~\n"
>> +
>> +int kmod_module_signature_info(const struct kmod_file *file, struct kmod_signature_info *sig_info)
>
> please make this bool since we only return true or false.
>
>> +{
>> + const char *mem;
>> + off_t size;
>> + const struct module_signature *modsig;
>> + size_t sig_len;
>> +
>> +
>> + size = kmod_file_get_size(file);
>> + mem = kmod_file_get_contents(file);
>> + if (size < (off_t)strlen(SIG_MAGIC))
>> + return 0;
>> + size -= strlen(SIG_MAGIC);
>> + if (memcmp(SIG_MAGIC, mem + size, strlen(SIG_MAGIC)) != 0)
>> + return 0;
>> +
>> + if (size < (off_t)sizeof(struct module_signature))
>> + return 0;
>> + size -= sizeof(struct module_signature);
>> + modsig = (struct module_signature *)(mem + size);
>> + if (modsig->algo >= PKEY_ALGO__LAST ||
>> + modsig->hash >= PKEY_HASH__LAST ||
>> + modsig->id_type >= PKEY_ID_TYPE__LAST)
>> + return 0;
>> + sig_len = be32toh(modsig->sig_len);
>> + if (size < (off_t)(modsig->signer_len + modsig->key_id_len + sig_len))
>> + return 0;
>> +
>> + size -= modsig->key_id_len + sig_len;
>> + sig_info->key_id = mem + size;
>> + sig_info->key_id_len = modsig->key_id_len;
>> +
>> + size -= modsig->signer_len;
>> + sig_info->signer = mem + size;
>> + sig_info->signer_len = modsig->signer_len;
>> +
>> + sig_info->algo = pkey_algo[modsig->algo];
>> + sig_info->hash_algo = pkey_hash_algo[modsig->hash];
>> + sig_info->id_type = pkey_id_type[modsig->id_type];
>> +
>> + return 1;
>> +}
>> --
>> 1.7.10.4
>
> Otherwise it looks good.
ohh... I and forgot to say. Please add a test under testsuite:
testsuite/test-modinfo.c adding the module and correct output to
rootfs-pristine.
cheers
Lucas de Marchi
--
To unsubscribe from this list: send the line "unsubscribe linux-modules" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
