[19611] in Kerberos_V5_Development
Re: [PATCH] Add missing KDC "status" on some fail conditions
daemon@ATHENA.MIT.EDU (Samuel Cabrero)
Wed Jul 12 11:59:03 2017
Date: Wed, 12 Jul 2017 17:58:42 +0200
From: Samuel Cabrero <scabrero@suse.de>
To: Greg Hudson <ghudson@mit.edu>
Message-Id: <1499875122.4485.0@imap.suse.de>
In-Reply-To: <2b16f511-f30f-d214-bfa6-f6e45e6d00c0@mit.edu>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="=-ulL+aGuX9TFd/TFjsR05"
Cc: krbdev@mit.edu
Errors-To: krbdev-bounces@mit.edu
--=-ulL+aGuX9TFd/TFjsR05
Content-Type: text/plain; charset=us-ascii; format=flowed
Hi Greg,
yes, I triggered the assertion by testing samba 4.7rc1 built to use MIT
instead bundled Heimdal.
Once a Windows 7 machine is joined to the domain, every time it powers
up sends a TGS-REQ which triggers it. This is the backtrace:
#0 __GI_raise (sig=sig@entry=6) at
../sysdeps/unix/sysv/linux/raise.c:51
#1 0x00007f16a602d0c6 in __GI_abort () at abort.c:78
#2 0x00007f16a602471a in __assert_fail_base (fmt=0x7f16a6163e80
"%s%s%s:%u: %s%sAssertion `%s' failed.\n%n",
assertion=assertion@entry=0x55a129c524f6 "status != NULL",
file=file@entry=0x55a129c523b8 "do_tgs_req.c",
line=line@entry=826, function=function@entry=0x55a129c525d0
<__PRETTY_FUNCTION__.13702> "process_tgs_req")
at assert.c:92
#3 0x00007f16a6024792 in __GI___assert_fail
(assertion=assertion@entry=0x55a129c524f6 "status != NULL",
file=file@entry=0x55a129c523b8 "do_tgs_req.c", line=line@entry=826,
function=function@entry=0x55a129c525d0 <__PRETTY_FUNCTION__.13702>
"process_tgs_req") at assert.c:101
#4 0x000055a129c41212 in process_tgs_req
(handle=handle@entry=0x55a129e592c0 <shandle>,
pkt=pkt@entry=0x55a12b325b38, from=from@entry=0x55a12be8a728,
response=response@entry=0x7ffe74820f18)
at do_tgs_req.c:826
#5 0x000055a129c3eb5e in dispatch (cb=0x55a129e592c0 <shandle>,
local_saddr=<optimized out>,
from=from@entry=0x55a12be8a728, pkt=pkt@entry=0x55a12b325b38,
is_tcp=is_tcp@entry=1,
vctx=vctx@entry=0x55a12b0307e0, respond=0x55a129c4fd60
<process_tcp_response>, arg=0x55a12b325ab0)
at dispatch.c:180
#6 0x000055a129c50019 in process_tcp_connection_read
(ctx=0x55a12b0307e0, ev=0x55a12bc59cd0) at net-server.c:1409
#7 0x00007f16a639fcb8 in verto_fire () from /usr/lib64/libverto.so.1
#8 0x00007f168e472736 in ev_invoke_pending (loop=0x7f168e67ca40
<default_loop_struct>) at ev.c:3288
#9 0x00007f168e475d48 in ev_run (loop=0x7f168e67ca40
<default_loop_struct>, flags=0) at ev.c:3688
#10 0x000055a129c3d98f in main (argc=2, argv=0x7ffe74821288) at
main.c:1065
And the involved data structures:
(gdb) p *request->server
$10 = {magic = 0, realm = {magic = 0, length = 7, data = 0x55a12afa88f0
"SUSE.AD"}, data = 0x55a12afa8770,
length = 1, type = 10}
(gdb) p *header_ticket->enc_part2->client
$11 = {magic = 0, realm = {magic = 0, length = 7, data = 0x55a12b3ab880
"SUSE.AD"}, data = 0x55a12be0ab40,
length = 1, type = 0}
(gdb) p *header_ticket->enc_part2->client->data
$12 = {magic = 0, length = 5, data = 0x55a12afaa970 "WIN7$U"}
(gdb) p *request->server->data
$13 = {magic = 0, length = 13, data = 0x55a12afa8a90 "win7$@SUSE.ADU"}
(gdb) printf "%x", request->kdc_options
40830000
(gdb) print *request
$14 = {magic = 0, msg_type = 12, padata = 0x55a12afadc00, kdc_options =
1082327040, client = 0x0,
server = 0x55a12b7eb890, from = 0, till = 1499852047, rtime = 0,
nonce = 1809206369, nktypes = 5,
ktype = 0x55a12afa8cd0, addresses = 0x0, authorization_data = {magic
= 0, enctype = 0, kvno = 0, ciphertext = {
magic = 0, length = 0, data = 0x0}}, unenc_authdata = 0x0,
second_ticket = 0x55a12afa8b50}
I have traced it down and the function kdc_process_s4u2proxy_req
returns KRB5KDC_ERR_SERVER_NOMATCH without setting the status.
After your comment, this second version version of the patch is more
convenient as it respect the status if already set.
Also, just for reference, I have attached network captures with and
without patch.
Samuel Cabrero / SUSE Labs Samba Team
GPG: D7D6 E259 F91C F0B3 2E61 1239 3655 6EC9 7051 0856
scabrero@suse.com
scabrero@suse.de
On mar, jul 11, 2017 at 9:20 , Greg Hudson <ghudson@mit.edu> wrote:
> On 07/11/2017 01:48 PM, Samuel Cabrero wrote:
>> The process_tgs_req function requires the status string to be set
>> for all fail conditions before jump to "cleanup" label, otherwise
>> it will cause an assertion failure.
>
> In all of these cases, the status string is supposed to be set by the
> subsidiary function, and as far as I know always is. Did you run
> into a
> case where the assertion failure triggered?
--=-ulL+aGuX9TFd/TFjsR05
Content-Type: multipart/mixed; boundary="=-n5knyRQd4XSYvUaspDqW"
--=-n5knyRQd4XSYvUaspDqW
Content-Type: text/x-patch
Content-Disposition: attachment;
filename=0001-Add-missing-KDC-status-on-some-fail-conditions.patch
Content-Transfer-Encoding: quoted-printable
>From 3de5963c39432bff3deb6098fab7fa74a463e479 Mon Sep 17 00:00:00 2001
From: Samuel Cabrero <scabrero@suse.de>
Date: Tue, 11 Jul 2017 18:56:07 +0200
Subject: [PATCH] Add missing KDC "status" on some fail conditions
The process_tgs_req function requires the status string to be set
for all fail conditions before jump to "cleanup" label, otherwise
it will cause an assertion failure.
Signed-off-by: Samuel Cabrero <scabrero@suse.de>
---
src/kdc/do_tgs_req.c | 30 ++++++++++++++++++++++++------
src/kdc/kdc_util.c | 2 ++
2 files changed, 26 insertions(+), 6 deletions(-)
diff --git a/src/kdc/do_tgs_req.c b/src/kdc/do_tgs_req.c
index 547a41441..5d7a74660 100644
--- a/src/kdc/do_tgs_req.c
+++ b/src/kdc/do_tgs_req.c
@@ -253,8 +253,11 @@ process_tgs_req(struct server_handle *handle, krb5_dat=
a *pkt,
=20
errcode =3D search_sprinc(kdc_active_realm, request, s_flags, &server,
&status);
- if (errcode !=3D 0)
+ if (errcode !=3D 0) {
+ if (!status)
+ status =3D "SEARCH_SPRINC";
goto cleanup;
+ }
sprinc =3D server->princ;
=20
/* If we got a cross-realm TGS which is not the requested server, we a=
re
@@ -304,8 +307,11 @@ process_tgs_req(struct server_handle *handle, krb5_dat=
a *pkt,
au_state->s4u2self_user =3D NULL;
}
=20
- if (errcode)
+ if (errcode) {
+ if (!status)
+ status =3D "PROCESS_S4U2SELF";
goto cleanup;
+ }
if (s4u_x509_user !=3D NULL) {
setflag(c_flags, KRB5_KDB_FLAG_PROTOCOL_TRANSITION);
if (is_referral) {
@@ -320,8 +326,11 @@ process_tgs_req(struct server_handle *handle, krb5_dat=
a *pkt,
/* Deal with user-to-user and constrained delegation */
errcode =3D decrypt_2ndtkt(kdc_active_realm, request, c_flags,
&stkt_server, &status);
- if (errcode)
+ if (errcode) {
+ if (!status)
+ status =3D "DECRYPT_2NDTKT";
goto cleanup;
+ }
=20
if (isflagset(request->kdc_options, KDC_OPT_CNAME_IN_ADDL_TKT)) {
/* Do constrained delegation protocol and authorization checks */
@@ -345,8 +354,11 @@ process_tgs_req(struct server_handle *handle, krb5_dat=
a *pkt,
goto cleanup;
}
kau_s4u2proxy(kdc_context, errcode ? FALSE : TRUE, au_state);
- if (errcode)
+ if (errcode) {
+ if (!status)
+ status =3D "KAU_S4U2PROXY";
goto cleanup;
+ }
=20
setflag(c_flags, KRB5_KDB_FLAG_CONSTRAINED_DELEGATION);
=20
@@ -365,8 +377,11 @@ process_tgs_req(struct server_handle *handle, krb5_dat=
a *pkt,
=20
errcode =3D gen_session_key(kdc_active_realm, request, server, &sessio=
n_key,
&status);
- if (errcode)
+ if (errcode) {
+ if (!status)
+ status =3D "GEN_SESSION_KEY";
goto cleanup;
+ }
=20
/*
* subject_tkt will refer to the evidence ticket (for constrained
@@ -745,8 +760,11 @@ process_tgs_req(struct server_handle *handle, krb5_dat=
a *pkt,
au_state->status =3D status;
}
kau_s4u2self(kdc_context, errcode ? FALSE : TRUE, au_state);
- if (errcode)
+ if (errcode) {
+ if (!status)
+ status =3D "KAU_S4U2SELF";
goto cleanup;
+ }
}
=20
reply.client =3D enc_tkt_reply.client;
diff --git a/src/kdc/kdc_util.c b/src/kdc/kdc_util.c
index 29f9dbbf0..43f9eed18 100644
--- a/src/kdc/kdc_util.c
+++ b/src/kdc/kdc_util.c
@@ -1624,6 +1624,7 @@ kdc_process_s4u2proxy_req(kdc_realm_t *kdc_active_rea=
lm,
* that is validated previously in validate_tgs_request().
*/
if (request->kdc_options & (NON_TGT_OPTION | KDC_OPT_ENC_TKT_IN_SKEY))=
{
+ *status =3D "BAD_OPTIONS";
return KRB5KDC_ERR_BADOPTION;
}
=20
@@ -1631,6 +1632,7 @@ kdc_process_s4u2proxy_req(kdc_realm_t *kdc_active_rea=
lm,
if (!krb5_principal_compare(kdc_context,
server->princ, /* after canon */
server_princ)) {
+ *status =3D "SERVER_NOMATCH";
return KRB5KDC_ERR_SERVER_NOMATCH;
}
=20
--=20
2.13.2
=
--=-n5knyRQd4XSYvUaspDqW--
--=-ulL+aGuX9TFd/TFjsR05
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
_______________________________________________
krbdev mailing list krbdev@mit.edu
https://mailman.mit.edu/mailman/listinfo/krbdev
--=-ulL+aGuX9TFd/TFjsR05--
