Ticket History #6851: pkinit can't parse some valid cms messages

From krb5-bugs-incoming-bounces@PCH.mit.edu Mon Jan 24 18:33:44 2011
Return-Path: <krb5-bugs-incoming-bounces@PCH.mit.edu>
Received: from pch.mit.edu (PCH.MIT.EDU [18.7.21.90])
by krbdev.mit.edu (Postfix) with ESMTP id 73CCD3E6A6;
Mon, 24 Jan 2011 18:33:44 -0500 (EST)
Received: from pch.mit.edu (pch.mit.edu [127.0.0.1])
by pch.mit.edu (8.13.6/8.12.8) with ESMTP id p0ONXiWL024604;
Mon, 24 Jan 2011 18:33:44 -0500
Received: from mailhub-dmz-1.mit.edu (MAILHUB-DMZ-1.MIT.EDU [18.9.21.41])
by pch.mit.edu (8.13.6/8.12.8) with ESMTP id p0ONTEYt024009
for <krb5-bugs-incoming@PCH.mit.edu>; Mon, 24 Jan 2011 18:29:15 -0500
Received: from dmz-mailsec-scanner-1.mit.edu (DMZ-MAILSEC-SCANNER-1.MIT.EDU
[18.9.25.12])
by mailhub-dmz-1.mit.edu (8.13.8/8.9.2) with ESMTP id p0ONTENE013440
for <krb5-bugs@mit.edu>; Mon, 24 Jan 2011 18:29:14 -0500
X-AuditID: 1209190c-b7ba9ae0000009f8-98-4d3e0b49e2b2
Received: from mx1.redhat.com ( [209.132.183.28])
by dmz-mailsec-scanner-1.mit.edu (Symantec Brightmail Gateway) with
SMTP id 2D.1D.02552.94B0E3D4; Mon, 24 Jan 2011 18:29:14 -0500 (EST)
Received: from int-mx12.intmail.prod.int.phx2.redhat.com
(int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25])
by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id p0ONTDEA011353
(version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK)
for <krb5-bugs@mit.edu>; Mon, 24 Jan 2011 18:29:13 -0500
Received: from blade.bos.redhat.com (blade.bos.redhat.com [10.16.0.23])
by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP
id p0ONTCL8004634
(version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO)
for <krb5-bugs@mit.edu>; Mon, 24 Jan 2011 18:29:12 -0500
Received: from blade.bos.redhat.com (localhost.localdomain [127.0.0.1])
by blade.bos.redhat.com (8.14.4/8.14.3) with ESMTP id p0ONTBLp018141
for <krb5-bugs@mit.edu>; Mon, 24 Jan 2011 18:29:11 -0500
Received: (from nalin@localhost)
by blade.bos.redhat.com (8.14.4/8.14.4/Submit) id p0ONTBAu018136;
Mon, 24 Jan 2011 18:29:11 -0500
Date: Mon, 24 Jan 2011 18:29:11 -0500
Message-Id: <201101242329.p0ONTBAu018136@blade.bos.redhat.com>
To: krb5-bugs@mit.edu
Subject: pkinit can't parse some valid cms messages
From: nalin@redhat.com
X-send-pr-version: 3.99
X-Scanned-By: MIMEDefang 2.68 on 10.5.11.25
X-Brightmail-Tracker: AAAAARcylxY=
X-Mailman-Approved-At: Mon, 24 Jan 2011 18:33:42 -0500
X-BeenThere: krb5-bugs-incoming@mailman.mit.edu
X-Mailman-Version: 2.1.6
Precedence: list
Reply-To: nalin@redhat.com
Sender: krb5-bugs-incoming-bounces@PCH.mit.edu
Errors-To: krb5-bugs-incoming-bounces@PCH.mit.edu



System: Linux blade.bos.redhat.com 2.6.35.6-48.fc14.x86_64 #1 SMP Fri Oct 22 15:36:08 UTC 2010 x86_64 x86_64 x86_64 GNU/Linux
Architecture: x86_64

