kinit error message (Heimdal 7.4.0)

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

kinit error message (Heimdal 7.4.0)

Harald Barth-2

Well, I' making some progress.

The context I get when krb5_init_creds_get() returns is

$23 = {etypes = 0x0, cfg_etypes = 0x0, etypes_des = 0x0, as_etypes = 0x0,
  tgs_etypes = 0x0, permitted_enctypes = 0x0, default_realms = 0x0,
  max_skew = 300, kdc_timeout = 30, host_timeout = 3, max_retries = 3,
  kdc_sec_offset = 0, kdc_usec_offset = 0, cf = 0x60bb40, et_list = 0x60a0c0,
  warn_dest = 0x0, debug_dest = 0x0, cc_ops = 0x60ebe0, num_cc_ops = 6,
  http_proxy = 0x0, time_fmt = 0x7ffff79abada "%Y-%m-%dT%H:%M:%S",
  log_utc = 0, default_keytab = 0x7ffff79ab985 "FILE:/etc/krb5.keytab",
  default_keytab_modify = 0x0, use_admin_kdc = 0, extra_addresses = 0x0,
  scan_interfaces = 1, srv_lookup = 1, srv_try_txt = 0, fcache_vno = 0,
  num_kt_types = 6, kt_types = 0x60ecd0, date_fmt = 0x7ffff79abaf8 "%Y-%m-%d",
  error_string = 0x6112e0 "No ENC-TS found", error_code = -1765328383,
(...)

And because the return code ret is the same as the error_code in the
context, krb5_get_error_message() just copies the string from the
context. However, if krb5_get_error_message() does its own lookup of
-1765328383 it gets "Client's entry in database has expired" which is
more like it. But where does "No ENC-TS found" come from and why is it
"better" than the own lookup?

Of course the error string printing function could return all text it
finds and leave the interpretation to the user. Like this, patch at
the end.

$ /usr/heimdal/bin/kinit
haba/[hidden email]'s Password: <correct password but principal expired>
kinit: krb5_get_init_creds: Client's entry in database has expired, No ENC-TS found


String operations i C are always fun.

Harald.


--- error_string.c.orig 2017-07-11 07:14:16.000000000 +0200
+++ error_string.c      2017-10-05 13:46:04.654728188 +0200
@@ -234,8 +234,8 @@
         }
        HEIMDAL_MUTEX_unlock(&context->mutex);
 
