[SSSD] sssdpac_verify() treats krb5_pac_verify() error as fatal - mspac_verify() does not

Jakub Hrozek jhrozek at redhat.com
Mon Aug 4 11:40:05 UTC 2014


On Mon, Aug 04, 2014 at 02:24:47PM +0300, Alexander Bokovoy wrote:
> On Sun, 03 Aug 2014, Thomas Sondergaard wrote:
> >Hello,
> >
> >I've run into a GSSAPI authentication problem that has caused me a little
> >time to diagnose, but turns out to only occur with sssd_pac_plugin.so
> >present. I am not an AD, Kerberos or sssd expert, so let me just present
> >the information I have collected.
> >
> >Our code, which uses GSSAPI, fails with the following error on a Fedora 20
> >box with sssd-client (sssd_pac_plugin.so) installed with the following
> >error codes:
> >
> >major: 851968='Unspecified GSS failure.  Minor code may provide more
> >information', minor: 22='Invalid argument'
> >
> >KRB5_TRACE does not give any useful indication for what goes wrong.
> >
> >With sssd-client (sssd_pac_plugin.so) removed the GSSAPI authentication
> >works. With KRB5_TRACE I get the following warning:
> >
> >[23801] 1407018816.872001: PAC checksum verification failed:
> >-1765328196/Bad encryption type
> >
> >Looking at the code, I find that without sssd_pac_plugin.so installed the
> >following verification function is used:
> >
> >static krb5_error_code
> >mspac_verify(krb5_context kcontext,
> >            krb5_authdata_context context,
> >            void *plugin_context,
> >            void *request_context,
> >            const krb5_auth_context *auth_context,
> >            const krb5_keyblock *key,
> >            const krb5_ap_req *req)
> >{
> >   krb5_error_code code;
> >   struct mspac_context *pacctx = (struct mspac_context *)request_context;
> >
> >   if (pacctx->pac == NULL)
> >       return EINVAL;
> >
> >   code = krb5_pac_verify(kcontext, pacctx->pac,
> >                          req->ticket->enc_part2->times.authtime,
> >                          req->ticket->enc_part2->client, key, NULL);
> >   if (code != 0)
> >       TRACE_MSPAC_VERIFY_FAIL(kcontext, code);
> >
> >   /*
> >    * If the above verification failed, don't fail the whole
> >authentication,
> >    * just don't mark the PAC as verified.  A checksum mismatch can occur
> >if
> >    * the PAC was copied from a cross-realm TGT by an ignorant KDC, and
> >Apple
> >    * Mac OS X Server Open Directory (as of 10.6) generates PACs with no
> >    * server checksum at all.
> >    */
> >   return 0;
> >}
> >
> >
> >This incarnation of the mspac_verify() function is from
> >
> >commit 76ebe5d07c1002b674eb1c4e3ab35f6001eec91c
> >Author: Greg Hudson <ghudson at mit.edu>
> >Date:   Wed Feb 16 23:34:37 2011 +0000
> >
> >   Don't reject AP-REQs based on PACs
> >
> >   Experience has shown that it was a mistake to fail AP-REQ verification
> >   based on failure to verify the signature of PAC authdata contained in
> >   the ticket.  We've had two rounds of interoperability issues with the
> >   hmac-md5 checksum code, an interoperability issue OSX generating
> >   unsigned PACs, and another problem where PACs are copied by older KDCs
> >   from a cross-realm TGT into the service ticket.  If a PAC signature
> >   cannot be verified, just don't mark it as verified and continue on
> >   with the AP exchange.
> >
> >   ticket: 6870
> >   target_version: 1.9.1
> >   tags: pullup
> >
> >   git-svn-id: svn://anonsvn.mit.edu/krb5/trunk@24640
> >dc483132-0cff-0310-8789-dd5450dbe970
> >
> >
> >
> >With sssd-client (sssd_pac_plugin.so) installed the function
> >sssdpac_verify is used instead, which looks like this
> >(sssd-client-1.11.6-1):
> >
> >static krb5_error_code sssdpac_verify(krb5_context kcontext,
> >                                     krb5_authdata_context context,
> >                                     void *plugin_context,
> >                                     void *request_context,
> >                                     const krb5_auth_context
> >*auth_context,
> >                                     const krb5_keyblock *key,
> >                                     const krb5_ap_req *req)
> >{
> >   krb5_error_code kerr;
> >   int ret;
> >   krb5_pac pac;
> >   struct sssd_context *sssdctx = (struct sssd_context *)request_context;
> >   struct sss_cli_req_data sss_data;
> >   int errnop;
> >
> >   if (sssdctx == NULL || sssdctx->data.data == NULL) {
> >       return EINVAL;
> >   }
> >
> >   kerr = krb5_pac_parse(kcontext, sssdctx->data.data,
> >                         sssdctx->data.length, &pac);
> >   if (kerr != 0) {
> >       return EINVAL;
> >   }
> >
> >   kerr = krb5_pac_verify(kcontext, pac,
> >                          req->ticket->enc_part2->times.authtime,
> >                          req->ticket->enc_part2->client, key, NULL);
> >   if (kerr != 0) {
> >       return EINVAL;
> >   }
> >
> >   sss_data.len = sssdctx->data.length;
> >   sss_data.data = sssdctx->data.data;
> >
> >   ret = sss_pac_make_request(SSS_PAC_ADD_PAC_USER, &sss_data,
> >                              NULL, NULL, &errnop);
> >   if (ret != 0) {
> >       /* Ignore the error */
> >   }
> >
> >   return 0;
> >}
> >
> >The krb5_pac_verify header documentation from krb5.h has this to say:
> >
> >* @note A checksum mismatch can occur if the PAC was copied from a
> >cross-realm
> >* TGT by an ignorant KDC; also Apple Mac OS X Server Open Directory (as of
> >* 10.6) generates PACs with no server checksum at all.  One should
> >consider
> >* not failing the whole authentication because of this reason, but,
> >instead,
> >* treating the ticket as if it did not contain a PAC or marking the PAC
> >* information as non-verified.
> >
> >So, sssdpac_verify() treats krb5_pack_verify() errors as fatal, whereas
> >krb5's mspac_verify() does not. This is all the information that I have
> >gathered. It looks to me like sssd-client (sssd_pac_plugin.so) is not
> >doing this right, but I'm looking forward to your comments.
> I think you are on the right way here. We need to shortcut to return 0
> there instead of EINVAL because unverified PAC wouldn't need to be added
> to the cache.
> 
> Jakub, can you change the    if (kerr != 0) {
>        return EINVAL;
>    }
> 
> to    if (kerr != 0) {
>        return 0;
>    }
> 
> here?
> 
> Unfortunately, since tracing code is not available outside internals of
> libkrb5, we cannot inject TRACE_MSPAC_VERIFY_FAIL(kcontext, kerr); here.

