Strange behavior of libkrb5 since karmic ...

Bug #489418 reported by Frederic Urban
22
This bug affects 3 people
Affects Status Importance Assigned to Milestone
krb5 (Debian)
Fix Released
Unknown
krb5 (Ubuntu)
Fix Released
Undecided
Unassigned
Karmic
Fix Released
High
Thierry Carrez

Bug Description

Since karmic libkrb5 is acting weird. When a user fail to give a valid password, it loops until account is locked ... After some wireshark capture and some comparaison between 1.6 (from Jaunty) et the 1.7 of Karmic i succeded to make a patch which solve the problem...

In 1.7 while the client get KDC_ERR_PREAUTH_FAILED or KDC_ERR_PREAUTH_REQUIRED from the server it loops until it gets another message (in our case it's KDC_ACCOUNT_LOCKED or something...)

In 1.6 it loops only if it receive KDC_ERR_PREAUTH_REQUIRED which is quite normal...

I'm tired ;) it's 1am here if you need more info just ask here. patch + pcaps in attachement

Btw, there is no problem without our configuration since it's the same we were used to use in jaunty and it was generated by pam-auth-config tool :)

Revision history for this message
Frederic Urban (frederic-urban) wrote :
Revision history for this message
Frederic Urban (frederic-urban) wrote :

Hello, again... Looks like this problem has already been fixed in the MIT krb5 developpement tree. The way they did it is more clean than mine :)

So using my patch is only a workaround until Debian rebuild their package against a newer krb5 release.

Revision history for this message
Frederic Urban (frederic-urban) wrote :

Here you can find the modification done by the dev of krb5 to fix the problem I described above
http://src.mit.edu/fisheye/browse/krb5/trunk/src/lib/krb5/krb/get_in_tkt.c?r1=22888&r2=22890

Revision history for this message
Sam Hartman (hartmans) wrote :

This has been fixed upstream; I will be fixing for Debian shortly.

Changed in krb5 (Ubuntu):
status: New → Confirmed
Changed in krb5 (Debian):
status: Unknown → New
Changed in krb5 (Debian):
status: New → Fix Released
Revision history for this message
Sam Hartman (hartmans) wrote : Re: [Bug 489418] Re: Strange behavior of libkrb5 since karmic ...
Download full text (3.5 KiB)

I released 1.7+dfsg-3 to Debian unstable. That includes a fix to this
bug. I'd recommend that Ubuntu sync that version into a karmic update
once it hits squeeze in order to address this issue. The code changes
between what's in karmic now and 1.7+dfsg-3 are all reasonably
important bug fixes including a number of user visible memory leak
fixes, fixes to the lockout problem and fixes to some rare crashes.
There were no code changes between 1.7 beta3 and 1.7; I have hand
picked patches that resolve important problems people were having for
any code changes since the version in karmic.

I understand you try to be conservative about what you accept in an
update, although I think it will probably be easier to evaluate the
debian diff than to subset the changes I've made. I've tried to show
what all is involved below so you can estimate whether my proposal is
a viable option. Specific patches are all in the debian krb5 git repo
if you do want to subset.

The diffs to the code are reasonably small and
address specific bug fixes:

2 3 src/appl/gssftp/ftpd/ftpd.c
7 0 src/lib/gssapi/spnego/spnego_mech.c
17 13 src/lib/kadm5/srv/server_acl.c
16 25 src/lib/kdb/kdb_default.c
1 1 src/lib/krb5/krb/chpw.c
1 2 src/lib/krb5/krb/get_in_tkt.c
1 1 src/lib/krb5/krb/kerrs.c
3 1 src/lib/krb5/krb/pac.c
2 0 src/lib/krb5/krb/t_pac.c
8 2 src/lib/krb5/rcache/rc_none.c
3 3 src/patchlevel.h
7 0 src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
14 14 src/util/profile/prof_file.c
3 0 src/util/profile/prof_int.h
2 7 src/util/profile/prof_tree.c

