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

PKCS7 Parser - RFC 2315 #3431

Merged
merged 38 commits into from
Nov 25, 2022
Merged
Show file tree
Hide file tree
Changes from 35 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
c9deb18
mbedtls: add support for pkcs7
naynajain Nov 16, 2020
673a226
pkcs7: add support for signed data
naynajain Dec 14, 2020
ca07f06
mbedtls: add pkcs7 in generate_errors.pl
naynajain Jun 12, 2020
aa91d4e
pkcs7: build under CMake
daxtens May 28, 2020
106a0af
pkcs7: provide fuzz harness
naynajain Nov 3, 2020
136c6aa
mbedtls: add pkcs7 test data
naynajain Nov 18, 2020
c448c94
pkcs7: pkcs7_get_content_info_type should reset *p on error
nick-child-ibm Jul 1, 2021
390e61a
pkcs7.h: Make pkcs7 fields private
nick-child-ibm Aug 9, 2021
600bd30
Avoid unwanted eol conversion of test data
mpg Feb 21, 2022
6671841
pkcs7.c: Do not ignore return value of mbedlts_md
nick-child-ibm Feb 22, 2022
6427b34
pkcs7.c: Use pkcs7_get_version for signerInfo
nick-child-ibm Feb 25, 2022
45525d3
pkcs7: Fix dependencies for pkcs7 tests
nick-child-ibm Feb 25, 2022
5d881c3
pkcs7: Change copyright
nick-child-ibm Feb 28, 2022
8a10f66
test/pkcs7: Add init for PSA tests
nick-child-ibm Jun 6, 2022
3538479
pkcs7: support multiple signers
daxtens Sep 2, 2020
62b2d7e
pkcs7: Support verification of hash with multiple signers
nick-child-ibm Jul 14, 2022
9f4fb3e
pkcs7: Unite function return style
nick-child-ibm Sep 12, 2022
8a94de4
test/pkcs7: Reduce number of test functions
nick-child-ibm Sep 14, 2022
7089ce8
pkcs7: Handle md errors in multisigner pkcs7 verification
nick-child-ibm Sep 14, 2022
34d5e93
pkcs7: Use better return code for unimplemented specifications
nick-child-ibm Sep 14, 2022
8ce1b1a
pkcs7: Correct various syntatical mistakes
nick-child-ibm Sep 14, 2022
9512bde
pkcs7: Fix pkcs7 error code values
nick-child-ibm Sep 16, 2022
5f9456f
pkcs7: Fix trailing whitespace
nick-child-ibm Sep 19, 2022
7dbe852
pkcs7: Import header files with included directory path not relative …
nick-child-ibm Sep 30, 2022
73621ef
pkcs7: Improve verify logic and rebuild test data
nick-child-ibm Oct 28, 2022
bb82ab7
pkcs7: Respond to feeback on parsing logic
nick-child-ibm Oct 28, 2022
5f39767
pkcs7: Fix imports
nick-child-ibm Oct 28, 2022
3951a4f
pkcs7: Use better error codes
nick-child-ibm Oct 31, 2022
fc234b7
test/pkcs7: Add Windows CRLF EOF to data files
nick-child-ibm Nov 2, 2022
2364aae
Update tests/suites/test_suite_pkcs7.function
daverodgman Nov 4, 2022
89e82e1
pkcs7: Add dependecy on MBEDTLS_MD_C
nick-child-ibm Nov 9, 2022
f58172f
Merge remote-tracking branch 'origin/development' into pr3431
daverodgman Nov 10, 2022
50e5616
Fix typo in check_config.h
daverodgman Nov 10, 2022
ebd0caf
Fix test memory allocation
daverodgman Nov 10, 2022
71565cf
Disable PKCS7 for some TLS 1.3 tests
daverodgman Nov 11, 2022
a17d038
Merge branch 'development' into pr3431
bensze01 Nov 22, 2022
ae79fb2
Merge branch 'development' into pr3431
bensze01 Nov 25, 2022
12269e2
Add changelog for PKCS7 parser
bensze01 Nov 25, 2022
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
3 changes: 2 additions & 1 deletion include/mbedtls/asn1.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,9 @@