The PKINIT spec incorporates the CMS spec by reference. When built
with OpenSSL, the pkinit module uses OpenSSL's PKCS7 APIs to parse CMS
messages.

The PKCS7 APIs can only parse a subset of CMS, and will fail if a
SignerInfo structure in a SignedData message identifies the signer
using a subjectKeyID rather than an issuer/serial pair.

The pkinit-nss module (now deprecated) would create signed-data which
would trigger this bug if its pkinit_signed_data_version setting was
set to the default value of "3". I suspect this may have been the
root cause of Glenn Machin's problem here:
http://mailman.mit.edu/pipermail/kerberos/2010-December/016794.html

An example error that the KDC logs when this happens:
cms_signeddata_verify: failed to decode message: error:0D0680A8:asn1 encoding routines:ASN1_CHECK_TLEN:wrong tag

This patch should switch cms_signeddata_verify() to using the OpenSSL
CMS APIs if we're building with a version of OpenSSL which supplies them
(0.9.8g or later, assuming I'm reading the OpenSSL NEWS file right).

diff --git a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
index bb8f036..ca28967 100644
--- a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
+++ b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
@@ -41,6 +41,11 @@

#include "pkinit_crypto_openssl.h"

+#if OPENSSL_VERSION_NUMBER >= 0x00908070L
+#define PKINIT_CMS
+#include <openssl/cms.h>
+#endif
+
static struct pkcs11_errstrings {
short code;
char *text;
@@ -1127,21 +1132,32 @@ cms_signeddata_verify(krb5_context context,
int *is_signed)
{
krb5_error_code retval = KRB5KDC_ERR_PREAUTH_FAILED;
+#ifdef PKINIT_CMS
+ CMS_ContentInfo *cms = NULL;
+ int flags = CMS_NO_SIGNER_CERT_VERIFY;
+ STACK_OF(CMS_SignerInfo) *si_sk = NULL;
+ CMS_SignerInfo *si = NULL;
+#else
PKCS7 *p7 = NULL;
- BIO *out = NULL;
int flags = PKCS7_NOVERIFY;
+ STACK_OF(PKCS7_SIGNER_INFO) *si_sk = NULL;
+ PKCS7_SIGNER_INFO *si = NULL;
+#endif
+ BIO *out = NULL;
unsigned int i = 0;
unsigned int vflags = 0, size = 0;
const unsigned char *p = signed_data;
- STACK_OF(PKCS7_SIGNER_INFO) *si_sk = NULL;
- PKCS7_SIGNER_INFO *si = NULL;
X509 *x = NULL;
X509_STORE *store = NULL;
X509_STORE_CTX cert_ctx;
+ STACK_OF(X509) *signerCerts = NULL;
STACK_OF(X509) *intermediateCAs = NULL;
+ STACK_OF(X509_CRL) *signerRevoked = NULL;
STACK_OF(X509_CRL) *revoked = NULL;
STACK_OF(X509) *verified_chain = NULL;
ASN1_OBJECT *oid = NULL;
+ const ASN1_OBJECT *type = NULL, *etype = NULL;
+ ASN1_OCTET_STRING *octets;
krb5_external_principal_identifier **krb5_verified_chain = NULL;
krb5_data *authz = NULL;
char buf[DN_BUF_LEN];
@@ -1157,8 +1173,12 @@ cms_signeddata_verify(krb5_context context,
if (oid == NULL)
goto cleanup;

- /* decode received PKCS7 message */
+ /* decode received CMS/PKCS7 message */
+#ifdef PKINIT_CMS
+ if ((cms = d2i_CMS_ContentInfo(NULL, &p, (int)signed_data_len)) == NULL) {
+#else
if ((p7 = d2i_PKCS7(NULL, &p, (int)signed_data_len)) == NULL) {
+#endif
unsigned long err = ERR_peek_error();
krb5_set_error_message(context, retval, "%s\n",
ERR_error_string(err, NULL));
@@ -1168,37 +1188,47 @@ cms_signeddata_verify(krb5_context context,
}

/* Handle the case in pkinit anonymous where we get unsigned data. */
- if (is_signed && !OBJ_cmp(p7->type, oid)) {
+#ifdef PKINIT_CMS
+ type = CMS_get0_type(cms);
+#else
+ type = p7->type;
+#endif
+ if (is_signed && !OBJ_cmp(type, oid)) {
unsigned char *d;
*is_signed = 0;
- if (p7->d.other->type != V_ASN1_OCTET_STRING) {
+#ifdef PKINIT_CMS
+ octets = CMS_get0_content(cms);
+#else
+ octets = p7->d.other;
+#endif
+ if (octets->type != V_ASN1_OCTET_STRING) {
retval = KRB5KDC_ERR_PREAUTH_FAILED;
krb5_set_error_message(context, KRB5KDC_ERR_PREAUTH_FAILED,
"Invalid pkinit packet: octet string "
"expected");
goto cleanup;
}
- *data_len = ASN1_STRING_length(p7->d.other->value.octet_string);
+ *data_len = ASN1_STRING_length(octets);
d = malloc(*data_len);
if (d == NULL) {
retval = ENOMEM;
goto cleanup;
}
- memcpy(d, ASN1_STRING_data(p7->d.other->value.octet_string),
+ memcpy(d, ASN1_STRING_data(octets),
*data_len);
*data = d;
goto out;
} else {
- /* Verify that the received message is PKCS7 SignedData message. */
- if (OBJ_obj2nid(p7->type) != NID_pkcs7_signed) {
- pkiDebug("Expected id-signedData PKCS7 msg (received type = %d)\n",
- OBJ_obj2nid(p7->type));
+ /* Verify that the received message is CMS/PKCS7 SignedData message. */
+ if (OBJ_obj2nid(type) != NID_pkcs7_signed) {
+ pkiDebug("Expected id-signedData CMS msg (received type = %d)\n",
+ OBJ_obj2nid(type));
krb5_set_error_message(context, retval, "wrong oid\n");
goto cleanup;
}
}

- /* setup to verify X509 certificate used to sign PKCS7 message */
+ /* setup to verify X509 certificate used to sign CMS/PKCS7 message */
if (!(store = X509_STORE_new()))
goto cleanup;

@@ -1210,6 +1240,17 @@ cms_signeddata_verify(krb5_context context,
X509_STORE_set_verify_cb_func(store, openssl_callback_ignore_crls);
X509_STORE_set_flags(store, vflags);

+#ifdef PKINIT_CMS
+ /* get the signer's information from the CMS message */
+ CMS_set1_signers_certs(cms, NULL, 0);
+ if ((si_sk = CMS_get0_SignerInfos(cms)) == NULL)
+ goto cleanup;
+ if ((si = sk_CMS_SignerInfo_value(si_sk, 0)) == NULL)
+ goto cleanup;
+ CMS_SignerInfo_get0_algs(si, NULL, &x, NULL, NULL);
+ if (x == NULL)
+ goto cleanup;
+#else
/* get the signer's information from the PKCS7 message */
if ((si_sk = PKCS7_get_signer_info(p7)) == NULL)
goto cleanup;
@@ -1217,30 +1258,41 @@ cms_signeddata_verify(krb5_context context,
goto cleanup;
if ((x = PKCS7_cert_from_signer_info(p7, si)) == NULL)
goto cleanup;
+#endif

/* create available CRL information (get local CRLs and include CRLs
- * received in the PKCS7 message
+ * received in the CMS/PKCS7 message
*/
+#ifdef PKINIT_CMS
+ signerRevoked = CMS_get1_crls(cms);
+#else
+ signerRevoked = p7->d.sign->crl;
+#endif
if (idctx->revoked == NULL)
- revoked = p7->d.sign->crl;
- else if (p7->d.sign->crl == NULL)
+ revoked = signerRevoked;
+ else if (signerRevoked == NULL)
revoked = idctx->revoked;
else {
size = sk_X509_CRL_num(idctx->revoked);
revoked = sk_X509_CRL_new_null();
for (i = 0; i < size; i++)
sk_X509_CRL_push(revoked, sk_X509_CRL_value(idctx->revoked, i));
- size = sk_X509_CRL_num(p7->d.sign->crl);
+ size = sk_X509_CRL_num(signerRevoked);
for (i = 0; i < size; i++)
- sk_X509_CRL_push(revoked, sk_X509_CRL_value(p7->d.sign->crl, i));
+ sk_X509_CRL_push(revoked, sk_X509_CRL_value(signerRevoked, i));
}

/* create available intermediate CAs chains (get local intermediateCAs and
- * include the CA chain received in the PKCS7 message
+ * include the CA chain received in the CMS/PKCS7 message
*/
+#ifdef PKINIT_CMS
+ signerCerts = CMS_get1_certs(cms);
+#else
+ signerCerts = p7->d.sign->cert;
+#endif
if (idctx->intermediateCAs == NULL)
- intermediateCAs = p7->d.sign->cert;
- else if (p7->d.sign->cert == NULL)
+ intermediateCAs = signerCerts;
+ else if (signerCerts == NULL)
intermediateCAs = idctx->intermediateCAs;
else {
size = sk_X509_num(idctx->intermediateCAs);
@@ -1249,9 +1301,9 @@ cms_signeddata_verify(krb5_context context,
sk_X509_push(intermediateCAs,
sk_X509_value(idctx->intermediateCAs, i));
}
- size = sk_X509_num(p7->d.sign->cert);
+ size = sk_X509_num(signerCerts);
for (i = 0; i < size; i++) {
- sk_X509_push(intermediateCAs, sk_X509_value(p7->d.sign->cert, i));
+ sk_X509_push(intermediateCAs, sk_X509_value(signerCerts, i));
}
}

@@ -1329,10 +1381,10 @@ cms_signeddata_verify(krb5_context context,
krb5_set_error_message(context, retval, "%s\n",
X509_verify_cert_error_string(j));
#ifdef DEBUG_CERTCHAIN
- size = sk_X509_num(p7->d.sign->cert);
+ size = sk_X509_num(signerCerts);
pkiDebug("received cert chain of size %d\n", size);
for (j = 0; j < size; j++) {
- X509 *tmp_cert = sk_X509_value(p7->d.sign->cert, j);
+ X509 *tmp_cert = sk_X509_value(signerCerts, j);
X509_NAME_oneline(X509_get_subject_name(tmp_cert), buf, sizeof(buf));
pkiDebug("cert #%d: %s\n", j, buf);
}
@@ -1347,12 +1399,20 @@ cms_signeddata_verify(krb5_context context,
goto cleanup;

out = BIO_new(BIO_s_mem());
+#ifdef PKINIT_CMS
+ if (cms_msg_type == CMS_SIGN_DRAFT9)
+ flags |= CMS_NOATTR;
+ etype = CMS_get0_eContentType(cms);
+ if (CMS_verify(cms, NULL, store, NULL, out, flags)) {
+#else
if (cms_msg_type == CMS_SIGN_DRAFT9)
flags |= PKCS7_NOATTR;
+ etype = p7->d.sign->contents->type;
if (PKCS7_verify(p7, NULL, store, NULL, out, flags)) {
+#endif
int valid_oid = 0;

- if (!OBJ_cmp(p7->d.sign->contents->type, oid))
+ if (!OBJ_cmp(etype, oid))
valid_oid = 1;
else if (cms_msg_type == CMS_SIGN_DRAFT9) {
/*
@@ -1364,18 +1424,22 @@ cms_signeddata_verify(krb5_context context,
client_oid = pkinit_pkcs7type2oid(plgctx, CMS_SIGN_CLIENT);
server_oid = pkinit_pkcs7type2oid(plgctx, CMS_SIGN_SERVER);
rsa_oid = pkinit_pkcs7type2oid(plgctx, CMS_ENVEL_SERVER);
- if (!OBJ_cmp(p7->d.sign->contents->type, client_oid) ||
- !OBJ_cmp(p7->d.sign->contents->type, server_oid) ||
- !OBJ_cmp(p7->d.sign->contents->type, rsa_oid))
+ if (!OBJ_cmp(etype, client_oid) ||
+ !OBJ_cmp(etype, server_oid) ||
+ !OBJ_cmp(etype, rsa_oid))
valid_oid = 1;
}

if (valid_oid)
+#ifdef PKINIT_CMS
+ pkiDebug("CMS Verification successful\n");
+#else
pkiDebug("PKCS7 Verification successful\n");
+#endif
else {
pkiDebug("wrong oid in eContentType\n");
- print_buffer(p7->d.sign->contents->type->data,
- (unsigned int)p7->d.sign->contents->type->length);
+ print_buffer(etype->data,
+ (unsigned int)etype->length);
retval = KRB5KDC_ERR_PREAUTH_FAILED;
krb5_set_error_message(context, retval, "wrong oid\n");
goto cleanup;
@@ -1391,13 +1455,17 @@ cms_signeddata_verify(krb5_context context,
default:
retval = KRB5KDC_ERR_INVALID_SIG;
}
+#ifdef PKINIT_CMS
+ pkiDebug("CMS Verification failure\n");
+#else
pkiDebug("PKCS7 Verification failure\n");
+#endif
krb5_set_error_message(context, retval, "%s\n",
ERR_error_string(err, NULL));
goto cleanup;
}

- /* transfer the data from PKCS7 message into return buffer */
+ /* transfer the data from CMS message into return buffer */
for (size = 0;;) {
int remain;
retval = ENOMEM;
@@ -1452,13 +1520,27 @@ cleanup:
BIO_free(out);
if (store != NULL)
X509_STORE_free(store);
+#ifdef PKINIT_CMS
+ if (cms != NULL) {
+ if (signerCerts != NULL)
+ sk_X509_free(signerCerts);
+ if (idctx->intermediateCAs != NULL && signerCerts)
+ sk_X509_free(intermediateCAs);
+ if (signerRevoked != NULL)
+ sk_X509_CRL_free(signerRevoked);
+ if (idctx->revoked != NULL && signerRevoked)
+ sk_X509_CRL_free(revoked);
+ CMS_ContentInfo_free(cms);
+ }
+#else
if (p7 != NULL) {
- if (idctx->intermediateCAs != NULL && p7->d.sign->cert)
+ if (idctx->intermediateCAs != NULL && signerCerts)
sk_X509_free(intermediateCAs);
- if (idctx->revoked != NULL && p7->d.sign->crl)
+ if (idctx->revoked != NULL && signerRevoked)
sk_X509_CRL_free(revoked);
PKCS7_free(p7);
}
+#endif
if (verified_chain != NULL)
sk_X509_pop_free(verified_chain, X509_free);
if (krb5_verified_chain != NULL)

The patch looks reasonable, but fails to build on Ubuntu Lucid, and
probably on any system with an OpenSSL 0.9.8 version. Although 0.9.8g
claimed to include a backport of the CMS functionality, it doesn't appear
to install cms.h, and neither does any version of 0.9.8 as far as I can
tell. (In fact, 0.9.8g doesn't appear to have CMS source files either;
those showed up in 0.9.8h.)

When building PKINIT against OpenSSL 1.0 or later, use the CMS APIs for
better interoperability. From nalin@redhat.com.


https://github.com/krb5/krb5/commit/1b57e7650da41dbef6a50c6e925583d6c8ed9e52
Commit By: ghudson
Revision: 24605
Changed Files:
U trunk/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c

We have a report that this change breaks interoperability between the MIT
PKINIT client and a Heimdal 1.2.1 KDC; the proximal cause of the breakage
is that CMS_SignerInfo_get0_algs() returns no certs for the signature of
the PKINIT reply.

https://list.sics.se/sympa/arc/heimdal-discuss/2012-06/msg00023.html

Thanks. I've asked Hank for a packet capture of the KDC reply, which
should help narrow this down.