Here are the fixes that involve code changes:
  * Several fixes applied after the 1.7 release:
      - 6506: correctly handle keytab vs stash file
    - 6508: kadmind ACL parsing could reference uninitialized memory
    - 6509: kadmind can reference null pointer on ACL error
    - 6511: uninitialized memory passed to krb5_free_error in change
    password client path
    - 6514: none replay cache memory leak
    - 6515: profile library mutex performance improvements
    - 6541: memory leak in PAC verify code
    - 6542: Check for null characters in pkinit certs
    - 6543: login vs user order in ftpd sometimes wrong
    - 6551: Memory leak in spnego accept_sec_context error path
  * Avoid locking out accounts on PREAUTH_FAILED, Closes: #557979, (LP:
    #489418)

If you do not choose to accept the full Debian version, I strongly
recommend you take at least the fix to the lockout bug, 6543 (can
cause people to be unable to log into ftpd), 6542 (security concern
about accepting bogus certificates for authentication), and all the
memory leaks.

In addition to the code changes, this version includes:

* autoconf was rerun as part of transition from 1.7beta3 to 1.7
9 9 src/appl/libpty/configure
9 9 src/appl/telnet/configure
10 10 src/configure
9 9 src/appl/bsd/configure
9 9 src/appl/gssftp/configure

The following documentation updates were pulled in moving from
1.7.dfsg~beta3 to 1.7. You probably don't strictly need these, but it
should be fairly easy to see they are harmless.
77 25 README
22 3 doc/CHANGES
1021 939 doc/admin-guide.ps
83 2 doc/copyright.texinfo
873 792 doc/install-guide.ps
65 2 doc/krb5-admin.html
165 105 doc/krb5...

Read more...

Revision history for this message
Evan Broder (broder) wrote :
Download full text (4.5 KiB)

This shouldn't be a problem. We're still in sync phase for Ubuntu
Lucid, so the new krb5 package will get automatically pulled in when
it hits Debian testing.

On Mon, Nov 30, 2009 at 3:25 PM, Sam Hartman <email address hidden> wrote:
> I released 1.7+dfsg-3 to Debian unstable.  That includes a fix to this
> bug.  I'd recommend that Ubuntu sync that version into a karmic update
> once it hits squeeze in order to address this issue.  The code changes
> between what's in karmic now and 1.7+dfsg-3 are all reasonably
> important bug fixes including a number of user visible memory leak
> fixes, fixes to the lockout problem and fixes to some rare crashes.
> There were no code changes between 1.7 beta3 and 1.7; I have hand
> picked patches that resolve important problems people were having for
> any code changes since the version in karmic.
>
> I understand you try to be conservative about what you accept in an
> update, although I think it will probably be easier to evaluate the
> debian diff than to subset the changes I've made.  I've tried to show
> what all is involved below so you can estimate whether my proposal is
> a viable option.  Specific patches are all in the debian krb5 git repo
> if you do want to subset.
>
>
> The diffs to the code are reasonably small and
> address specific bug fixes:
>
> 2       3       src/appl/gssftp/ftpd/ftpd.c
> 7       0       src/lib/gssapi/spnego/spnego_mech.c
> 17      13      src/lib/kadm5/srv/server_acl.c
> 16      25      src/lib/kdb/kdb_default.c
> 1       1       src/lib/krb5/krb/chpw.c
> 1       2       src/lib/krb5/krb/get_in_tkt.c
> 1       1       src/lib/krb5/krb/kerrs.c
> 3       1       src/lib/krb5/krb/pac.c
> 2       0       src/lib/krb5/krb/t_pac.c
> 8       2       src/lib/krb5/rcache/rc_none.c
> 3       3       src/patchlevel.h
> 7       0       src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
> 14      14      src/util/profile/prof_file.c
> 3       0       src/util/profile/prof_int.h
> 2       7       src/util/profile/prof_tree.c
>
> Here are the fixes that involve code changes:
>  * Several fixes applied after the 1.7 release:
>      - 6506: correctly handle keytab vs stash file
>    - 6508: kadmind ACL parsing could reference uninitialized memory
>    - 6509: kadmind can reference null pointer on ACL error
>    - 6511: uninitialized memory passed to krb5_free_error in change
>    password client path
>    - 6514: none replay cache memory leak
>    - 6515: profile library mutex performance improvements
>    - 6541: memory leak in PAC verify code
>    - 6542: Check for null characters in pkinit certs
>    - 6543: login vs user order in ftpd sometimes wrong
>    - 6551: Memory leak in spnego accept_sec_context error path
>  *  Avoid locking out accounts on PREAUTH_FAILED, Closes: #557979, (LP:
>    #489418)
>
> If you do not choose to accept the full Debian version, I strongly
> recommend you take at least the fix to the lockout bug, 6543 (can
> cause people to be unable to log into ftpd), 6542 (security concern
> about accepting bogus certificates for authentication), and all the
> memory leaks.
>
> In addition to the code changes, this version includes:
>
>
> * autoconf was rerun as part...