/**
* \name ASN1 Error codes
* These error codes are OR'ed to X509 error codes for
* These error codes are combined with other error codes for
* higher error granularity.
* e.g. X.509 and PKCS #7 error codes
* ASN1 is a standard to specify data structures.
* \{
*/
Expand Down
8 changes: 8 additions & 0 deletions include/mbedtls/check_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -1062,6 +1062,14 @@
#error "MBEDTLS_SSL_TRUNCATED_HMAC was removed in Mbed TLS 3.0. See https://github.com/Mbed-TLS/mbedtls/issues/4341"
#endif

#if defined(MBEDTLS_PKCS7_C) && ( ( !defined(MBEDTLS_ASN1_PARSE_C) ) || \
( !defined(MBEDTLS_OID_C) ) || ( !defined(MBEDTLS_PK_PARSE_C) ) || \
( !defined(MBEDTLS_X509_CRT_PARSE_C) ) ||\
( !defined(MBEDTLS_X509_CRL_PARSE_C) ) || ( !defined(MBEDTLS_BIGNUM_C) ) || \
( !defined(MBEDTLS_MD_C) ) )
#error "MBEDTLS_PKCS7_C is defined, but not all prerequisites"
#endif

/*
* Avoid warning from -pedantic. This is a convenient place for this
* workaround since this is included by every single file before the
Expand Down
1 change: 1 addition & 0 deletions include/mbedtls/error.h
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@
* ECP 4 10 (Started from top)
* MD 5 5
* HKDF 5 1 (Started from top)
* PKCS7 5 12 (Started from 0x5300)
* SSL 5 2 (Started from 0x5F00)
* CIPHER 6 8 (Started from 0x6080)
* SSL 6 22 (Started from top, plus 0x6000)
Expand Down
16 changes: 16 additions & 0 deletions include/mbedtls/mbedtls_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -2780,6 +2780,22 @@
*/
#define MBEDTLS_PKCS5_C

/**
* \def MBEDTLS_PKCS7_C
*
* Enable PKCS7 core for using PKCS7 formatted signatures.
* RFC Link - https://tools.ietf.org/html/rfc2315
*
* Module: library/pkcs7.c
*
* Requires: MBEDTLS_ASN1_PARSE_C, MBEDTLS_OID_C, MBEDTLS_PK_PARSE_C,
* MBEDTLS_X509_CRT_PARSE_C MBEDTLS_X509_CRL_PARSE_C,
* MBEDTLS_BIGNUM_C, MBEDTLS_MD_C
*
* This module is required for the PKCS7 parsing modules.
*/
#define MBEDTLS_PKCS7_C

/**
* \def MBEDTLS_PKCS12_C
*
Expand Down
11 changes: 11 additions & 0 deletions include/mbedtls/oid.h
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,7 @@
#define MBEDTLS_OID_PKCS MBEDTLS_OID_RSA_COMPANY "\x01" /**< pkcs OBJECT IDENTIFIER ::= { iso(1) member-body(2) us(840) rsadsi(113549) 1 } */
#define MBEDTLS_OID_PKCS1 MBEDTLS_OID_PKCS "\x01" /**< pkcs-1 OBJECT IDENTIFIER ::= { iso(1) member-body(2) us(840) rsadsi(113549) pkcs(1) 1 } */
#define MBEDTLS_OID_PKCS5 MBEDTLS_OID_PKCS "\x05" /**< pkcs-5 OBJECT IDENTIFIER ::= { iso(1) member-body(2) us(840) rsadsi(113549) pkcs(1) 5 } */
#define MBEDTLS_OID_PKCS7 MBEDTLS_OID_PKCS "\x07" /**< pkcs-7 OBJECT IDENTIFIER ::= { iso(1) member-body(2) us(840) rsadsi(113549) pkcs(1) 7 } */
#define MBEDTLS_OID_PKCS9 MBEDTLS_OID_PKCS "\x09" /**< pkcs-9 OBJECT IDENTIFIER ::= { iso(1) member-body(2) us(840) rsadsi(113549) pkcs(1) 9 } */
#define MBEDTLS_OID_PKCS12 MBEDTLS_OID_PKCS "\x0c" /**< pkcs-12 OBJECT IDENTIFIER ::= { iso(1) member-body(2) us(840) rsadsi(113549) pkcs(1) 12 } */

Expand Down Expand Up @@ -300,6 +301,16 @@
#define MBEDTLS_OID_PKCS5_PBE_SHA1_DES_CBC MBEDTLS_OID_PKCS5 "\x0a" /**< pbeWithSHA1AndDES-CBC OBJECT IDENTIFIER ::= {pkcs-5 10} */
#define MBEDTLS_OID_PKCS5_PBE_SHA1_RC2_CBC MBEDTLS_OID_PKCS5 "\x0b" /**< pbeWithSHA1AndRC2-CBC OBJECT IDENTIFIER ::= {pkcs-5 11} */

