| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <D44CWVY4FZ00.2C7VG1HOAMQLD@kernel.org>
Date: Thu, 12 Sep 2024 16:57:34 +0300
From: "Jarkko Sakkinen" <jarkko@...nel.org>
To: "Roberto Sassu" <roberto.sassu@...weicloud.com>, <dhowells@...hat.com>,
<dwmw2@...radead.org>, <herbert@...dor.apana.org.au>, <davem@...emloft.net>
Cc: <linux-kernel@...r.kernel.org>, <keyrings@...r.kernel.org>,
<linux-crypto@...r.kernel.org>, <zohar@...ux.ibm.com>,
<linux-integrity@...r.kernel.org>, <torvalds@...ux-foundation.org>,
"Roberto Sassu" <roberto.sassu@...wei.com>
Subject: Re: [PATCH v3 04/14] PGPLIB: Basic packet parser
On Wed Sep 11, 2024 at 3:29 PM EEST, Roberto Sassu wrote:
> From: David Howells <dhowells@...hat.com>
>
> Provide a simple parser that extracts the packets from a PGP packet blob
> and passes the desirous ones to the given processor function:
>
> struct pgp_parse_context {
> u64 types_of_interest;
> int (*process_packet)(struct pgp_parse_context *context,
> enum pgp_packet_tag type,
> u8 headerlen,
> const u8 *data,
> size_t datalen);
> };
>
> int pgp_parse_packets(const u8 *data, size_t datalen,
> struct pgp_parse_context *ctx);
>
> This is configured on with CONFIG_PGP_LIBRARY.
>
> Signed-off-by: David Howells <dhowells@...hat.com>
> Co-developed-by: Roberto Sassu <roberto.sassu@...wei.com>
> Signed-off-by: Roberto Sassu <roberto.sassu@...wei.com>
> ---
> crypto/asymmetric_keys/Kconfig | 6 +
> crypto/asymmetric_keys/Makefile | 5 +
> crypto/asymmetric_keys/pgp_library.c | 262 +++++++++++++++++++++++++++
> crypto/asymmetric_keys/pgplib.h | 33 ++++
> 4 files changed, 306 insertions(+)
> create mode 100644 crypto/asymmetric_keys/pgp_library.c
> create mode 100644 crypto/asymmetric_keys/pgplib.h
>
> diff --git a/crypto/asymmetric_keys/Kconfig b/crypto/asymmetric_keys/Kconfig
> index e1345b8f39f1..8215e3fcd8db 100644
> --- a/crypto/asymmetric_keys/Kconfig
> +++ b/crypto/asymmetric_keys/Kconfig
> @@ -103,4 +103,10 @@ config FIPS_SIGNATURE_SELFTEST_ECDSA
> depends on CRYPTO_SHA256=y || CRYPTO_SHA256=FIPS_SIGNATURE_SELFTEST
> depends on CRYPTO_ECDSA=y || CRYPTO_ECDSA=FIPS_SIGNATURE_SELFTEST
>
> +config PGP_LIBRARY
> + tristate "PGP parsing library"
> + help
> + This option enables a library that provides a number of simple
> + utility functions for parsing PGP (RFC 9580) packet-based messages.
> +
> endif # ASYMMETRIC_KEY_TYPE
> diff --git a/crypto/asymmetric_keys/Makefile b/crypto/asymmetric_keys/Makefile
> index bc65d3b98dcb..055b28207111 100644
> --- a/crypto/asymmetric_keys/Makefile
> +++ b/crypto/asymmetric_keys/Makefile
> @@ -79,3 +79,8 @@ verify_signed_pefile-y := \
>
> $(obj)/mscode_parser.o: $(obj)/mscode.asn1.h $(obj)/mscode.asn1.h
> $(obj)/mscode.asn1.o: $(obj)/mscode.asn1.c $(obj)/mscode.asn1.h
> +
> +#
> +# PGP handling
> +#
> +obj-$(CONFIG_PGP_LIBRARY) += pgp_library.o
> diff --git a/crypto/asymmetric_keys/pgp_library.c b/crypto/asymmetric_keys/pgp_library.c
> new file mode 100644
> index 000000000000..1b87f8af411b
> --- /dev/null
> +++ b/crypto/asymmetric_keys/pgp_library.c
> @@ -0,0 +1,262 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* PGP packet parser (RFC 9580)
/*
* PGP packet parser (RFC 9580)
> + *
> + * Copyright (C) 2011 Red Hat, Inc. All Rights Reserved.
> + * Written by David Howells (dhowells@...hat.com)
> + */
> +
> +#define pr_fmt(fmt) "PGPL: "fmt
> +#include <linux/errno.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +
> +#include "pgplib.h"
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("PGP library");
> +
> +const char *const pgp_hash_algorithms[PGP_HASH__LAST] = {
> + [PGP_HASH_MD5] = "md5",
> + [PGP_HASH_SHA1] = "sha1",
> + [PGP_HASH_RIPE_MD_160] = "rmd160",
> + [PGP_HASH_SHA256] = "sha256",
> + [PGP_HASH_SHA384] = "sha384",
> + [PGP_HASH_SHA512] = "sha512",
> + [PGP_HASH_SHA224] = "sha224",
> + [PGP_HASH_SHA3_256] = "sha3-256",
> + [PGP_HASH_SHA3_512] = "sha3-512",
> +};
> +EXPORT_SYMBOL_GPL(pgp_hash_algorithms);
> +
> +/**
> + * pgp_parse_packet_header - Parse a PGP packet header
> + * @_data: Start of the PGP packet (updated to PGP packet data)
> + * @_datalen: Amount of data remaining in buffer (decreased)
> + * @_type: Where the packet type will be returned
> + * @_headerlen: Where the header length will be returned
> + *
> + * Parse a set of PGP packet header [RFC 9580: 4.2].
> + *
> + * Return: Packet data size on success; non-zero on error. If successful,
> + * *_data and *_datalen will have been updated and *_headerlen will be set to
> + * hold the length of the packet header.
> + */
> +static ssize_t pgp_parse_packet_header(const u8 **_data, size_t *_datalen,
> + enum pgp_packet_tag *_type,
> + u8 *_headerlen)
> +{
> + enum pgp_packet_tag type;
> + const u8 *data = *_data;
> + size_t size, datalen = *_datalen;
Move the last declaration as the first declaration, makes more
readable (reverse christmas tree order).
I don't have time to go this patch fully for this round but I
saw some other similar nits below.
> +
> + pr_devel("-->%s(,%zu,,)\n", __func__, datalen);
> +
> + if (datalen < 2)
> + goto short_packet;
> +
> + pr_devel("pkthdr %02x, %02x\n", data[0], data[1]);
> +
> + type = *data++;
> + datalen--;
> + if (!(type & 0x80)) {
> + pr_debug("Packet type does not have MSB set\n");
> + return -EBADMSG;
> + }
> + type &= ~0x80;
> +
> + if (type & 0x40) {
> + /* New packet length format */
> + type &= ~0x40;
> + pr_devel("new format: t=%u\n", type);
> + switch (data[0]) {
> + case 0x00 ... 0xbf:
> + /* One-byte length */
> + size = data[0];
> + data++;
> + datalen--;
> + *_headerlen = 2;
> + break;
> + case 0xc0 ... 0xdf:
> + /* Two-byte length */
> + if (datalen < 2)
> + goto short_packet;
> + size = (data[0] - 192) * 256;
> + size += data[1] + 192;
> + data += 2;
> + datalen -= 2;
> + *_headerlen = 3;
> + break;
> + case 0xff:
> + /* Five-byte length */
> + if (datalen < 5)
> + goto short_packet;
> + size = data[1] << 24;
> + size |= data[2] << 16;
> + size |= data[3] << 8;
> + size |= data[4];
> + data += 5;
> + datalen -= 5;
> + *_headerlen = 6;
> + break;
> + default:
> + pr_debug("Partial body length packet not supported\n");
> + return -EBADMSG;
> + }
> + } else {
> + /* Old packet length format */
> + u8 length_type = type & 0x03;
> +
> + type >>= 2;
> + pr_devel("old format: t=%u lt=%u\n", type, length_type);
> +
> + switch (length_type) {
> + case 0:
> + /* One-byte length */
> + size = data[0];
> + data++;
> + datalen--;
> + *_headerlen = 2;
> + break;
> + case 1:
> + /* Two-byte length */
> + if (datalen < 2)
> + goto short_packet;
> + size = data[0] << 8;
> + size |= data[1];
> + data += 2;
> + datalen -= 2;
> + *_headerlen = 3;
> + break;
> + case 2:
> + /* Four-byte length */
> + if (datalen < 4)
> + goto short_packet;
> + size = data[0] << 24;
> + size |= data[1] << 16;
> + size |= data[2] << 8;
> + size |= data[3];
> + data += 4;
> + datalen -= 4;
> + *_headerlen = 5;
> + break;
> + default:
> + pr_debug("Indefinite length packet not supported\n");
> + return -EBADMSG;
> + }
> + }
> +
> + pr_devel("datalen=%zu size=%zu\n", datalen, size);
> + if (datalen < size)
> + goto short_packet;
> + if (size > INT_MAX)
> + goto too_big;
> +
> + *_data = data;
> + *_datalen = datalen;
> + *_type = type;
> + pr_devel("Found packet type=%u size=%zd\n", type, size);
> + return size;
> +
> +short_packet:
> + pr_debug("Attempt to parse short packet\n");
> + return -EBADMSG;
> +too_big:
> + pr_debug("Signature subpacket size >2G\n");
> + return -EMSGSIZE;
> +}
> +
> +/**
> + * pgp_parse_packets - Parse a set of PGP packets
> + * @data: Data to be parsed (updated)
> + * @datalen: Amount of data (updated)
> + * @ctx: Parsing context
> + *
> + * Parse a set of PGP packets [RFC 9580: 4].
> + *
> + * Return: 0 on successful parsing, a negative value otherwise.
> + */
> +int pgp_parse_packets(const u8 *data, size_t datalen,
> + struct pgp_parse_context *ctx)
> +{
> + enum pgp_packet_tag type;
> + ssize_t pktlen;
> + u8 headerlen;
> + int ret;
> +
> + while (datalen > 2) {
> + pktlen = pgp_parse_packet_header(&data, &datalen, &type,
> + &headerlen);
> + if (pktlen < 0)
> + return pktlen;
> +
> + if ((ctx->types_of_interest >> type) & 1) {
> + ret = ctx->process_packet(ctx, type, headerlen,
> + data, pktlen);
> + if (ret < 0)
> + return ret;
> + }
> + data += pktlen;
> + datalen -= pktlen;
> + }
> +
> + if (datalen != 0) {
> + pr_debug("Excess octets in packet stream\n");
> + return -EBADMSG;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(pgp_parse_packets);
> +
> +/**
> + * pgp_parse_public_key - Parse the common part of a PGP pubkey packet
> + * @_data: Content of packet (updated)
> + * @_datalen: Length of packet remaining (updated)
> + * @pk: Public key data
> + *
> + * Parse the common data struct for a PGP pubkey packet [RFC 9580: 5.5.2].
> + *
> + * Return: 0 on successful parsing, a negative value otherwise.
> + */
> +int pgp_parse_public_key(const u8 **_data, size_t *_datalen,
> + struct pgp_parse_pubkey *pk)
> +{
> + const u8 *data = *_data;
> + size_t datalen = *_datalen;
> + unsigned int tmp;
> +
> + if (datalen < 12) {
> + pr_debug("Public key packet too short\n");
> + return -EBADMSG;
> + }
> +
> + pk->version = *data++;
> + switch (pk->version) {
> + case PGP_KEY_VERSION_4:
> + break;
> + default:
> + pr_debug("Public key packet with unhandled version %d\n",
> + pk->version);
> + return -EBADMSG;
> + }
> +
> + tmp = *data++ << 24;
> + tmp |= *data++ << 16;
> + tmp |= *data++ << 8;
> + tmp |= *data++;
> + pk->creation_time = tmp;
> + if (pk->version == PGP_KEY_VERSION_4)
> + pk->expires_at = 0; /* Have to get it from the selfsignature */
> +
> + pk->pubkey_algo = *data++;
> + datalen -= 6;
> +
> + pr_devel("%x,%x,%lx,%lx\n",
> + pk->version, pk->pubkey_algo, pk->creation_time,
> + pk->expires_at);
> +
> + *_data = data;
> + *_datalen = datalen;
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(pgp_parse_public_key);
> diff --git a/crypto/asymmetric_keys/pgplib.h b/crypto/asymmetric_keys/pgplib.h
> new file mode 100644
> index 000000000000..3ec4b408a11e
> --- /dev/null
> +++ b/crypto/asymmetric_keys/pgplib.h
> @@ -0,0 +1,33 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* PGP library definitions (RFC 9580)
> + *
> + * Copyright (C) 2012 Red Hat, Inc. All Rights Reserved.
> + * Written by David Howells (dhowells@...hat.com)
> + */
> +
> +#include "pgp.h"
> +
> +/*
> + * PGP library packet parser
> + */
> +struct pgp_parse_context {
> + u64 types_of_interest;
> + int (*process_packet)(struct pgp_parse_context *context,
> + enum pgp_packet_tag type,
> + u8 headerlen,
> + const u8 *data,
> + size_t datalen);
> +};
> +
> +extern int pgp_parse_packets(const u8 *data, size_t datalen,
> + struct pgp_parse_context *ctx);
> +
> +struct pgp_parse_pubkey {
> + enum pgp_key_version version : 8;
> + enum pgp_pubkey_algo pubkey_algo : 8;
> + __kernel_old_time_t creation_time;
> + __kernel_old_time_t expires_at;
> +};
> +
> +extern int pgp_parse_public_key(const u8 **_data, size_t *_datalen,
> + struct pgp_parse_pubkey *pk);
BR, Jarkko
Powered by blists - more mailing lists