From 54ac55310fcad408e9cfeee2891f8c7bfbc47044 Mon Sep 17 00:00:00 2001 From: Sam Hartman Date: Mon, 19 Sep 2011 00:34:25 +0000 Subject: [PATCH] Support pkinit: SignedData with no signers (KDC) For anonymous, MIT 1.9 sends ContentInfo rather than SignedData. This violates RFc 6112. This patch accepts the RFC 6112 style encoding. ticket: 6961 git-svn-id: svn://anonsvn.mit.edu/krb5/trunk@25184 dc483132-0cff-0310-8789-dd5450dbe970 --- .../preauth/pkinit/pkinit_crypto_openssl.c | 394 +++++++++--------- src/plugins/preauth/pkinit/pkinit_srv.c | 2 +- 2 files changed, 209 insertions(+), 187 deletions(-) diff --git a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c index 2f2446ab7..0090b5dbc 100644 --- a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c +++ b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c @@ -44,6 +44,8 @@ #if OPENSSL_VERSION_NUMBER >= 0x10000000L /* Use CMS support present in OpenSSL 1.0 and later. */ #include +#define pkinit_CMS_get0_content_signed(_cms) CMS_Get0_Content(_cms) +#define pkinit_CMS_get0_content_data(_cms) CMS_Get0_Content(_cms) #define pkinit_CMS_free1_crls(_sk_x509crl) sk_X509_CRL_free((_sk_x509crl)) #define pkinit_CMS_free1_certs(_sk_x509) sk_X509_free((_sk_x509)) #define pkinit_CMS_SignerInfo_get_cert(_cms,_si,_x509_pp) \ @@ -58,7 +60,8 @@ #define CMS_SignerInfo PKCS7_SIGNER_INFO #define d2i_CMS_ContentInfo d2i_PKCS7 #define CMS_get0_type(_p7) ((_p7)->type) -#define CMS_get0_content(_p7) (&((_p7)->d.other->value.octet_string)) +#define pkinit_CMS_get0_content_signed(_p7) (&((_p7)->d.sign->contents->d.other->value.octet_string)) +#define pkinit_CMS_get0_content_data(_p7) (&((_p7)->d.other->value.octet_string)) #define CMS_set1_signers_certs(_p7,_stack_of_x509,_uint) #define CMS_get0_SignerInfos PKCS7_get_signer_info #define stack_st_CMS_SignerInfo stack_st_PKCS7_SIGNER_INFO @@ -1158,10 +1161,18 @@ cms_signeddata_verify(krb5_context context, unsigned int *authz_data_len, int *is_signed) { + /* + * Warning: Since most openssl functions do not set retval, large chunks of + * this function assume that retval is always a failure and may go to + * cleanup without setting retval explicitly. Make sure retval is not set + * to 0 or errors such as signature verification failure may be converted + * to success with significant security consequences. + */ krb5_error_code retval = KRB5KDC_ERR_PREAUTH_FAILED; CMS_ContentInfo *cms = NULL; BIO *out = NULL; int flags = CMS_NO_SIGNER_CERT_VERIFY; + int valid_oid = 0; unsigned int i = 0; unsigned int vflags = 0, size = 0; const unsigned char *p = signed_data; @@ -1202,13 +1213,18 @@ cms_signeddata_verify(krb5_context context, __FUNCTION__, ERR_error_string(err, NULL)); goto cleanup; } + etype = CMS_get0_eContentType(cms); - /* Handle the case in pkinit anonymous where we get unsigned data. */ + /* + * Prior to 1.10 the MIT client incorrectly omitted the pkinit structure + * directly in a CMS ContentInfo rather than using SignedData with no + * signers. Handle that case. + */ type = CMS_get0_type(cms); if (is_signed && !OBJ_cmp(type, oid)) { unsigned char *d; *is_signed = 0; - octets = CMS_get0_content(cms); + octets = pkinit_CMS_get0_content_data(cms); if (!octets || ((*octets)->type != V_ASN1_OCTET_STRING)) { retval = KRB5KDC_ERR_PREAUTH_FAILED; krb5_set_error_message(context, retval, @@ -1250,181 +1266,156 @@ cms_signeddata_verify(krb5_context context, /* 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; - pkinit_CMS_SignerInfo_get_cert(cms, si, &x); - if (x == NULL) - goto cleanup; - - /* create available CRL information (get local CRLs and include CRLs - * received in the CMS message - */ - signerRevoked = CMS_get1_crls(cms); - if (idctx->revoked == 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(signerRevoked); - for (i = 0; i < size; i++) - sk_X509_CRL_push(revoked, sk_X509_CRL_value(signerRevoked, i)); - } + if (((si_sk = CMS_get0_SignerInfos(cms)) == NULL) || + ((si = sk_CMS_SignerInfo_value(si_sk, 0)) == NULL)) { + /* Not actually signed; anonymous case */ + if (!is_signed) + goto cleanup; + *is_signed = 0; + /* We cannot use CMS_dataInit because there may be no digest */ + octets = pkinit_CMS_get0_content_signed(cms); + if (octets) + out = BIO_new_mem_buf((*octets)->data, (*octets)->length); + if (out == NULL) + goto cleanup; + } else { + pkinit_CMS_SignerInfo_get_cert(cms, si, &x); + if (x == NULL) + goto cleanup; - /* create available intermediate CAs chains (get local intermediateCAs and - * include the CA chain received in the CMS message - */ - signerCerts = CMS_get1_certs(cms); - if (idctx->intermediateCAs == NULL) - intermediateCAs = signerCerts; - else if (signerCerts == NULL) - intermediateCAs = idctx->intermediateCAs; - else { - size = sk_X509_num(idctx->intermediateCAs); - intermediateCAs = sk_X509_new_null(); - for (i = 0; i < size; i++) { - sk_X509_push(intermediateCAs, - sk_X509_value(idctx->intermediateCAs, i)); + /* create available CRL information (get local CRLs and include CRLs + * received in the CMS message + */ + signerRevoked = CMS_get1_crls(cms); + if (idctx->revoked == 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(signerRevoked); + for (i = 0; i < size; i++) + sk_X509_CRL_push(revoked, sk_X509_CRL_value(signerRevoked, i)); } - size = sk_X509_num(signerCerts); - for (i = 0; i < size; i++) { - sk_X509_push(intermediateCAs, sk_X509_value(signerCerts, i)); + + /* create available intermediate CAs chains (get local intermediateCAs and + * include the CA chain received in the CMS message + */ + signerCerts = CMS_get1_certs(cms); + if (idctx->intermediateCAs == NULL) + intermediateCAs = signerCerts; + else if (signerCerts == NULL) + intermediateCAs = idctx->intermediateCAs; + else { + size = sk_X509_num(idctx->intermediateCAs); + intermediateCAs = sk_X509_new_null(); + for (i = 0; i < size; i++) { + sk_X509_push(intermediateCAs, + sk_X509_value(idctx->intermediateCAs, i)); + } + size = sk_X509_num(signerCerts); + for (i = 0; i < size; i++) { + sk_X509_push(intermediateCAs, sk_X509_value(signerCerts, i)); + } } - } - /* initialize x509 context with the received certificate and - * trusted and intermediate CA chains and CRLs - */ - if (!X509_STORE_CTX_init(&cert_ctx, store, x, intermediateCAs)) - goto cleanup; + /* initialize x509 context with the received certificate and + * trusted and intermediate CA chains and CRLs + */ + if (!X509_STORE_CTX_init(&cert_ctx, store, x, intermediateCAs)) + goto cleanup; - X509_STORE_CTX_set0_crls(&cert_ctx, revoked); + X509_STORE_CTX_set0_crls(&cert_ctx, revoked); - /* add trusted CAs certificates for cert verification */ - if (idctx->trustedCAs != NULL) - X509_STORE_CTX_trusted_stack(&cert_ctx, idctx->trustedCAs); - else { - pkiDebug("unable to find any trusted CAs\n"); - goto cleanup; - } + /* add trusted CAs certificates for cert verification */ + if (idctx->trustedCAs != NULL) + X509_STORE_CTX_trusted_stack(&cert_ctx, idctx->trustedCAs); + else { + pkiDebug("unable to find any trusted CAs\n"); + goto cleanup; + } #ifdef DEBUG_CERTCHAIN - if (intermediateCAs != NULL) { - size = sk_X509_num(intermediateCAs); - pkiDebug("untrusted cert chain of size %d\n", size); - for (i = 0; i < size; i++) { - X509_NAME_oneline(X509_get_subject_name( - sk_X509_value(intermediateCAs, i)), buf, sizeof(buf)); - pkiDebug("cert #%d: %s\n", i, buf); + if (intermediateCAs != NULL) { + size = sk_X509_num(intermediateCAs); + pkiDebug("untrusted cert chain of size %d\n", size); + for (i = 0; i < size; i++) { + X509_NAME_oneline(X509_get_subject_name( + sk_X509_value(intermediateCAs, i)), buf, sizeof(buf)); + pkiDebug("cert #%d: %s\n", i, buf); + } } - } - if (idctx->trustedCAs != NULL) { - size = sk_X509_num(idctx->trustedCAs); - pkiDebug("trusted cert chain of size %d\n", size); - for (i = 0; i < size; i++) { - X509_NAME_oneline(X509_get_subject_name( - sk_X509_value(idctx->trustedCAs, i)), buf, sizeof(buf)); - pkiDebug("cert #%d: %s\n", i, buf); + if (idctx->trustedCAs != NULL) { + size = sk_X509_num(idctx->trustedCAs); + pkiDebug("trusted cert chain of size %d\n", size); + for (i = 0; i < size; i++) { + X509_NAME_oneline(X509_get_subject_name( + sk_X509_value(idctx->trustedCAs, i)), buf, sizeof(buf)); + pkiDebug("cert #%d: %s\n", i, buf); + } } - } - if (revoked != NULL) { - size = sk_X509_CRL_num(revoked); - pkiDebug("CRL chain of size %d\n", size); - for (i = 0; i < size; i++) { - X509_CRL *crl = sk_X509_CRL_value(revoked, i); - X509_NAME_oneline(X509_CRL_get_issuer(crl), buf, sizeof(buf)); - pkiDebug("crls by CA #%d: %s\n", i , buf); + if (revoked != NULL) { + size = sk_X509_CRL_num(revoked); + pkiDebug("CRL chain of size %d\n", size); + for (i = 0; i < size; i++) { + X509_CRL *crl = sk_X509_CRL_value(revoked, i); + X509_NAME_oneline(X509_CRL_get_issuer(crl), buf, sizeof(buf)); + pkiDebug("crls by CA #%d: %s\n", i , buf); + } } - } #endif - i = X509_verify_cert(&cert_ctx); - if (i <= 0) { - int j = X509_STORE_CTX_get_error(&cert_ctx); + i = X509_verify_cert(&cert_ctx); + if (i <= 0) { + int j = X509_STORE_CTX_get_error(&cert_ctx); - reqctx->received_cert = X509_dup(cert_ctx.current_cert); - switch(j) { - case X509_V_ERR_CERT_REVOKED: - retval = KRB5KDC_ERR_REVOKED_CERTIFICATE; - break; - case X509_V_ERR_UNABLE_TO_GET_CRL: - retval = KRB5KDC_ERR_REVOCATION_STATUS_UNKNOWN; - break; - case X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT: - case X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY: - retval = KRB5KDC_ERR_CANT_VERIFY_CERTIFICATE; - break; - default: - retval = KRB5KDC_ERR_INVALID_CERTIFICATE; - } - if (reqctx->received_cert == NULL) - strlcpy(buf, "(none)", sizeof(buf)); - else - X509_NAME_oneline(X509_get_subject_name(reqctx->received_cert), - buf, sizeof(buf)); - pkiDebug("problem with cert DN = %s (error=%d) %s\n", buf, j, - X509_verify_cert_error_string(j)); - krb5_set_error_message(context, retval, "%s\n", - X509_verify_cert_error_string(j)); + reqctx->received_cert = X509_dup(cert_ctx.current_cert); + switch(j) { + case X509_V_ERR_CERT_REVOKED: + retval = KRB5KDC_ERR_REVOKED_CERTIFICATE; + break; + case X509_V_ERR_UNABLE_TO_GET_CRL: + retval = KRB5KDC_ERR_REVOCATION_STATUS_UNKNOWN; + break; + case X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT: + case X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY: + retval = KRB5KDC_ERR_CANT_VERIFY_CERTIFICATE; + break; + default: + retval = KRB5KDC_ERR_INVALID_CERTIFICATE; + } + if (reqctx->received_cert == NULL) + strlcpy(buf, "(none)", sizeof(buf)); + else + X509_NAME_oneline(X509_get_subject_name(reqctx->received_cert), + buf, sizeof(buf)); + pkiDebug("problem with cert DN = %s (error=%d) %s\n", buf, j, + X509_verify_cert_error_string(j)); + krb5_set_error_message(context, retval, "%s\n", + X509_verify_cert_error_string(j)); #ifdef DEBUG_CERTCHAIN - 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(signerCerts, j); - X509_NAME_oneline(X509_get_subject_name(tmp_cert), buf, sizeof(buf)); - pkiDebug("cert #%d: %s\n", j, buf); - } + 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(signerCerts, j); + X509_NAME_oneline(X509_get_subject_name(tmp_cert), buf, sizeof(buf)); + pkiDebug("cert #%d: %s\n", j, buf); + } #endif - } else { - /* retrieve verified certificate chain */ - if (cms_msg_type == CMS_SIGN_CLIENT || cms_msg_type == CMS_SIGN_DRAFT9) - verified_chain = X509_STORE_CTX_get1_chain(&cert_ctx); - } - X509_STORE_CTX_cleanup(&cert_ctx); - if (i <= 0) - goto cleanup; - - out = BIO_new(BIO_s_mem()); - if (cms_msg_type == CMS_SIGN_DRAFT9) - flags |= CMS_NOATTR; - etype = CMS_get0_eContentType(cms); - if (CMS_verify(cms, NULL, store, NULL, out, flags)) { - int valid_oid = 0; - - if (!OBJ_cmp(etype, oid)) - valid_oid = 1; - else if (cms_msg_type == CMS_SIGN_DRAFT9) { - /* - * Various implementations of the pa-type 15 request use - * different OIDS. We check that the returned object - * has any of the acceptable OIDs - */ - ASN1_OBJECT *client_oid = NULL, *server_oid = NULL, *rsa_oid = NULL; - 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(etype, client_oid) || - !OBJ_cmp(etype, server_oid) || - !OBJ_cmp(etype, rsa_oid)) - valid_oid = 1; + } else { + /* retrieve verified certificate chain */ + if (cms_msg_type == CMS_SIGN_CLIENT || cms_msg_type == CMS_SIGN_DRAFT9) + verified_chain = X509_STORE_CTX_get1_chain(&cert_ctx); } - - if (valid_oid) - pkiDebug("CMS Verification successful\n"); - else { - pkiDebug("wrong oid in eContentType\n"); - print_buffer(etype->data, - (unsigned int)etype->length); - retval = KRB5KDC_ERR_PREAUTH_FAILED; - krb5_set_error_message(context, retval, "wrong oid\n"); + X509_STORE_CTX_cleanup(&cert_ctx); + if (i <= 0) goto cleanup; - } - } - else { + out = BIO_new(BIO_s_mem()); + if (cms_msg_type == CMS_SIGN_DRAFT9) + flags |= CMS_NOATTR; + if (CMS_verify(cms, NULL, store, NULL, out, flags) == 0) { unsigned long err = ERR_peek_error(); switch(ERR_GET_REASON(err)) { case PKCS7_R_DIGEST_FAILURE: @@ -1439,6 +1430,35 @@ cms_signeddata_verify(krb5_context context, ERR_error_string(err, NULL)); goto cleanup; } + } /* message was signed */ + if (!OBJ_cmp(etype, oid)) + valid_oid = 1; + else if (cms_msg_type == CMS_SIGN_DRAFT9) { + /* + * Various implementations of the pa-type 15 request use + * different OIDS. We check that the returned object + * has any of the acceptable OIDs + */ + ASN1_OBJECT *client_oid = NULL, *server_oid = NULL, *rsa_oid = NULL; + 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(etype, client_oid) || + !OBJ_cmp(etype, server_oid) || + !OBJ_cmp(etype, rsa_oid)) + valid_oid = 1; + } + + if (valid_oid) + pkiDebug("CMS Verification successful\n"); + else { + pkiDebug("wrong oid in eContentType\n"); + print_buffer(etype->data, + (unsigned int)etype->length); + retval = KRB5KDC_ERR_PREAUTH_FAILED; + krb5_set_error_message(context, retval, "wrong oid\n"); + goto cleanup; + } /* transfer the data from CMS message into return buffer */ for (size = 0;;) { @@ -1454,38 +1474,40 @@ cms_signeddata_verify(krb5_context context, } *data_len = size; - reqctx->received_cert = X509_dup(x); + if (x) { + reqctx->received_cert = X509_dup(x); - /* generate authorization data */ - if (cms_msg_type == CMS_SIGN_CLIENT || cms_msg_type == CMS_SIGN_DRAFT9) { + /* generate authorization data */ + if (cms_msg_type == CMS_SIGN_CLIENT || cms_msg_type == CMS_SIGN_DRAFT9) { - if (authz_data == NULL || authz_data_len == NULL) - goto out; + if (authz_data == NULL || authz_data_len == NULL) + goto out; - *authz_data = NULL; - retval = create_identifiers_from_stack(verified_chain, - &krb5_verified_chain); - if (retval) { - pkiDebug("create_identifiers_from_stack failed\n"); - goto cleanup; - } + *authz_data = NULL; + retval = create_identifiers_from_stack(verified_chain, + &krb5_verified_chain); + if (retval) { + pkiDebug("create_identifiers_from_stack failed\n"); + goto cleanup; + } - retval = k5int_encode_krb5_td_trusted_certifiers((const krb5_external_principal_identifier **)krb5_verified_chain, &authz); - if (retval) { - pkiDebug("encode_krb5_td_trusted_certifiers failed\n"); - goto cleanup; - } + retval = k5int_encode_krb5_td_trusted_certifiers((const krb5_external_principal_identifier **)krb5_verified_chain, &authz); + if (retval) { + pkiDebug("encode_krb5_td_trusted_certifiers failed\n"); + goto cleanup; + } #ifdef DEBUG_ASN1 - print_buffer_bin((unsigned char *)authz->data, authz->length, - "/tmp/kdc_ad_initial_verified_cas"); + print_buffer_bin((unsigned char *)authz->data, authz->length, + "/tmp/kdc_ad_initial_verified_cas"); #endif - *authz_data = malloc(authz->length); - if (*authz_data == NULL) { - retval = ENOMEM; - goto cleanup; + *authz_data = malloc(authz->length); + if (*authz_data == NULL) { + retval = ENOMEM; + goto cleanup; + } + memcpy(*authz_data, authz->data, authz->length); + *authz_data_len = authz->length; } - memcpy(*authz_data, authz->data, authz->length); - *authz_data_len = authz->length; } out: retval = 0; diff --git a/src/plugins/preauth/pkinit/pkinit_srv.c b/src/plugins/preauth/pkinit/pkinit_srv.c index 1dea777e7..1e208fa84 100644 --- a/src/plugins/preauth/pkinit/pkinit_srv.c +++ b/src/plugins/preauth/pkinit/pkinit_srv.c @@ -1055,7 +1055,7 @@ static int pkinit_server_get_flags(krb5_context kcontext, krb5_preauthtype patype) { if (patype == KRB5_PADATA_PKINIT_KX) - return PA_PSEUDO; + return PA_INFO; return PA_SUFFICIENT | PA_REPLACES_KEY; } -- 2.26.2