Skip Menu |
 

From: gsu@UU.NET
Date: Tue, 6 Jan 2004 14:32:35 -0500 (EST)
To: krb5-bugs@mit.edu
Cc: grace.su@mci.com
Subject: bug in krb5_cc_remove_cred API?

Hi,

I am coding a test program that calls krb5_cc_remove_cred and
encountered a problem. Checking out the source
code (krb5-1.3.1.tar) that I downloaded from your site, I believe
the problem is caused by a bug in the code.
The file is src/lib/krb5/ccache/ccfns.c
The function is krb5_cc_remove_cred. This function calls
cache->ops->remove_cred without checking if cache->ops->remove_cred
is NULL. In fact cache->ops->remove_cred is NULL, hence calling
program core dumps. cache->ops is defined as krb5_fcc_ops in
src/lib/krb5/ccache/cc_file.c and the remove_cred entry is NULL.

Please let me know if I am correct or I missed anything.
Thank you.

Grace Su
grace.su@mci.com
The cc_remove field of every implementation of krb5_cc_ops is NULL.
This is because there are no functions defined to implement the
cc_remove functionality. This has been true since the very beginning.
Clearly no one calls this function.

This can be fixed in one of two ways. Add checks to the wrapper
functions to test for NULL and return an error if NULL is found; or add
the functions to each of the implementations and have the
implementations return an error.