Read more...

Revision history for this message
Philipp Kaluza (pixelpapst) wrote :

I think Sam was proposing to pull this new version as an update into karmic. The SRU guidelines state that this should hit lucid first, before we ask for an SRU. (Since this bug is a bit high-profile, it might make sense to speed up this process by uploading a dedicated bugfix package to lucid in any case.)
The changes sure look reasonable enough, debdiff attached (except that you have to wade through a lot of .ps diff lines).

Revision history for this message
Sam Hartman (hartmans) wrote :

>>>>> "Evan" == Evan Broder <email address hidden> writes:

    Evan> This shouldn't be a problem. We're still in sync phase for
    Evan> Ubuntu Lucid, so the new krb5 package will get automatically
    Evan> pulled in when it hits Debian testing.

I understand that. The user proposed and I agree at least that this
bug and I believe a number of the other bugs are worth a karmic
update.

Revision history for this message
Evan Broder (broder) wrote :

I think the patch is a little extensive to be directly uploaded as a Karmic SRU, but I'll look at pulling a SRU patch together for just the bugfixes.

Changed in krb5 (Ubuntu):
assignee: nobody → Evan Broder (broder)
status: Confirmed → In Progress
Revision history for this message
Sam Hartman (hartmans) wrote :

>>>>> "Evan" == Evan Broder <email address hidden> writes:

    Evan> I think the patch is a little extensive to be directly
    Evan> uploaded as a Karmic SRU, but I'll look at pulling a SRU
    Evan> patch together for just the bugfixes.

If you don't want to take the full patch, then take a look at
upstream_post_1_7_fixes branch in debian git. That should contain the
bug fixes.

Revision history for this message
Evan Broder (broder) wrote :

Ok - I've pulled out just the actual source patches. Since these are all bugfixes, I think they're reasonable for an SRU, so I'll subscribe ubuntu-sru and see what they think.

Changed in krb5 (Ubuntu):
assignee: Evan Broder (broder) → nobody
status: In Progress → Confirmed
Revision history for this message
Colin Whittaker (colin-netech) wrote :

I built new packages based on Evan's debdiff and have deployed them.
This removed all the impact we were seeing with single password errors causing account lockout.

This bug has massive impact on user experience and I would encourage the update to be released.

Revision history for this message
Sam Hartman (hartmans) wrote :

>>>>> "Colin" == Colin Whittaker <email address hidden> writes:

    Colin> I built new packages based on Evan's debdiff and have
    Colin> deployed them. This removed all the impact we were seeing
    Colin> with single password errors causing account lockout.

    Colin> This bug has massive impact on user experience and I would
    Colin> encourage the update to be released.

This should make it into lucid as soon as the sync happens: the new
packages are in Debian testing.

