[Patches] require a PAC within a Kerberos ticket/map to guest = bad uid
Stefan Metzmacher
metze at samba.org
Fri Mar 16 09:39:35 UTC 2018
Hi,
I recently noticed that we have fallback code that tries to build
an auth_session_info from a Kerberos principal if there's no
PAC present in the ticket.
I think think allowing that is completely stupid.
This can only happen if the service has UF_NO_AUTH_DATA_REQUIRED
and we never set this, so we'll always get a PAC.
The attached patches let us require a valid PAC blob
in side Kerberos service tickets.
Please review and push:-)
In source3 we also have code that implements "map to guest = bad uid"
and maps a kerberos authenticated user to guest.
Now that we require a running winbindd on a member server,
we should remove the "bad uid" hacks. Would anyone object
to that? It would simplify a lot and might make it possible
to understand all the strange code paths we have to construct
an auth_session_info.
I guess it is not needed to deprecate it first
as this can only happen if /etc/nsswitch.conf is not configured correctly.
Should I prepare patches to remove this ("bad uid")?
Comments please:-)
Thanks!
metze
-------------- next part --------------
From d2a89017e703bbfa7db48ea58df56c583766dd41 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Sun, 4 Mar 2018 11:55:11 +0100
Subject: [PATCH 1/8] s3:gse: require a PAC in the kerberos ticket.
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
source3/librpc/crypto/gse.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/source3/librpc/crypto/gse.c b/source3/librpc/crypto/gse.c
index 70e29c2..b5d244d 100644
--- a/source3/librpc/crypto/gse.c
+++ b/source3/librpc/crypto/gse.c
@@ -1215,8 +1215,7 @@ static NTSTATUS gensec_gse_session_info(struct gensec_security *gensec_security,
TALLOC_CTX *tmp_ctx;
struct auth_session_info *session_info = NULL;
OM_uint32 maj_stat, min_stat;
- DATA_BLOB pac_blob, *pac_blob_ptr = NULL;
-
+ DATA_BLOB pac_blob;
gss_buffer_desc name_token;
char *principal_string;
@@ -1245,20 +1244,21 @@ static NTSTATUS gensec_gse_session_info(struct gensec_security *gensec_security,
return NT_STATUS_NO_MEMORY;
}
+ /*
+ * We require a PAC in the ticket.
+ */
nt_status = gssapi_obtain_pac_blob(tmp_ctx, gse_ctx->gssapi_context,
gse_ctx->client_name,
&pac_blob);
-
- /* IF we have the PAC - otherwise we need to get this
- * data from elsewere
- */
- if (NT_STATUS_IS_OK(nt_status)) {
- pac_blob_ptr = &pac_blob;
+ if (!NT_STATUS_IS_OK(nt_status)) {
+ talloc_free(tmp_ctx);
+ return nt_status;
}
+
nt_status = gensec_generate_session_info_pac(tmp_ctx,
gensec_security,
NULL,
- pac_blob_ptr, principal_string,
+ &pac_blob, principal_string,
gensec_get_remote_address(gensec_security),
&session_info);
if (!NT_STATUS_IS_OK(nt_status)) {
--
1.9.1
From ea64a5082dc5e6cc9cf547647ea8efc60623edd7 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Sun, 4 Mar 2018 11:55:11 +0100
Subject: [PATCH 2/8] s4:gensec_gssapi: require a PAC in the kerberos ticket.
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
source4/auth/gensec/gensec_gssapi.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/source4/auth/gensec/gensec_gssapi.c b/source4/auth/gensec/gensec_gssapi.c
index a61b2b2..35189db 100644
--- a/source4/auth/gensec/gensec_gssapi.c
+++ b/source4/auth/gensec/gensec_gssapi.c
@@ -1466,8 +1466,7 @@ static NTSTATUS gensec_gssapi_session_info(struct gensec_security *gensec_securi
= talloc_get_type(gensec_security->private_data, struct gensec_gssapi_state);
struct auth_session_info *session_info = NULL;
OM_uint32 maj_stat, min_stat;
- DATA_BLOB pac_blob, *pac_blob_ptr = NULL;
-
+ DATA_BLOB pac_blob;
gss_buffer_desc name_token;
char *principal_string;
@@ -1496,21 +1495,21 @@ static NTSTATUS gensec_gssapi_session_info(struct gensec_security *gensec_securi
return NT_STATUS_NO_MEMORY;
}
+ /*
+ * We require a PAC in the ticket.
+ */
nt_status = gssapi_obtain_pac_blob(tmp_ctx, gensec_gssapi_state->gssapi_context,
gensec_gssapi_state->client_name,
&pac_blob);
-
- /* IF we have the PAC - otherwise we need to get this
- * data from elsewere - local ldb, or (TODO) lookup of some
- * kind...
- */
- if (NT_STATUS_IS_OK(nt_status)) {
- pac_blob_ptr = &pac_blob;
+ if (!NT_STATUS_IS_OK(nt_status)) {
+ talloc_free(tmp_ctx);
+ return nt_status;
}
+
nt_status = gensec_generate_session_info_pac(tmp_ctx,
gensec_security,
gensec_gssapi_state->smb_krb5_context,
- pac_blob_ptr, principal_string,
+ &pac_blob, principal_string,
gensec_get_remote_address(gensec_security),
&session_info);
if (!NT_STATUS_IS_OK(nt_status)) {
--
1.9.1
From 8f520b566f3e70674e681d319924a082361fad3a Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Sun, 4 Mar 2018 11:55:11 +0100
Subject: [PATCH 3/8] auth:gensec: require a PAC in the kerberos ticket in
gensec_generate_session_info_pac()
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
auth/gensec/gensec_util.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/auth/gensec/gensec_util.c b/auth/gensec/gensec_util.c
index 20c9c2a..74a3e7c 100644
--- a/auth/gensec/gensec_util.c
+++ b/auth/gensec/gensec_util.c
@@ -45,14 +45,10 @@ NTSTATUS gensec_generate_session_info_pac(TALLOC_CTX *mem_ctx,
session_info_flags |= AUTH_SESSION_INFO_DEFAULT_GROUPS;
- if (!pac_blob) {
- if (gensec_setting_bool(gensec_security->settings, "gensec", "require_pac", false)) {
- DEBUG(1, ("Unable to find PAC in ticket from %s, failing to allow access\n",
- principal_string));
- return NT_STATUS_ACCESS_DENIED;
- }
- DBG_NOTICE("Unable to find PAC for %s, resorting to local "
- "user lookup\n", principal_string);
+ if (pac_blob == NULL) {
+ DBG_WARNING("Unable to find PAC in ticket from %s, "
+ "failing to allow access\n", principal_string);
+ return NT_STATUS_ACCESS_DENIED;
}
if (gensec_security->auth_context && gensec_security->auth_context->generate_session_info_pac) {
--
1.9.1
From fa80293e075cb1a230bcdd23120bc7c8cdf97662 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Sun, 4 Mar 2018 11:55:11 +0100
Subject: [PATCH 4/8] s4:auth: require a PAC in the kerberos ticket in
auth_generate_session_info_pac()
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
source4/auth/ntlm/auth.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/source4/auth/ntlm/auth.c b/source4/auth/ntlm/auth.c
index 7e10a55..5133472 100644
--- a/source4/auth/ntlm/auth.c
+++ b/source4/auth/ntlm/auth.c
@@ -89,6 +89,7 @@ PAC isn't available, and for tokenGroups in the DSDB stack.
Supply either a principal or a DN
****************************************************************************/
+#if 0
static NTSTATUS auth_generate_session_info_principal(struct auth4_context *auth_ctx,
TALLOC_CTX *mem_ctx,
const char *principal,
@@ -124,6 +125,7 @@ static NTSTATUS auth_generate_session_info_principal(struct auth4_context *auth_
return NT_STATUS_NOT_IMPLEMENTED;
}
+#endif
/**
* Check a user's Plaintext, LM or NTLM password.
@@ -652,9 +654,11 @@ static NTSTATUS auth_generate_session_info_pac(struct auth4_context *auth_ctx,
struct auth_user_info_dc *user_info_dc;
TALLOC_CTX *tmp_ctx;
- if (!pac_blob) {
- return auth_generate_session_info_principal(auth_ctx, mem_ctx, principal_name,
- NULL, session_info_flags, session_info);
+ if (pac_blob == NULL) {
+ /*
+ * The caller should have already checked this!
+ */
+ return NT_STATUS_INTERNAL_ERROR;
}
tmp_ctx = talloc_named(mem_ctx, 0, "gensec_gssapi_session_info context");
--
1.9.1
From 28d825f0deac4c730818304330983ab7891dcde0 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Wed, 7 Mar 2018 10:08:05 +0100
Subject: [PATCH 5/8] s4:auth: remove unused
auth_generate_session_info_principal()
We still make authsam_get_user_info_dc_principal(), but it's
not required to tunnel this through the auth methods.
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
source4/auth/auth.h | 6 ------
source4/auth/ntlm/auth.c | 44 --------------------------------------------
source4/auth/ntlm/auth_sam.c | 12 ------------
3 files changed, 62 deletions(-)
diff --git a/source4/auth/auth.h b/source4/auth/auth.h
index f88489b..7880d92 100644
--- a/source4/auth/auth.h
+++ b/source4/auth/auth.h
@@ -74,12 +74,6 @@ struct auth_operations {
struct auth_user_info_dc **interim_info,
bool *authoritative);
- /* Lookup a 'session info interim' return based only on the principal or DN */
- NTSTATUS (*get_user_info_dc_principal)(TALLOC_CTX *mem_ctx,
- struct auth4_context *auth_context,
- const char *principal,
- struct ldb_dn *user_dn,
- struct auth_user_info_dc **interim_info);
uint32_t flags;
};
diff --git a/source4/auth/ntlm/auth.c b/source4/auth/ntlm/auth.c
index 5133472..560f8cd 100644
--- a/source4/auth/ntlm/auth.c
+++ b/source4/auth/ntlm/auth.c
@@ -83,50 +83,6 @@ _PUBLIC_ NTSTATUS auth_get_challenge(struct auth4_context *auth_ctx, uint8_t cha
return NT_STATUS_OK;
}
-/****************************************************************************
-Used in the gensec_gssapi and gensec_krb5 server-side code, where the
-PAC isn't available, and for tokenGroups in the DSDB stack.
-
- Supply either a principal or a DN
-****************************************************************************/
-#if 0
-static NTSTATUS auth_generate_session_info_principal(struct auth4_context *auth_ctx,
- TALLOC_CTX *mem_ctx,
- const char *principal,
- struct ldb_dn *user_dn,
- uint32_t session_info_flags,
- struct auth_session_info **session_info)
-{
- NTSTATUS nt_status;
- struct auth_method_context *method;
- struct auth_user_info_dc *user_info_dc;
-
- for (method = auth_ctx->methods; method; method = method->next) {
- if (!method->ops->get_user_info_dc_principal) {
- continue;
- }
-
- nt_status = method->ops->get_user_info_dc_principal(mem_ctx, auth_ctx, principal, user_dn, &user_info_dc);
- if (NT_STATUS_EQUAL(nt_status, NT_STATUS_NOT_IMPLEMENTED)) {
- continue;
- }
- if (!NT_STATUS_IS_OK(nt_status)) {
- return nt_status;
- }
-
- nt_status = auth_generate_session_info_wrapper(auth_ctx, mem_ctx,
- user_info_dc,
- user_info_dc->info->account_name,
- session_info_flags, session_info);
- talloc_free(user_info_dc);
-
- return nt_status;
- }
-
- return NT_STATUS_NOT_IMPLEMENTED;
-}
-#endif
-
/**
* Check a user's Plaintext, LM or NTLM password.
* (sync version)
diff --git a/source4/auth/ntlm/auth_sam.c b/source4/auth/ntlm/auth_sam.c
index d63a7d1..f44c4d0 100644
--- a/source4/auth/ntlm/auth_sam.c
+++ b/source4/auth/ntlm/auth_sam.c
@@ -851,28 +851,16 @@ static NTSTATUS authsam_want_check(struct auth_method_context *ctx,
return NT_STATUS_OK;
}
-/* Wrapper for the auth subsystem pointer */
-static NTSTATUS authsam_get_user_info_dc_principal_wrapper(TALLOC_CTX *mem_ctx,
- struct auth4_context *auth_context,
- const char *principal,
- struct ldb_dn *user_dn,
- struct auth_user_info_dc **user_info_dc)
-{
- return authsam_get_user_info_dc_principal(mem_ctx, auth_context->lp_ctx, auth_context->sam_ctx,
- principal, user_dn, user_info_dc);
-}
static const struct auth_operations sam_ignoredomain_ops = {
.name = "sam_ignoredomain",
.want_check = authsam_ignoredomain_want_check,
.check_password = authsam_check_password_internals,
- .get_user_info_dc_principal = authsam_get_user_info_dc_principal_wrapper,
};
static const struct auth_operations sam_ops = {
.name = "sam",
.want_check = authsam_want_check,
.check_password = authsam_check_password_internals,
- .get_user_info_dc_principal = authsam_get_user_info_dc_principal_wrapper,
};
_PUBLIC_ NTSTATUS auth4_sam_init(TALLOC_CTX *);
--
1.9.1
From 30af5e140e9bb849aefae9886e1a6553c8f75903 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Sun, 4 Mar 2018 11:55:11 +0100
Subject: [PATCH 6/8] s3:auth: require a PAC in the kerberos ticket in
auth3_generate_session_info_pac()
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
source3/auth/auth_generic.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/source3/auth/auth_generic.c b/source3/auth/auth_generic.c
index 167d4e0..971fc5f 100644
--- a/source3/auth/auth_generic.c
+++ b/source3/auth/auth_generic.c
@@ -59,6 +59,13 @@ static NTSTATUS auth3_generate_session_info_pac(struct auth4_context *auth_ctx,
NTSTATUS status;
int rc;
+ if (pac_blob == NULL) {
+ /*
+ * The caller should have already checked this!
+ */
+ return NT_STATUS_INTERNAL_ERROR;
+ }
+
tmp_ctx = talloc_new(mem_ctx);
if (!tmp_ctx) {
return NT_STATUS_NO_MEMORY;
--
1.9.1
From 1ebb144faf19513490654ced981fcfe755d47b2c Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Sun, 4 Mar 2018 11:55:11 +0100
Subject: [PATCH 7/8] s3:auth: simplify the #ifdef HAVE_KRB5 logic in
auth3_generate_session_info_pac()
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
source3/auth/auth_generic.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/source3/auth/auth_generic.c b/source3/auth/auth_generic.c
index 971fc5f..df852e6 100644
--- a/source3/auth/auth_generic.c
+++ b/source3/auth/auth_generic.c
@@ -46,6 +46,7 @@ static NTSTATUS auth3_generate_session_info_pac(struct auth4_context *auth_ctx,
uint32_t session_info_flags,
struct auth_session_info **session_info)
{
+#ifdef HAVE_KRB5
TALLOC_CTX *tmp_ctx;
struct PAC_LOGON_INFO *logon_info = NULL;
struct netr_SamInfo3 *info3_copy = NULL;
@@ -72,7 +73,6 @@ static NTSTATUS auth3_generate_session_info_pac(struct auth4_context *auth_ctx,
}
if (pac_blob) {
-#ifdef HAVE_KRB5
struct wbcAuthUserParams params = {};
struct wbcAuthUserInfo *info = NULL;
struct wbcAuthErrorInfo *err = NULL;
@@ -120,9 +120,6 @@ static NTSTATUS auth3_generate_session_info_pac(struct auth4_context *auth_ctx,
status = kerberos_pac_logon_info(tmp_ctx, *pac_blob, NULL, NULL,
NULL, NULL, 0, &logon_info);
-#else
- status = NT_STATUS_ACCESS_DENIED;
-#endif
if (!NT_STATUS_IS_OK(status)) {
goto done;
}
@@ -191,6 +188,13 @@ static NTSTATUS auth3_generate_session_info_pac(struct auth4_context *auth_ctx,
done:
TALLOC_FREE(tmp_ctx);
return status;
+#else
+ /*
+ * This should never be called if
+ * we don't support kerberos.
+ */
+ return NT_STATUS_INTERNAL_ERROR;
+#endif
}
static struct auth4_context *make_auth4_context_s3(TALLOC_CTX *mem_ctx, struct auth_context *auth_context)
--
1.9.1
From 83e856945edec7ed7184292fa771db854ef5a68b Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Sun, 4 Mar 2018 11:55:11 +0100
Subject: [PATCH 8/8] s3:auth: remove unused indentation level in
auth3_generate_session_info_pac()
We always require a pac_blob now.
Check with git show -w
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
source3/auth/auth_generic.c | 91 ++++++++++++++++++++++-----------------------
1 file changed, 44 insertions(+), 47 deletions(-)
diff --git a/source3/auth/auth_generic.c b/source3/auth/auth_generic.c
index df852e6..0081e11 100644
--- a/source3/auth/auth_generic.c
+++ b/source3/auth/auth_generic.c
@@ -59,6 +59,10 @@ static NTSTATUS auth3_generate_session_info_pac(struct auth4_context *auth_ctx,
struct passwd *pw;
NTSTATUS status;
int rc;
+ struct wbcAuthUserParams params = {};
+ struct wbcAuthUserInfo *info = NULL;
+ struct wbcAuthErrorInfo *err = NULL;
+ wbcErr wbc_err;
if (pac_blob == NULL) {
/*
@@ -72,57 +76,50 @@ static NTSTATUS auth3_generate_session_info_pac(struct auth4_context *auth_ctx,
return NT_STATUS_NO_MEMORY;
}
- if (pac_blob) {
- struct wbcAuthUserParams params = {};
- struct wbcAuthUserInfo *info = NULL;
- struct wbcAuthErrorInfo *err = NULL;
- wbcErr wbc_err;
-
- /*
- * Let winbind decode the PAC.
- * This will also store the user
- * data in the netsamlogon cache.
- *
- * We need to do this *before* we
- * call get_user_from_kerberos_info()
- * as that does a user lookup that
- * expects info in the netsamlogon cache.
- *
- * See BUG: https://bugzilla.samba.org/show_bug.cgi?id=11259
- */
- params.level = WBC_AUTH_USER_LEVEL_PAC;
- params.password.pac.data = pac_blob->data;
- params.password.pac.length = pac_blob->length;
-
- become_root();
- wbc_err = wbcAuthenticateUserEx(¶ms, &info, &err);
- unbecome_root();
+ /*
+ * Let winbind decode the PAC.
+ * This will also store the user
+ * data in the netsamlogon cache.
+ *
+ * We need to do this *before* we
+ * call get_user_from_kerberos_info()
+ * as that does a user lookup that
+ * expects info in the netsamlogon cache.
+ *
+ * See BUG: https://bugzilla.samba.org/show_bug.cgi?id=11259
+ */
+ params.level = WBC_AUTH_USER_LEVEL_PAC;
+ params.password.pac.data = pac_blob->data;
+ params.password.pac.length = pac_blob->length;
- /*
- * As this is merely a cache prime
- * WBC_ERR_WINBIND_NOT_AVAILABLE
- * is not a fatal error, treat it
- * as success.
- */
+ become_root();
+ wbc_err = wbcAuthenticateUserEx(¶ms, &info, &err);
+ unbecome_root();
- switch (wbc_err) {
- case WBC_ERR_WINBIND_NOT_AVAILABLE:
- case WBC_ERR_SUCCESS:
- break;
- case WBC_ERR_AUTH_ERROR:
- status = NT_STATUS(err->nt_status);
- wbcFreeMemory(err);
- goto done;
- default:
- status = NT_STATUS_LOGON_FAILURE;
- goto done;
- }
+ /*
+ * As this is merely a cache prime
+ * WBC_ERR_WINBIND_NOT_AVAILABLE
+ * is not a fatal error, treat it
+ * as success.
+ */
- status = kerberos_pac_logon_info(tmp_ctx, *pac_blob, NULL, NULL,
- NULL, NULL, 0, &logon_info);
- if (!NT_STATUS_IS_OK(status)) {
+ switch (wbc_err) {
+ case WBC_ERR_WINBIND_NOT_AVAILABLE:
+ case WBC_ERR_SUCCESS:
+ break;
+ case WBC_ERR_AUTH_ERROR:
+ status = NT_STATUS(err->nt_status);
+ wbcFreeMemory(err);
goto done;
- }
+ default:
+ status = NT_STATUS_LOGON_FAILURE;
+ goto done;
+ }
+
+ status = kerberos_pac_logon_info(tmp_ctx, *pac_blob, NULL, NULL,
+ NULL, NULL, 0, &logon_info);
+ if (!NT_STATUS_IS_OK(status)) {
+ goto done;
}
rc = get_remote_hostname(remote_address,
--
1.9.1
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20180316/c01cf5c6/signature.sig>
More information about the samba-technical
mailing list