-        if (str)
-            return str;
+        /*if (str)
+         return str; return later*/
     }
     else
     {
@@ -249,10 +249,19 @@
     if (free_context)
         krb5_free_context(context);
 
-    if (cstr)
-        return strdup(cstr);
+    if (!cstr)
+       cstr = error_message(code);
+
+    if (str && !cstr)
+       return str;
 
-    cstr = error_message(code);
+    if (str && cstr)
+       if (strncmp(str, cstr, sizeof(buf)) == 0)
+           return str;
+       else
+           strncat(cstr, ", ", sizeof(buf));
+           return strdup(strncat(cstr, str, sizeof(buf)));
+
     if (cstr)
         return strdup(cstr);
 


Harald.
Reply | Threaded
Open this post in threaded view
|

Re: kinit error message (Heimdal 7.4.0)

Harald Barth-2

I wrote:

> +           strncat(cstr, ", ", sizeof(buf));
> +           return strdup(strncat(cstr, str, sizeof(buf)));

which is broken and buggy usage of strncat(), so don't copy that.

Harald.
Reply | Threaded
Open this post in threaded view
|

Re: kinit error message (Heimdal 7.4.0)

Greg Hudson
In reply to this post by Harald Barth-2
On 10/05/2017 07:52 AM, Harald Barth wrote:
> And because the return code ret is the same as the error_code in the
> context, krb5_get_error_message() just copies the string from the
> context. However, if krb5_get_error_message() does its own lookup of
> -1765328383 it gets "Client's entry in database has expired" which is
> more like it. But where does "No ENC-TS found" come from and why is it
> "better" than the own lookup?

I didn't find anything like "No ENC-TS found" in the Heimdal source
code, so my best guess is that this is coming from
rd_error.c:krb5_error_from_rd_error() which does:

    ret = error->error_code;
    if (error->e_text != NULL) {
        krb5_set_error_message(context, ret, "%s", *error->e_text);
    } ...

If my theory is correct, the KDC is sending unhelpful e_text and/or
Heimdal is too trusting in using the e_text in preference to the string
corresponding to the error code.  In this case, concatenating the error
code string with the e_text would produce a better result but not an
ideal one, as "No ENC-TS found" shouldn't appear in the error message at
all.
Reply | Threaded
Open this post in threaded view
|

Re: kinit error message (Heimdal 7.4.0)

Jeffrey Hutzelman
On October 5, 2017 12:06:51 PM EDT, Greg Hudson <[hidden email]> wrote:

>On 10/05/2017 07:52 AM, Harald Barth wrote:
>> And because the return code ret is the same as the error_code in the
>> context, krb5_get_error_message() just copies the string from the
>> context. However, if krb5_get_error_message() does its own lookup of
>> -1765328383 it gets "Client's entry in database has expired" which is
>> more like it. But where does "No ENC-TS found" come from and why is
>it
>> "better" than the own lookup?
>
>I didn't find anything like "No ENC-TS found" in the Heimdal source
>code, so my best guess is that this is coming from
>rd_error.c:krb5_error_from_rd_error() which does:
>
>    ret = error->error_code;
>    if (error->e_text != NULL) {
>        krb5_set_error_message(context, ret, "%s", *error->e_text);
>    } ...
>
>If my theory is correct, the KDC is sending unhelpful e_text and/or
>Heimdal is too trusting in using the e_text in preference to the string
>corresponding to the error code.

Both, I think. kinit (and other clients) ought to report something like "error_message (e_text)", unless the e_text is empty or the same as the message for the error code. of course, more complex variations are possible, what with both locally- and KDC-generated error codes and additional messages. but just blindly using the e_text and nothing else is clearly wrong.

That said, the KDC should not be sending this particular e_text in this situation. I'll bet there's a loop that looks for suitable PA data, and that message gets produced if it finishes without finding any, even though the problem is something else entirely.

-- Jeff




  In this case, concatenating the error
>code string with the e_text would produce a better result but not an
>ideal one, as "No ENC-TS found" shouldn't appear in the error message
>at
>all.


Reply | Threaded
Open this post in threaded view
|

Re: kinit error message (Heimdal 7.4.0)

Nico Williams
In reply to this post by Greg Hudson
On Thu, Oct 05, 2017 at 12:06:51PM -0400, Greg Hudson wrote:
> If my theory is correct, the KDC is sending unhelpful e_text and/or
> Heimdal is too trusting in using the e_text in preference to the string
> corresponding to the error code.  In this case, concatenating the error
> code string with the e_text would produce a better result but not an
> ideal one, as "No ENC-TS found" shouldn't appear in the error message at
> all.

I concluded it's the KDC, but I couldn't find where that's produced.  My
theory was that it was _kdc_as_rep(), when it couldn't find a PA, but I
didn't see how.

Nico
--
Reply | Threaded
Open this post in threaded view
|

Re: kinit error message (Heimdal 7.4.0)

Nico Williams
In reply to this post by Jeffrey Hutzelman
On Thu, Oct 05, 2017 at 12:28:29PM -0400, Jeffrey Hutzelman wrote:
> Both, I think. kinit (and other clients) ought to report something
> like "error_message (e_text)", unless the e_text is empty or the same
> as the message for the error code. of course, more complex variations
> are possible, what with both locally- and KDC-generated error codes
> and additional messages. but just blindly using the e_text and nothing
> else is clearly wrong.

Yes.  (And let's not even get into how to localize e-text.)

> That said, the KDC should not be sending this particular e_text in
> this situation. I'll bet there's a loop that looks for suitable PA
> data, and that message gets produced if it finishes without finding
> any, even though the problem is something else entirely.

I looked for that and couldn't find it.

There's only one place where the string "ENC-TS" occurs; I could not
find how it gets turned into "No ENC-TS found", now could I find a
printf format string that would do that.

Nico
--
Reply | Threaded
Open this post in threaded view
|

Re: kinit error message (Heimdal 7.4.0)

Harald Barth-2

> I looked for that and couldn't find it.

I may have found it! The 1.5.X KDC has a mile long function
_kdc_as_rep() which in the middle somewhere has

 e_text = "No ENC-TS found";

That function looks quite different in 7.X....

> So, to reproduce just mark a principal as expired and try to kinit?

Yes. Your newer KDC may fill in something else in the
context->error_string or context->error_code compared to the return
value. So if you put a breakpoint at krb5_get_error_message and
compare these with the variable "code" which should be -1765328383.

Harald.


Reply | Threaded
Open this post in threaded view
|

Re: kinit error message (Heimdal 7.4.0)

Nico Williams
On Thu, Oct 05, 2017 at 08:01:00PM +0200, Harald Barth wrote:
> I may have found it! The 1.5.X KDC has a mile long function
> _kdc_as_rep() which in the middle somewhere has
>
>  e_text = "No ENC-TS found";

Ahh, ok, got it.

Nico
--
Reply | Threaded
Open this post in threaded view
|

Re: kinit error message (Heimdal 7.4.0)

Harald Barth-2

I may have not yet shared my final version of the patch which gives
the user as much error message(s) that are extractable from both the
context and the return code:

$ diff -u error_string.c.orig error_string.c
--- error_string.c.orig 2017-07-11 07:14:16.000000000 +0200
+++ error_string.c      2017-10-05 21:04:50.064079947 +0200
@@ -234,8 +234,6 @@
         }
        HEIMDAL_MUTEX_unlock(&context->mutex);
 
-        if (str)
-            return str;
     }
     else
     {
@@ -249,10 +247,26 @@
     if (free_context)
         krb5_free_context(context);
 
-    if (cstr)
-        return strdup(cstr);
+    if (!cstr) {
+       if (cstr = error_message(code)) {
+           strlcpy(buf, cstr, sizeof(buf));
+           cstr = buf;
+       }
+    }
 
-    cstr = error_message(code);
+    if (str && !cstr)
+       return str;
+
+    if (str && cstr) {
+       if (strncmp(str, cstr, sizeof(buf)) == 0)
+           return str;
+       else {
+           strlcat(buf, ", ", sizeof(buf));
+           strlcat(buf,  str, sizeof(buf));
+           return strdup(cstr);
+       }
+    }
+
     if (cstr)
         return strdup(cstr);
 
So...whatyouthinkaboutthat? Yes, str*foo() in C is a pain.

Harald.
Reply | Threaded
Open this post in threaded view
|

Re: kinit error message (Heimdal 7.4.0)

Jeffrey Altman-2
When possible, please submit patches and requests for review via GitHub
as a pull request.   This will ensure the patch is not lost and provides
the ability to apply comments both on the pull request as well as
comment inline to the patch.

The patch will be tested by the Continuous Integration system and the
results posted back to the conversation.

Thanks.


On 10/26/2017 8:44 AM, Harald Barth wrote:

>
> I may have not yet shared my final version of the patch which gives
> the user as much error message(s) that are extractable from both the
> context and the return code:
>
> $ diff -u error_string.c.orig error_string.c
> --- error_string.c.orig 2017-07-11 07:14:16.000000000 +0200
> +++ error_string.c      2017-10-05 21:04:50.064079947 +0200
> @@ -234,8 +234,6 @@
>          }
>         HEIMDAL_MUTEX_unlock(&context->mutex);
>  
> -        if (str)
> -            return str;
>      }
>      else
>      {
> @@ -249,10 +247,26 @@
>      if (free_context)
>          krb5_free_context(context);
>  
> -    if (cstr)
> -        return strdup(cstr);
> +    if (!cstr) {
> +       if (cstr = error_message(code)) {
> +           strlcpy(buf, cstr, sizeof(buf));
> +           cstr = buf;
> +       }
> +    }
>  
> -    cstr = error_message(code);
> +    if (str && !cstr)
> +       return str;
> +
> +    if (str && cstr) {
> +       if (strncmp(str, cstr, sizeof(buf)) == 0)
> +           return str;
> +       else {
> +           strlcat(buf, ", ", sizeof(buf));
> +           strlcat(buf,  str, sizeof(buf));
> +           return strdup(cstr);
> +       }
> +    }
> +
>      if (cstr)
>          return strdup(cstr);
>  
> So...whatyouthinkaboutthat? Yes, str*foo() in C is a pain.
>
> Harald.
>


smime.p7s (5K) Download Attachment