I agree with you getting an SRU for Karmic would be a good idea.

Revision history for this message
Steve Langasek (vorlon) wrote :

This package is synced to lucid today.

Changed in krb5 (Ubuntu):
status: Confirmed → Fix Released
Mathias Gug (mathiaz)
Changed in krb5 (Ubuntu Karmic):
importance: Undecided → High
Revision history for this message
Evan Broder (broder) wrote :

Here's my last debdiff rebased on top of the recent security update.

I've also dispatched builds to my PPA (https://launchpad.net/~broder/+archive/ubuntu-tests), which should be running shortly.

Revision history for this message
Martin Pitt (pitti) wrote :

Server team, can you please vet and upload this patch? Thanks!

Changed in krb5 (Ubuntu Karmic):
assignee: nobody → Canonical Server Team (canonical-server)
Thierry Carrez (ttx)
Changed in krb5 (Ubuntu Karmic):
assignee: Canonical Server Team (canonical-server) → Thierry Carrez (ttx)
status: New → In Progress
Revision history for this message
Thierry Carrez (ttx) wrote :

Looks sane, if SRU team doesn't mind that it fixes a lot more issues than just the two reported bugs:
- libkrb5-dev depends on libkrb5client6 (LP: #472080)
- Avoid locking out accounts on PREAUTH_FAILED, Closes: #557979 (LP: #489418)
- 6506: correctly handle keytab vs stash file
- 6508: kadmind ACL parsing could reference uninitialized memory
- 6509: kadmind can reference null pointer on ACL error
- 6511: uninitialized memory passed to krb5_free_error in change password client path
- 6514: none replay cache memory leak
- 6515: profile library mutex performance improvements
- 6541: memory leak in PAC verify code
- 6542: Check for null characters in pkinit certs
- 6543: login vs user order in ftpd sometimes wrong
- 6551: Memory leak in spnego accept_sec_context error path

Uploading to karmic-proposed, thanks a lot Evan !

Revision history for this message
Martin Pitt (pitti) wrote :

Thierry, I rejected the upload. Please reupload against the 0.3 security update in karmic-security.

Revision history for this message
Thierry Carrez (ttx) wrote :

Re-sponsored as 0.4, rebased against recent karmic-security upload.

Revision history for this message
Martin Pitt (pitti) wrote : Please test proposed package

Accepted krb5 into karmic-proposed, the package will build now and be available in a few hours. Please test and give feedback here. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Thank you in advance!

Changed in krb5 (Ubuntu Karmic):
status: In Progress → Fix Committed
tags: added: verification-needed
Revision history for this message
Colin Whittaker (colin-netech) wrote :

this is working correctly - can this be pushed to karmic-updates

Martin Pitt (pitti)
tags: added: verification-done
removed: verification-needed
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package krb5 - 1.7dfsg~beta3-1ubuntu0.4

---------------
krb5 (1.7dfsg~beta3-1ubuntu0.4) karmic-proposed; urgency=low

   * Cherry-pick various fixes from Debian:
      - libkrb5-dev depends on libkrb5client6 (LP: #472080)
      - Avoid locking out accounts on PREAUTH_FAILED, Closes: #557979 (LP:
        #489418)
      - 6506: correctly handle keytab vs stash file
      - 6508: kadmind ACL parsing could reference uninitialized memory
      - 6509: kadmind can reference null pointer on ACL error
      - 6511: uninitialized memory passed to krb5_free_error in change
        password client path
      - 6514: none replay cache memory leak
      - 6515: profile library mutex performance improvements
      - 6541: memory leak in PAC verify code
      - 6542: Check for null characters in pkinit certs
      - 6543: login vs user order in ftpd sometimes wrong
      - 6551: Memory leak in spnego accept_sec_context error path
 -- Evan Broder <email address hidden> Thu, 07 Jan 2010 21:28:45 -0500

Changed in krb5 (Ubuntu Karmic):
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.