Support pkinit: SignedData with no signers (KDC)
authorSam Hartman <hartmans@mit.edu>
Mon, 19 Sep 2011 00:34:25 +0000 (00:34 +0000)
committerSam Hartman <hartmans@mit.edu>
Mon, 19 Sep 2011 00:34:25 +0000 (00:34 +0000)
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

src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
src/plugins/preauth/pkinit/pkinit_srv.c

index 2f2446ab7f71313b27b99f79f8f1ec1b54aeecaf..0090b5dbc0d8530c7ffb269d44e8203a0406e68b 100644 (file)
@@ -44,6 +44,8 @@
 #if OPENSSL_VERSION_NUMBER >= 0x10000000L
 /* Use CMS support present in OpenSSL 1.0 and later. */
 #include <openssl/cms.h>
+#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;
index 1dea777e7d706782d066415443aa267198734dfc..1e208fa847b163fc18dbbc95630cc933a9a54bd1 100644 (file)
@@ -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;
 }