From c316802300e1480c92e646608970cba502644591 Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 6 May 2014 16:48:07 -0700 Subject: [PATCH 1/5] s3-krb5: Limit search for old kvno to 8bits BUG: https://bugzilla.samba.org/show_bug.cgi?id=11418 Some keytab files store the kvno only in 8bits. Limit the compare to 8bits, so that we don't miss old keys and delete them. This fixes the problem that updates to the keytab file removed all previous keys. Signed-off-by: Christof Schmitt Reviewed-by: Simo Sorce Autobuild-User(master): Christof Schmitt Autobuild-Date(master): Thu May 8 00:54:15 CEST 2014 on sn-devel-104 (cherry picked from commit a5b96ee5fb97528767fc63aa8e70a314686ee38a) --- source3/libads/kerberos_keytab.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/source3/libads/kerberos_keytab.c b/source3/libads/kerberos_keytab.c index 56f0a77..ae3d80e39 100644 --- a/source3/libads/kerberos_keytab.c +++ b/source3/libads/kerberos_keytab.c @@ -49,6 +49,7 @@ static krb5_error_code seek_and_delete_old_entries(krb5_context context, krb5_keytab_entry kt_entry; krb5_keytab_entry zero_kt_entry; char *ktprinc = NULL; + krb5_kvno old_kvno = kvno - 1; ZERO_STRUCT(cursor); ZERO_STRUCT(zero_csr); @@ -115,12 +116,14 @@ static krb5_error_code seek_and_delete_old_entries(krb5_context context, * changes, all kerberizied sessions will 'break' until either * the client reboots or the client's session key expires and * they get a new session ticket with the new kvno. + * Some keytab files only store the kvno in 8bits, limit + * the compare accordingly. */ - if (!flush && (kt_entry.vno == kvno - 1)) { + if (!flush && ((kt_entry.vno & 0xff) == (old_kvno & 0xff))) { DEBUG(5, (__location__ ": Saving previous (kvno %d) " "entry for principal: %s.\n", - kvno - 1, princ_s)); + old_kvno, princ_s)); continue; } -- 2.4.6 From 84435756b84da0f0cc687c8bbbb11517d9189045 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Wed, 4 Mar 2015 10:09:18 +0100 Subject: [PATCH 2/5] libads: Fix CID 1273306 Uninitialized scalar variable BUG: https://bugzilla.samba.org/show_bug.cgi?id=11418 Signed-off-by: Volker Lendecke Reviewed-by: David Disseldorp (cherry picked from commit 4a686c5b0bbcf0bdb089348403a3c35b8aff67e4) --- source3/libads/kerberos_keytab.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source3/libads/kerberos_keytab.c b/source3/libads/kerberos_keytab.c index ae3d80e39..2d5c7ff 100644 --- a/source3/libads/kerberos_keytab.c +++ b/source3/libads/kerberos_keytab.c @@ -508,7 +508,7 @@ int ads_keytab_create_default(ADS_STRUCT *ads) krb5_context context = NULL; krb5_keytab keytab = NULL; krb5_kt_cursor cursor; - krb5_keytab_entry kt_entry; + krb5_keytab_entry kt_entry = {0}; krb5_kvno kvno; size_t found = 0; char *sam_account_name, *upn; -- 2.4.6 From 6e15687727bde972c58e8c3eabeef9cc677026fc Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Wed, 4 Mar 2015 10:09:51 +0100 Subject: [PATCH 3/5] libads: Fix CID 1273305 Uninitialized scalar variable BUG: https://bugzilla.samba.org/show_bug.cgi?id=11418 Signed-off-by: Volker Lendecke Reviewed-by: David Disseldorp (cherry picked from commit 706770d7a8c4625ecb555db40c146126d2c160f0) --- source3/libads/kerberos_keytab.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source3/libads/kerberos_keytab.c b/source3/libads/kerberos_keytab.c index 2d5c7ff..bbd981c 100644 --- a/source3/libads/kerberos_keytab.c +++ b/source3/libads/kerberos_keytab.c @@ -507,7 +507,7 @@ int ads_keytab_create_default(ADS_STRUCT *ads) krb5_error_code ret = 0; krb5_context context = NULL; krb5_keytab keytab = NULL; - krb5_kt_cursor cursor; + krb5_kt_cursor cursor = {0}; krb5_keytab_entry kt_entry = {0}; krb5_kvno kvno; size_t found = 0; -- 2.4.6 From f35ae8d8f285c70dcd52771c76e6a67ab9bc7168 Mon Sep 17 00:00:00 2001 From: Uri Simchoni Date: Sat, 2 May 2015 13:44:52 +0300 Subject: [PATCH 4/5] libads: Fix free of uninitialized pointer BUG: https://bugzilla.samba.org/show_bug.cgi?id=11418 In ads_keytab_creat_default(), if the keytab to be created cannot be opened, the bail-out code calls smb_krb5_kt_free_entry() on an uninitialized entry. To reproduce: 1. Join a domain 2. KRB5_KTNAME=FILE:/non-existant-path/krb5.keytab net ads keytab create -P Signed-off-by: Uri Simchoni Reviewed-by: Jeremy Allison Reviewed-by: Andreas Schneider (cherry picked from commit df91bc5159b24f6f10fd9742b49192921d51f821) --- source3/libads/kerberos_keytab.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/source3/libads/kerberos_keytab.c b/source3/libads/kerberos_keytab.c index bbd981c..ef6374a 100644 --- a/source3/libads/kerberos_keytab.c +++ b/source3/libads/kerberos_keytab.c @@ -520,6 +520,9 @@ int ads_keytab_create_default(ADS_STRUCT *ads) size_t i; ADS_STATUS status; + ZERO_STRUCT(kt_entry); + ZERO_STRUCT(cursor); + frame = talloc_stackframe(); if (frame == NULL) { ret = -1; @@ -575,8 +578,6 @@ int ads_keytab_create_default(ADS_STRUCT *ads) #endif memset(princ_s, '\0', sizeof(princ_s)); - ZERO_STRUCT(kt_entry); - ZERO_STRUCT(cursor); initialize_krb5_error_table(); ret = krb5_init_context(&context); -- 2.4.6 From 8eceef2f8026f17fc2a5ad79ceea71d63e14fc1e Mon Sep 17 00:00:00 2001 From: Uri Simchoni Date: Sat, 2 May 2015 13:44:53 +0300 Subject: [PATCH 5/5] libads: Fix deadlock when re-joining a domain and updating keytab BUG: https://bugzilla.samba.org/show_bug.cgi?id=11418 When updating the system keytab as a result of joining a domain, if the keytb had prior entries, ads_keytab_create_default tries to update those entries. However, it starts updating before freeing the cursor which was used for finding those entries, and hence causes an an attempt to write-lock the keytab while a read-lock exists. To reproduce configure smb.conf for ads domain member and run this twice: net ads join -U '--option=kerberos method=secrets and keytab' Signed-off-by: Uri Simchoni Reviewed-by: Jeremy Allison Reviewed-by: Andreas Schneider Autobuild-User(master): Jeremy Allison Autobuild-Date(master): Mon May 4 21:01:41 CEST 2015 on sn-devel-104 (cherry picked from commit 38beef2ff63664d7d5805f1032bb9f69d0b965d7) --- source3/libads/kerberos_keytab.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/source3/libads/kerberos_keytab.c b/source3/libads/kerberos_keytab.c index ef6374a..309e614 100644 --- a/source3/libads/kerberos_keytab.c +++ b/source3/libads/kerberos_keytab.c @@ -731,13 +731,14 @@ int ads_keytab_create_default(ADS_STRUCT *ads) smb_krb5_kt_free_entry(context, &kt_entry); ZERO_STRUCT(kt_entry); } + krb5_kt_end_seq_get(context, keytab, &cursor); + ZERO_STRUCT(cursor); + ret = 0; for (i = 0; oldEntries[i]; i++) { ret |= ads_keytab_add_entry(ads, oldEntries[i]); TALLOC_FREE(oldEntries[i]); } - krb5_kt_end_seq_get(context, keytab, &cursor); - ZERO_STRUCT(cursor); done: TALLOC_FREE(oldEntries); -- 2.4.6