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

[Thomas Sondergaard]

On 04-08-2014 15:38, Jakub Hrozek wrote:
>  On Mon, Aug 04, 2014 at 03:10:17PM +0200, Thomas Sondergaard wrote:
> > On 04-08-2014 13:40, Jakub Hrozek wrote:
> >> 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(a)mit.edu&gt;
> >>>> 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..
> >
> > Don't worry about a test build. I will test your patch.
> >
> > Regards,
> > Thomas
>
>  Sorry about the delay, I was in a meeting until now. There is a test
>  build available here:
>       https://jhrozek.fedorapeople.org/sssd-test-builds/sssd-pac-verify/ 
Your patch solves the problem. I have not tested whether it introduces 
other problems, but my code works now with the new sssd-client installed.

Thanks!

Thomas