/*
* PKCS#7 OIDs
*/
#define MBEDTLS_OID_PKCS7_DATA MBEDTLS_OID_PKCS7 "\x01" /**< Content type is Data OBJECT IDENTIFIER ::= {pkcs-7 1} */
#define MBEDTLS_OID_PKCS7_SIGNED_DATA MBEDTLS_OID_PKCS7 "\x02" /**< Content type is Signed Data OBJECT IDENTIFIER ::= {pkcs-7 2} */
#define MBEDTLS_OID_PKCS7_ENVELOPED_DATA MBEDTLS_OID_PKCS7 "\x03" /**< Content type is Enveloped Data OBJECT IDENTIFIER ::= {pkcs-7 3} */
#define MBEDTLS_OID_PKCS7_SIGNED_AND_ENVELOPED_DATA MBEDTLS_OID_PKCS7 "\x04" /**< Content type is Signed and Enveloped Data OBJECT IDENTIFIER ::= {pkcs-7 4} */
#define MBEDTLS_OID_PKCS7_DIGESTED_DATA MBEDTLS_OID_PKCS7 "\x05" /**< Content type is Digested Data OBJECT IDENTIFIER ::= {pkcs-7 5} */
#define MBEDTLS_OID_PKCS7_ENCRYPTED_DATA MBEDTLS_OID_PKCS7 "\x06" /**< Content type is Encrypted Data OBJECT IDENTIFIER ::= {pkcs-7 6} */

/*
* PKCS#8 OIDs
*/
Expand Down
241 changes: 241 additions & 0 deletions include/mbedtls/pkcs7.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,241 @@
/**
* \file pkcs7.h
*
* \brief PKCS7 generic defines and structures
* https://tools.ietf.org/html/rfc2315
*/
/*
* Copyright The Mbed TLS Contributors
* SPDX-License-Identifier: Apache-2.0
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

/**
* Note: For the time being, this implementation of the PKCS7 cryptographic
* message syntax is a partial implementation of RFC 2315.
* Differences include:
* - The RFC specifies 6 different content types. The only type currently
* supported in Mbed TLS is the signed data content type.
* - The only supported PKCS7 Signed Data syntax version is version 1
* - The RFC specifies support for BER. This implementation is limited to
* DER only.
* - The RFC specifies that multiple digest algorithms can be specified
* in the Signed Data type. Only one digest algorithm is supported in Mbed TLS.
* - The RFC specifies the Signed Data type can contain multiple X509 or PKCS6
* certificates. In Mbed TLS, this list can only contain 0 or 1 certificates
* and they must be in X509 format.
* - The RFC specifies the Signed Data type can contain
* certificate-revocation lists (crls). This implementation has no support
* for crls so it is assumed to be an empty list.
* - The RFC allows for SignerInfo structure to optionally contain
* unauthenticatedAttributes and authenticatedAttributes. In Mbed TLS it is
* assumed these fields are empty.
*/

#ifndef MBEDTLS_PKCS7_H
#define MBEDTLS_PKCS7_H

Copy link
Contributor

Choose a reason for hiding this comment

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