My preference is to add functions to each of the implementations. The
hardest question to answer is which error should be returned. I do not
believe any of the existing error values is appropriate and a new error:
KRB5_CC_NOSUPP should be added.
To: rt@krbdev.mit.edu
Cc:
Subject: Re: [krbdev.mit.edu #2106] bug in krb5_cc_remove_cred API?
From: Sam Hartman <hartmans@mit.edu>
Date: Tue, 06 Jan 2004 15:23:55 -0500
RT-Send-Cc:
Well, it is not really clear to me if this is a bug or missing code.
We certainly don't implement remove_cred, so there is no way your test
program could successfully remove a credential.

Clearly we should eventually implement that code and its absence is a
bug.

Having the unimplemented API segfault definitely does draw the missing
functionality to one's attention. It's true that an assertion would
be cleaner than a null pointer dereference.
The alternative would be to return an error indicating the API is
unimplemented.
Date: Tue, 06 Jan 2004 15:45:46 -0500
From: Jeffrey Altman <jaltman@columbia.edu>
To: rt-comment@krbdev.mit.edu
Cc: gsu@UU.NET, krb5-prs@MIT.EDU
Subject: Re: [krbdev.mit.edu #2106] bug in krb5_cc_remove_cred API?
RT-Send-Cc:
One could easily argue that if the desire is to leave a function
unimplemented that the best way to indicate that would be to
surround the function declaration in the header with

   #ifdef KRB5_UNIMPLEMENTED_API
   #endif


From: gsu@UU.NET
Date: Tue, 6 Jan 2004 15:58:55 -0500 (EST)
To: Sam Hartman via RT <rt-comment@krbdev.mit.edu>
Cc: gsu@UU.NET, krb5-prs@mit.edu
Subject: Re: [krbdev.mit.edu #2106] bug in krb5_cc_remove_cred API?
RT-Send-Cc:

Returning an error would certainly be much cleaner than a null pointer
reference. I was so puzzled that I checked the API guide and my code
repeatly. If no implementation is intended, why advertise it in the
guide?

So there is no way that I can remove any expired credential?



On Tue, 6 Jan 2004, Sam Hartman via RT wrote:

Show quoted text
> Well, it is not really clear to me if this is a bug or missing code.
> We certainly don't implement remove_cred, so there is no way your test
> program could successfully remove a credential.
>
> Clearly we should eventually implement that code and its absence is a
> bug.
>
> Having the unimplemented API segfault definitely does draw the missing
> functionality to one's attention. It's true that an assertion would
> be cleaner than a null pointer dereference.
> The alternative would be to return an error indicating the API is
> unimplemented.
>
>
To: rt-comment@krbdev.mit.edu
Subject: Re: [krbdev.mit.edu #2106] bug in krb5_cc_remove_cred API?
From: Sam Hartman <hartmans@mit.edu>
Date: Tue, 06 Jan 2004 16:00:09 -0500
RT-Send-Cc:
Show quoted text
>>>>> "Jeffrey" == Jeffrey Altman via RT <rt-comment@krbdev.mit.edu> writes:

Show quoted text
Jeffrey> My preference is to add functions to each of the
Jeffrey> implementations. The hardest question to answer is which
Jeffrey> error should be returned. I do not believe any of the
Jeffrey> existing error values is appropriate and a new error:
Jeffrey> KRB5_CC_NOSUPP should be added.

But I don't object to either proposal.
To: rt@krbdev.mit.edu
Subject: Re: [krbdev.mit.edu #2106] bug in krb5_cc_remove_cred API?
From: Sam Hartman <hartmans@mit.edu>
Date: Tue, 06 Jan 2004 16:14:23 -0500
RT-Send-Cc:
Show quoted text
>>>>> "gsu" == gsu <gsu@UU.NET> writes:

Show quoted text
gsu> Returning an error would certainly be much cleaner than a
gsu> null pointer reference. I was so puzzled that I checked the
gsu> API guide and my code repeatly. If no implementation is
gsu> intended, why advertise it in the guide?

An implementation is intended. The API guide in doc/api does not
particularly correspond to the source code. We do not really have API
documentation to speak of at this time.

Show quoted text
gsu> So there is no way that I can remove any expired credential?

Correct, but it is probably not a major problem; expired credentials
will not be used. If your cache is getting too full, remove all the
credentials and get a new TGT.
Date: Tue, 06 Jan 2004 16:49:54 -0500
From: Jeffrey Altman <jaltman@columbia.edu>
To: rt-comment@krbdev.mit.edu
Cc: krb5-prs@MIT.EDU
Subject: Re: [krbdev.mit.edu #2106] bug in krb5_cc_remove_cred API?
RT-Send-Cc:
Sam:

I have taken this ticket and will add functions to each implementation
which return a new KRB5_CC_NOSUPP error code. I will check in the changes
and tag them for 1.3.2.

- Jeff
Download smime.p7s
application/x-pkcs7-signature 3.3KiB

Message body not shown because it is not plain text.

From: gsu@UU.NET
Date: Tue, 6 Jan 2004 17:14:07 -0500 (EST)
To: Sam Hartman via RT <rt-comment@krbdev.mit.edu>
Cc: gsu@UU.NET, krb5-prs@mit.edu
Subject: Re: [krbdev.mit.edu #2106] bug in krb5_cc_remove_cred API?
RT-Send-Cc:


On Tue, 6 Jan 2004, Sam Hartman via RT wrote:

Show quoted text
> gsu> So there is no way that I can remove any expired credential?
>
> Correct, but it is probably not a major problem; expired credentials
> will not be used. If your cache is getting too full, remove all the
> credentials and get a new TGT.
>

I noticed that if there are more than one credentials for the same server,
krb5_get_credentials returns the first one found which may be expired.
I have to use krb5_cc_retrieve_cred with KRB5_TC_MATCH_TIMES option
to get the good credential and send to the server for authentication.
Since I have to keep getting new service ticket, I thought it would be
nice if I can remove all old ones.

Thank you for the info.
From: jaltman@mit.edu
Subject: CVS Commit
Add stub function implementations to support krb5_cc_remove_cred() which
would cause a null pointer dereference if called. The new KRB5_CC_NOSUPP
error is returned to indicate the lack of implementation.


To generate a diff of this commit:



cvs diff -r5.93 -r5.94 krb5/src/lib/krb5/ccache/ChangeLog
cvs diff -r5.27 -r5.28 krb5/src/lib/krb5/ccache/cc_file.c
cvs diff -r5.11 -r5.12 krb5/src/lib/krb5/ccache/cc_memory.c
cvs diff -r5.5 -r5.6 krb5/src/lib/krb5/ccache/cc_mslsa.c
cvs diff -r5.96 -r5.97 krb5/src/lib/krb5/error_tables/ChangeLog
cvs diff -r5.74 -r5.75 krb5/src/lib/krb5/error_tables/krb5_err.et
From: jaltman@mit.edu
Subject: CVS Commit
fix typos


To generate a diff of this commit:



cvs diff -r5.28 -r5.29 krb5/src/lib/krb5/ccache/cc_file.c
cvs diff -r5.12 -r5.13 krb5/src/lib/krb5/ccache/cc_memory.c
cvs diff -r5.6 -r5.7 krb5/src/lib/krb5/ccache/cc_mslsa.c
To: rt@krbdev.mit.edu
Cc:
Subject: Re: [krbdev.mit.edu #2106] bug in krb5_cc_remove_cred API?
From: Sam Hartman <hartmans@mit.edu>
Date: Tue, 06 Jan 2004 19:41:48 -0500
RT-Send-Cc:
Show quoted text
>>>>> "gsu@UU" == gsu@UU NET via RT <rt-comment@krbdev.mit.edu> writes:
gsu@UU> I noticed that if there are more than one credentials for
gsu@UU> the same server, krb5_get_credentials returns the first
gsu@UU> one found which may be expired. I have to use
gsu@UU> krb5_cc_retrieve_cred with KRB5_TC_MATCH_TIMES option to
gsu@UU> get the good credential and send to the server for
gsu@UU> authentication. Since I have to keep getting new service
gsu@UU> ticket, I thought it would be nice if I can remove all old
gsu@UU> ones.

The logic used by krb5_mk_req in 1.3.x should correctly use only
unexpired credentials. Previous versions of Kerberos did not tend to
do this correctly.
From: gsu@UU.NET
Date: Wed, 7 Jan 2004 11:11:25 -0500 (EST)
To: Sam Hartman via RT <rt-comment@krbdev.mit.edu>
Cc: gsu@UU.NET, krb5-prs@mit.edu
Subject: Re: [krbdev.mit.edu #2106] bug in krb5_cc_remove_cred API?
RT-Send-Cc:
Download (untitled) / with headers
text/plain 1.3KiB


On Tue, 6 Jan 2004, Sam Hartman via RT wrote:

Show quoted text
> >>>>> "gsu@UU" == gsu@UU NET via RT <rt-comment@krbdev.mit.edu> writes:
> gsu@UU> I noticed that if there are more than one credentials for
> gsu@UU> the same server, krb5_get_credentials returns the first
> gsu@UU> one found which may be expired. I have to use
> gsu@UU> krb5_cc_retrieve_cred with KRB5_TC_MATCH_TIMES option to
> gsu@UU> get the good credential and send to the server for
> gsu@UU> authentication. Since I have to keep getting new service
> gsu@UU> ticket, I thought it would be nice if I can remove all old
> gsu@UU> ones.
>
> The logic used by krb5_mk_req in 1.3.x should correctly use only
> unexpired credentials. Previous versions of Kerberos did not tend to
> do this correctly.
>
>

Is this new logic in release after 1.3.1?

I am looking at the 1.3.1 source tree. Suppose I have 2 service tickets,
the first one is expired.
krb5_mk_req calls krb5_get_credentials which returns the expired ticket.
krb5_mk_req calls krb5_mk_req_extended with this expired credential.
krb5_mk_req_extended calls krb5_validate_times.
krb5_validate_times returns KRB5KRB_AP_ERR_TKT_EXPIRED.
krb5_mk_req returns KRB5KRB_AP_ERR_TKT_EXPIRED to the caller.

So instead of calling krb5_mk_req, I call krb5_cc_retrieve_cred, then
call krb5_mk_req_extended with the valid credential.
To: rt@krbdev.mit.edu
Cc:
Subject: Re: [krbdev.mit.edu #2106] bug in krb5_cc_remove_cred API?
From: Sam Hartman <hartmans@mit.edu>
Date: Wed, 07 Jan 2004 14:25:36 -0500
RT-Send-Cc:

That's not the behavior I see although it is the behavior in pre 1.3.x
Kerberos.

I set up my host to have 10-minute lifetime tickets and things
continue to work. Here's klist output:

01/07/04 13:57:14 01/07/04 14:07:14 host/luminous.mit.edu@SUCHDAMAGE.ORG
01/07/04 14:15:16 01/07/04 23:58:01 imap/solipsist-nation.suchdamage.org@SUCHDAMAGE.ORG
01/07/04 14:22:29 01/07/04 14:32:29 host/luminous.mit.edu@SUCHDAMAGE.ORG
From: tlyu@mit.edu
Subject: CVS Commit
pullup from trunk


To generate a diff of this commit:



cvs diff -r5.82.2.4 -r5.82.2.5 krb5/src/lib/krb5/ccache/ChangeLog
cvs diff -r5.26 -r5.26.2.1 krb5/src/lib/krb5/ccache/cc_file.c
cvs diff -r5.11 -r5.11.2.1 krb5/src/lib/krb5/ccache/cc_memory.c
cvs diff -r5.3.2.2 -r5.3.2.3 krb5/src/lib/krb5/ccache/cc_mslsa.c
cvs diff -r5.91.2.4 -r5.91.2.5
krb5/src/lib/krb5/error_tables/ChangeLog
cvs diff -r5.72.2.2 -r5.72.2.3
krb5/src/lib/krb5/error_tables/krb5_err.et