Thanks for looking into the issue. Attached is a patch that changes the
verification failure. Thomas, can you test the patch? If not, what
version on what OS are you running? Perhaps I can prepare you a test
build..
-------------- next part --------------
>From f3ea702a3a7cb0f83bc95d7af61109036ddcc577 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Mon, 4 Aug 2014 13:36:42 +0200
Subject: [PATCH] PAC: krb5_pac_verify failures should not be fatal

As noted in the MIT KRB5 documentation, some servers send PAC with no
checksum, therefire the PAC validation should not be fatal, instead, we
should treat a failure from krb5_pac_verify as if there was no PAC at
all.

Reported on sssd-devel by Thomas Sondergaard
---
 src/sss_client/sssd_pac.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/src/sss_client/sssd_pac.c b/src/sss_client/sssd_pac.c
index 469758a65c1ec3502becb48175753a965a48773c..d1790df50465772e1f310f84e7e6b0e364192720 100644
--- a/src/sss_client/sssd_pac.c
+++ b/src/sss_client/sssd_pac.c
@@ -151,7 +151,16 @@ static krb5_error_code sssdpac_verify(krb5_context kcontext,
                            req->ticket->enc_part2->times.authtime,
                            req->ticket->enc_part2->client, key, NULL);
     if (kerr != 0) {
-        return EINVAL;
+        /* The krb5 documentation says:
+         * A checksum mismatch can occur if the PAC was copied from a
+         * cross-realm TGT by an ignorant KDC; also Apple Mac OS X Server
+         * Open Directory (as of 10.6) generates PACs with no server checksum
+         * at all. One should consider not failing the whole authentication
+         * because of this reason, but, instead, treating the ticket as
+         * if it did not contain a PAC or marking the PAC information as
+         * non-verified.
+         */
+        return 0;
     }
 
     sss_data.len = sssdctx->data.length;
-- 
1.9.3



More information about the sssd-devel mailing list