Given that there are a few assumptions in pkcs7.c (e.g., we don't support multiple digest algorithms), I think it would be sensible to have a comment describing the limitations of our PKCS7 support here.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree this is a good idea, this is certainly only a partial implementation.

we don't support multiple digest algorithms

Are you saying we only support sha256 as digest alg or that we only allow the listing of one md alg?

  • Regarding the former, after looking over the code (for the first time in awhile) I believe the functions should work with other md algorithms. That being said we should probably add more test cases if you choose to support more than SHA256 initially.
  • The latter, yes we only allow one digest algorithm to be specified in the DigestAlgorithmIdentifiers field, I will add that to the list of assumptions here.

Copy link
Contributor

Choose a reason for hiding this comment

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

I meant the latter, thanks. I had assumed other algs would work but agreed, testing this assumption is probably desirable.

#include "mbedtls/private_access.h"

#include "mbedtls/build_info.h"

#include "mbedtls/asn1.h"
#include "mbedtls/x509.h"
#include "mbedtls/x509_crt.h"

/**
* \name PKCS7 Module Error codes
* \{
*/
#define MBEDTLS_ERR_PKCS7_INVALID_FORMAT -0x5300 /**< The format is invalid, e.g. different type expected. */
#define MBEDTLS_ERR_PKCS7_FEATURE_UNAVAILABLE -0x5380 /**< Unavailable feature, e.g. anything other than signed data. */
#define MBEDTLS_ERR_PKCS7_INVALID_VERSION -0x5400 /**< The PKCS7 version element is invalid or cannot be parsed. */
#define MBEDTLS_ERR_PKCS7_INVALID_CONTENT_INFO -0x5480 /**< The PKCS7 content info invalid or cannot be parsed. */
#define MBEDTLS_ERR_PKCS7_INVALID_ALG -0x5500 /**< The algorithm tag or value is invalid or cannot be parsed. */
#define MBEDTLS_ERR_PKCS7_INVALID_CERT -0x5580 /**< The certificate tag or value is invalid or cannot be parsed. */
#define MBEDTLS_ERR_PKCS7_INVALID_SIGNATURE -0x5600 /**< Error parsing the signature */
#define MBEDTLS_ERR_PKCS7_INVALID_SIGNER_INFO -0x5680 /**< Error parsing the signer's info */
#define MBEDTLS_ERR_PKCS7_BAD_INPUT_DATA -0x5700 /**< Input invalid. */
#define MBEDTLS_ERR_PKCS7_ALLOC_FAILED -0x5780 /**< Allocation of memory failed. */
#define MBEDTLS_ERR_PKCS7_VERIFY_FAIL -0x5800 /**< Verification Failed */
#define MBEDTLS_ERR_PKCS7_CERT_DATE_INVALID -0x5880 /**< The PKCS7 date issued/expired dates are invalid */
/* \} name */

/**
* \name PKCS7 Supported Version
* \{
*/
#define MBEDTLS_PKCS7_SUPPORTED_VERSION 0x01
/* \} name */

#ifdef __cplusplus
extern "C" {
#endif

/**
* Type-length-value structure that allows for ASN1 using DER.
*/
typedef mbedtls_asn1_buf mbedtls_pkcs7_buf;

/**
* Container for ASN1 named information objects.
* It allows for Relative Distinguished Names (e.g. cn=localhost,ou=code,etc.).
*/
typedef mbedtls_asn1_named_data mbedtls_pkcs7_name;

/**
* Container for a sequence of ASN.1 items
*/
typedef mbedtls_asn1_sequence mbedtls_pkcs7_sequence;

/**
* PKCS7 types
*/
typedef enum {
MBEDTLS_PKCS7_NONE=0,
MBEDTLS_PKCS7_DATA,
MBEDTLS_PKCS7_SIGNED_DATA,
MBEDTLS_PKCS7_ENVELOPED_DATA,
MBEDTLS_PKCS7_SIGNED_AND_ENVELOPED_DATA,
MBEDTLS_PKCS7_DIGESTED_DATA,
MBEDTLS_PKCS7_ENCRYPTED_DATA,
}
mbedtls_pkcs7_type;

/**
* Structure holding PKCS7 signer info
*/
typedef struct mbedtls_pkcs7_signer_info
{
int MBEDTLS_PRIVATE(version);
mbedtls_x509_buf MBEDTLS_PRIVATE(serial);
mbedtls_x509_name MBEDTLS_PRIVATE(issuer);
mbedtls_x509_buf MBEDTLS_PRIVATE(issuer_raw);
mbedtls_x509_buf MBEDTLS_PRIVATE(alg_identifier);
mbedtls_x509_buf MBEDTLS_PRIVATE(sig_alg_identifier);
mbedtls_x509_buf MBEDTLS_PRIVATE(sig);
struct mbedtls_pkcs7_signer_info *MBEDTLS_PRIVATE(next);
}
mbedtls_pkcs7_signer_info;

/**
* Structure holding attached data as part of PKCS7 signed data format
*/
typedef struct mbedtls_pkcs7_data
{
mbedtls_pkcs7_buf MBEDTLS_PRIVATE(oid);
mbedtls_pkcs7_buf MBEDTLS_PRIVATE(data);
}
mbedtls_pkcs7_data;

/**
* Structure holding the signed data section
*/
typedef struct mbedtls_pkcs7_signed_data
{
int MBEDTLS_PRIVATE(version);
mbedtls_pkcs7_buf MBEDTLS_PRIVATE(digest_alg_identifiers);
struct mbedtls_pkcs7_data MBEDTLS_PRIVATE(content);
int MBEDTLS_PRIVATE(no_of_certs);
mbedtls_x509_crt MBEDTLS_PRIVATE(certs);
int MBEDTLS_PRIVATE(no_of_crls);
mbedtls_x509_crl MBEDTLS_PRIVATE(crl);
int MBEDTLS_PRIVATE(no_of_signers);
mbedtls_pkcs7_signer_info MBEDTLS_PRIVATE(signers);
}
mbedtls_pkcs7_signed_data;

/**
* Structure holding PKCS7 structure, only signed data for now
*/
typedef struct mbedtls_pkcs7
{
mbedtls_pkcs7_buf MBEDTLS_PRIVATE(raw);
mbedtls_pkcs7_buf MBEDTLS_PRIVATE(content_type_oid);
mbedtls_pkcs7_signed_data MBEDTLS_PRIVATE(signed_data);
}
mbedtls_pkcs7;

/**
* \brief Initialize pkcs7 structure.
*
* \param pkcs7 pkcs7 structure.
*/
void mbedtls_pkcs7_init( mbedtls_pkcs7 *pkcs7 );

/**
* \brief Parse a single DER formatted pkcs7 content.
*
* \param pkcs7 The pkcs7 structure to be filled by parser for the output.
* \param buf The buffer holding the DER encoded pkcs7.
* \param buflen The size in Bytes of \p buf.
*
* \note This function makes an internal copy of the PKCS7 buffer
* \p buf. In particular, \p buf may be destroyed or reused
* after this call returns.
*
* \return The \c mbedtls_pkcs7_type of \p buf, if successful.
* \return A negative error code on failure.
*/
int mbedtls_pkcs7_parse_der( mbedtls_pkcs7 *pkcs7, const unsigned char *buf,
const size_t buflen );

/**
* \brief Verification of PKCS7 signature.
*
* \param pkcs7 PKCS7 structure containing signature.
* \param cert Certificate containing key to verify signature.
* \param data Plain data on which signature has to be verified.
* \param datalen Length of the data.
*
* \note This function internally calculates the hash on the supplied
* plain data for signature verification.
*
* \return A negative error code on failure.
*/
int mbedtls_pkcs7_signed_data_verify( mbedtls_pkcs7 *pkcs7,

Choose a reason for hiding this comment

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

Shouldn't pkcs7 and cert parameters also be 'const'? You certainly wouldn't expect the input certificate to get modified.

const mbedtls_x509_crt *cert,
const unsigned char *data,
size_t datalen );

/**
* \brief Verification of PKCS7 signature.
*
* \param pkcs7 PKCS7 structure containing signature.
* \param cert Certificate containing key to verify signature.
* \param hash Hash of the plain data on which signature has to be verified.
* \param hashlen Length of the hash.
*
* \note This function is different from mbedtls_pkcs7_signed_data_verify()
* in a way that it directly recieves the hash of the data.
*
* \return A negative error code on failure.
*/
int mbedtls_pkcs7_signed_hash_verify( mbedtls_pkcs7 *pkcs7,

Choose a reason for hiding this comment

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

Same 'const' comment as above.

const mbedtls_x509_crt *cert,
const unsigned char *hash, size_t hashlen);

/**
* \brief Unallocate all PKCS7 data and zeroize the memory.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is zeroizing needed for PKCS7 data? What secret data do we risk exposing?

Copy link
Contributor

Choose a reason for hiding this comment

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

Up to you guys, nothing is exposed that is more confidential than a public key and and data that has been signed with the private key. I will remove these zeroize calls for now and if anyone can come up with an argument against it than we can debate adding them back.

* It doesn't free pkcs7 itself. It should be done by the caller.
*
* \param pkcs7 PKCS7 structure to free.
*/
void mbedtls_pkcs7_free( mbedtls_pkcs7 *pkcs7 );

#ifdef __cplusplus
}
#endif

#endif /* pkcs7.h */
1 change: 1 addition & 0 deletions library/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ set(src_crypto
)

set(src_x509
pkcs7.c
x509.c
x509_create.c
x509_crl.c
Expand Down
1 change: 1 addition & 0 deletions library/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ OBJS_X509= \
x509_csr.o \
x509write_crt.o \
x509write_csr.o \
pkcs7.o \
# This line is intentionally left blank

OBJS_TLS= \
Expand Down
Loading