The Samba-Bugzilla – Attachment 18694 Details for
Bug 15893
Improve handling of principals and realms in client tools
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
backport to v.4.23
mr-4139-v4.23.patch (text/plain), 16.57 KB, created by
Alexander Bokovoy
on 2025-08-25 13:31:08 UTC
(
hide
)
Description:
backport to v.4.23
Filename:
MIME Type:
Creator:
Alexander Bokovoy
Created:
2025-08-25 13:31:08 UTC
Size:
16.57 KB
patch
obsolete
>From 1ccdbde2bde8454e27cb2b9a6d0dda140379b22a Mon Sep 17 00:00:00 2001 >From: Andreas Schneider <asn@samba.org> >Date: Wed, 6 Aug 2025 14:40:34 +0200 >Subject: [PATCH 1/9] auth:creds: Allow to reset the realm by passing NULL > >This is e.g. done by cli_credentials_set_anonymous(). > >We can't call TALLOC_FREE(cred->realm), as this would break >cli_credentials_shallow_copy(). > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15893 > >Signed-off-by: Andreas Schneider <asn@samba.org> >Reviewed-by: Alexander Bokovoy <ab@samba.org> >(cherry picked from commit e5608cdb2e5a7ef2641ec0e7b0ce0b4640a02ce1) >--- > auth/credentials/credentials.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > >diff --git a/auth/credentials/credentials.c b/auth/credentials/credentials.c >index c31470a81d2..5588a355c74 100644 >--- a/auth/credentials/credentials.c >+++ b/auth/credentials/credentials.c >@@ -933,7 +933,14 @@ _PUBLIC_ bool cli_credentials_set_realm(struct cli_credentials *cred, > enum credentials_obtained obtained) > { > if (obtained >= cred->realm_obtained) { >- cred->realm = strupper_talloc(cred, val); >+ /* If `val = NULL` is passed, realm is reset */ >+ cred->realm = NULL; >+ if (val != NULL) { >+ cred->realm = strupper_talloc(cred, val); >+ if (cred->realm == NULL) { >+ return false; >+ } >+ } > cred->realm_obtained = obtained; > cli_credentials_invalidate_ccache(cred, cred->realm_obtained); > return true; >-- >2.50.1 > > >From 224e7d09e98b1a4762c9297d2c44c0ab39af2886 Mon Sep 17 00:00:00 2001 >From: Andreas Schneider <asn@samba.org> >Date: Thu, 7 Aug 2025 13:32:47 +0200 >Subject: [PATCH 2/9] auth:creds: Also uppercase realm set via a callback > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15893 > >Signed-off-by: Andreas Schneider <asn@samba.org> >Reviewed-by: Alexander Bokovoy <ab@samba.org> >(cherry picked from commit 4f8ff3a567d6318c71b0960345592224721c9594) >--- > auth/credentials/credentials.c | 13 ++++++++++++- > 1 file changed, 12 insertions(+), 1 deletion(-) > >diff --git a/auth/credentials/credentials.c b/auth/credentials/credentials.c >index 5588a355c74..a558aada67c 100644 >--- a/auth/credentials/credentials.c >+++ b/auth/credentials/credentials.c >@@ -912,9 +912,20 @@ _PUBLIC_ const char *cli_credentials_get_realm(struct cli_credentials *cred) > > if (cred->realm_obtained == CRED_CALLBACK && > !cred->callback_running) { >+ const char *realm = NULL; >+ > cred->callback_running = true; >- cred->realm = cred->realm_cb(cred); >+ realm = cred->realm_cb(cred); > cred->callback_running = false; >+ >+ cred->realm = NULL; >+ if (realm != NULL) { >+ cred->realm = strupper_talloc(cred, realm); >+ if (cred->realm == NULL) { >+ return NULL; >+ } >+ } >+ > if (cred->realm_obtained == CRED_CALLBACK) { > cred->realm_obtained = CRED_CALLBACK_RESULT; > cli_credentials_invalidate_ccache(cred, cred->realm_obtained); >-- >2.50.1 > > >From ee5c993896cc53eb150518f6ffe7ce6f70a067fb Mon Sep 17 00:00:00 2001 >From: Andreas Schneider <asn@samba.org> >Date: Wed, 6 Aug 2025 14:42:51 +0200 >Subject: [PATCH 3/9] auth:creds: Allow to reset the principal by passing NULL > to set_principal > >We do that e.g. in cli_credentials_set_anonymous() > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15893 > >Signed-off-by: Andreas Schneider <asn@samba.org> >Reviewed-by: Alexander Bokovoy <ab@samba.org> >(cherry picked from commit 67c2feba290764c62ab01602d5bc9d4d122c2c12) >--- > auth/credentials/credentials.c | 15 ++++++++++----- > 1 file changed, 10 insertions(+), 5 deletions(-) > >diff --git a/auth/credentials/credentials.c b/auth/credentials/credentials.c >index a558aada67c..1992b1c6a74 100644 >--- a/auth/credentials/credentials.c >+++ b/auth/credentials/credentials.c >@@ -432,12 +432,15 @@ _PUBLIC_ bool cli_credentials_set_principal(struct cli_credentials *cred, > enum credentials_obtained obtained) > { > if (obtained >= cred->principal_obtained) { >- cred->principal = talloc_strdup(cred, val); >- if (cred->principal == NULL) { >- return false; >+ /* If `val = NULL` is passed, principal is reset */ >+ cred->principal = NULL; >+ if (val != NULL) { >+ cred->principal = talloc_strdup(cred, val); >+ if (cred->principal == NULL) { >+ return false; >+ } > } > cred->principal_obtained = obtained; >- > cli_credentials_invalidate_ccache(cred, cred->principal_obtained); > return true; > } >@@ -1553,7 +1556,9 @@ _PUBLIC_ void cli_credentials_get_ntlm_username_domain(struct cli_credentials *c > const char **username, > const char **domain) > { >- if (cred->principal_obtained >= cred->username_obtained) { >+ if (!cli_credentials_is_anonymous(cred) && >+ cred->principal_obtained >= cred->username_obtained) >+ { > *domain = talloc_strdup(mem_ctx, ""); > *username = cli_credentials_get_principal(cred, mem_ctx); > } else { >-- >2.50.1 > > >From 827fce02b9307e0c1e6fcdeba62cd0c3c0269123 Mon Sep 17 00:00:00 2001 >From: Andreas Schneider <asn@samba.org> >Date: Thu, 7 Aug 2025 13:45:48 +0200 >Subject: [PATCH 4/9] auth:creds: Keep the password secret > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15893 > >Signed-off-by: Andreas Schneider <asn@samba.org> >Reviewed-by: Alexander Bokovoy <ab@samba.org> >(cherry picked from commit 705db6c8b295f65f40b7dcd0d5dc0f6db901c8d7) >--- > auth/credentials/credentials.c | 1 + > 1 file changed, 1 insertion(+) > >diff --git a/auth/credentials/credentials.c b/auth/credentials/credentials.c >index 1992b1c6a74..1a64a2d8cdc 100644 >--- a/auth/credentials/credentials.c >+++ b/auth/credentials/credentials.c >@@ -626,6 +626,7 @@ _PUBLIC_ bool cli_credentials_set_password(struct cli_credentials *cred, > if (cred->password == NULL) { > return false; > } >+ talloc_keep_secret(discard_const(cred->password)); > > /* Don't print the actual password in talloc memory dumps */ > talloc_set_name_const(cred->password, >-- >2.50.1 > > >From e996ec967805d0641bf8074c5ff6dec244fe716a Mon Sep 17 00:00:00 2001 >From: Andreas Schneider <asn@samba.org> >Date: Thu, 7 Aug 2025 13:48:04 +0200 >Subject: [PATCH 5/9] auth:creds: Keep password secret in > cmdline_get_userpassword() > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15893 > >Signed-off-by: Andreas Schneider <asn@samba.org> >Reviewed-by: Alexander Bokovoy <ab@samba.org> >(cherry picked from commit 34482f4ad014a09c84b484097a8d03dfec4f6512) >--- > auth/credentials/credentials_cmdline.c | 1 + > 1 file changed, 1 insertion(+) > >diff --git a/auth/credentials/credentials_cmdline.c b/auth/credentials/credentials_cmdline.c >index c8c7c183c22..e9cdc80d52a 100644 >--- a/auth/credentials/credentials_cmdline.c >+++ b/auth/credentials/credentials_cmdline.c >@@ -46,6 +46,7 @@ static const char *cmdline_get_userpassword(struct cli_credentials *creds) > goto fail; > } > talloc_set_name_const(ret, __location__); >+ talloc_keep_secret(ret); > fail: > ZERO_STRUCT(pwd); > TALLOC_FREE(frame); >-- >2.50.1 > > >From f1c882b8e4748bcfeb2431c5ca03bbf916e25ebc Mon Sep 17 00:00:00 2001 >From: Andreas Schneider <asn@samba.org> >Date: Thu, 7 Aug 2025 13:48:26 +0200 >Subject: [PATCH 6/9] s3:utils: Keep password secret in ntlm_auth > get_password() > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15893 > >Signed-off-by: Andreas Schneider <asn@samba.org> >Reviewed-by: Alexander Bokovoy <ab@samba.org> >(cherry picked from commit f86739e3abd63ba0b7ba632d796968fec9fa2f8f) >--- > source3/utils/ntlm_auth.c | 1 + > 1 file changed, 1 insertion(+) > >diff --git a/source3/utils/ntlm_auth.c b/source3/utils/ntlm_auth.c >index d5ae7c85b22..a424990baa8 100644 >--- a/source3/utils/ntlm_auth.c >+++ b/source3/utils/ntlm_auth.c >@@ -254,6 +254,7 @@ static const char *get_password(struct cli_credentials *credentials) > > manage_squid_request(NUM_HELPER_MODES /* bogus */, NULL, state, manage_gensec_get_pw_request, (void **)&password); > talloc_steal(credentials, password); >+ talloc_keep_secret(password); > TALLOC_FREE(frame); > return password; > } >-- >2.50.1 > > >From f73a3bd67942b085bec34a8b696897ed929b9237 Mon Sep 17 00:00:00 2001 >From: Andreas Schneider <asn@samba.org> >Date: Tue, 5 Aug 2025 15:25:54 +0200 >Subject: [PATCH 7/9] auth:creds: Validate realm names in set_realm and > set_principal > >See also >https://web.mit.edu/kerberos/krb5-latest/doc/admin/realm_config.html#realm-name > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15893 > >Signed-off-by: Andreas Schneider <asn@samba.org> >Reviewed-by: Alexander Bokovoy <ab@samba.org> >(cherry picked from commit e848671f34f969634d55eb7b846d70e6334034ae) >--- > auth/credentials/credentials.c | 25 +++++++++++++++++++++++++ > 1 file changed, 25 insertions(+) > >diff --git a/auth/credentials/credentials.c b/auth/credentials/credentials.c >index 1a64a2d8cdc..777bf53430d 100644 >--- a/auth/credentials/credentials.c >+++ b/auth/credentials/credentials.c >@@ -33,6 +33,18 @@ > #include "system/filesys.h" > #include "system/passwd.h" > >+static bool str_is_ascii(const char *s) { >+ if (s != NULL) { >+ for (; s[0] != '\0'; s++) { >+ if (!isascii(s[0])) { >+ return false; >+ } >+ } >+ } >+ >+ return true; >+} >+ > /** > * Create a new credentials structure > * @param mem_ctx TALLOC_CTX parent for credentials structure >@@ -435,6 +447,14 @@ _PUBLIC_ bool cli_credentials_set_principal(struct cli_credentials *cred, > /* If `val = NULL` is passed, principal is reset */ > cred->principal = NULL; > if (val != NULL) { >+ char *p = strchr(val, '@'); >+ if (p != NULL) { >+ /* For realm names, only ASCII is allowed */ >+ if (!str_is_ascii(p + 1)) { >+ return false; >+ } >+ } >+ > cred->principal = talloc_strdup(cred, val); > if (cred->principal == NULL) { > return false; >@@ -951,6 +971,11 @@ _PUBLIC_ bool cli_credentials_set_realm(struct cli_credentials *cred, > /* If `val = NULL` is passed, realm is reset */ > cred->realm = NULL; > if (val != NULL) { >+ /* For realm names, only ASCII is allowed */ >+ if (!str_is_ascii(val)) { >+ return false; >+ } >+ > cred->realm = strupper_talloc(cred, val); > if (cred->realm == NULL) { > return false; >-- >2.50.1 > > >From d46cccd48e5674454462ef6f637f3944f2538374 Mon Sep 17 00:00:00 2001 >From: Andreas Schneider <asn@samba.org> >Date: Wed, 6 Aug 2025 16:33:21 +0200 >Subject: [PATCH 8/9] auth:creds: Make sure to uppercase the realm of a > principal > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15893 > >Signed-off-by: Andreas Schneider <asn@samba.org> >Reviewed-by: Alexander Bokovoy <ab@samba.org> >(cherry picked from commit 5879410caf9303a378f3d90365e60928a735e65a) >--- > auth/credentials/credentials.c | 40 ++++++++++++++++++++++++------- > python/samba/tests/credentials.py | 4 ++-- > 2 files changed, 34 insertions(+), 10 deletions(-) > >diff --git a/auth/credentials/credentials.c b/auth/credentials/credentials.c >index 777bf53430d..f7b95957124 100644 >--- a/auth/credentials/credentials.c >+++ b/auth/credentials/credentials.c >@@ -379,9 +379,31 @@ _PUBLIC_ char *cli_credentials_get_principal_and_obtained(struct cli_credentials > > if (cred->principal_obtained == CRED_CALLBACK && > !cred->callback_running) { >+ const char *princ = NULL; >+ > cred->callback_running = true; >- cred->principal = cred->principal_cb(cred); >+ princ = cred->principal_cb(cred); > cred->callback_running = false; >+ >+ cred->principal = NULL; >+ if (princ != NULL) { >+ char *p = NULL; >+ >+ cred->principal = talloc_strdup(cred, princ); >+ if (cred->principal == NULL) { >+ return NULL; >+ } >+ >+ p = strchr(cred->principal, '@'); >+ if (p != NULL) { >+ p += 1; >+ >+ for (; p[0] != '\0'; p++) { >+ *p = toupper(p[0]); >+ } >+ } >+ } >+ > if (cred->principal_obtained == CRED_CALLBACK) { > cred->principal_obtained = CRED_CALLBACK_RESULT; > cli_credentials_invalidate_ccache(cred, cred->principal_obtained); >@@ -459,6 +481,15 @@ _PUBLIC_ bool cli_credentials_set_principal(struct cli_credentials *cred, > if (cred->principal == NULL) { > return false; > } >+ >+ p = strchr(cred->principal, '@'); >+ if (p != NULL) { >+ p += 1; >+ >+ for (; p[0] != '\0'; p++) { >+ *p = toupper(p[0]); >+ } >+ } > } > cred->principal_obtained = obtained; > cli_credentials_invalidate_ccache(cred, cred->principal_obtained); >@@ -1077,8 +1108,6 @@ _PUBLIC_ void cli_credentials_parse_string(struct cli_credentials *credentials, > } > > if ((p = strchr_m(uname,'@'))) { >- char *x = NULL; >- > /* > * We also need to set username and domain > * in order to undo the effect of >@@ -1087,11 +1116,6 @@ _PUBLIC_ void cli_credentials_parse_string(struct cli_credentials *credentials, > cli_credentials_set_username(credentials, uname, obtained); > cli_credentials_set_domain(credentials, "", obtained); > >- /* Make sure the realm is uppercase */ >- for (x = p + 1; x[0] != '\0'; x++) { >- *x = toupper_m(*x); >- } >- > cli_credentials_set_principal(credentials, uname, obtained); > *p = 0; > cli_credentials_set_realm(credentials, p+1, obtained); >diff --git a/python/samba/tests/credentials.py b/python/samba/tests/credentials.py >index bc132681c48..1835d9b7b59 100644 >--- a/python/samba/tests/credentials.py >+++ b/python/samba/tests/credentials.py >@@ -361,7 +361,7 @@ class CredentialsTests(samba.tests.TestCaseInTempDir): > self.assertEqual(creds.get_username(), "env_user") > self.assertEqual(creds.get_domain(), lp.get("workgroup").upper()) > self.assertEqual(creds.get_realm(), realm.upper()) >- self.assertEqual(creds.get_principal(), "unknown@realm.example.com") >+ self.assertEqual(creds.get_principal(), "unknown@REALM.EXAMPLE.COM") > creds.parse_string("domain\\user") > self.assertEqual(creds.get_username(), "user") > self.assertEqual(creds.get_domain(), "DOMAIN") >@@ -385,7 +385,7 @@ class CredentialsTests(samba.tests.TestCaseInTempDir): > self.assertEqual(creds.get_username(), "env_user") > self.assertEqual(creds.get_domain(), lp.get("workgroup").upper()) > self.assertEqual(creds.get_realm(), realm.upper()) >- self.assertEqual(creds.get_principal(), "unknown@realm.example.com") >+ self.assertEqual(creds.get_principal(), "unknown@REALM.EXAMPLE.COM") > creds.parse_string("domain\\user") > self.assertEqual(creds.get_username(), "user") > self.assertEqual(creds.get_domain(), "DOMAIN") >-- >2.50.1 > > >From 14e4ac1341149b5bd4d76e94b1628f0f526e0cc7 Mon Sep 17 00:00:00 2001 >From: Andreas Schneider <asn@samba.org> >Date: Wed, 6 Aug 2025 07:54:52 +0200 >Subject: [PATCH 9/9] auth:creds: Update the documentation for set_principal > and set_realm > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15893 > >Signed-off-by: Andreas Schneider <asn@samba.org> >Reviewed-by: Alexander Bokovoy <ab@samba.org> > >Autobuild-User(master): Alexander Bokovoy <ab@samba.org> >Autobuild-Date(master): Mon Aug 25 12:08:22 UTC 2025 on atb-devel-224 > >(cherry picked from commit 7a19fde92605a3a3699998fb226e3e787de0b5ca) >--- > auth/credentials/credentials.c | 37 ++++++++++++++++++++++++++++------ > 1 file changed, 31 insertions(+), 6 deletions(-) > >diff --git a/auth/credentials/credentials.c b/auth/credentials/credentials.c >index f7b95957124..dab1c047c13 100644 >--- a/auth/credentials/credentials.c >+++ b/auth/credentials/credentials.c >@@ -461,9 +461,24 @@ _PUBLIC_ char *cli_credentials_get_principal(struct cli_credentials *cred, TALLO > return cli_credentials_get_principal_and_obtained(cred, mem_ctx, &obtained); > } > >+/** >+ * @brief Set the principal for the credentials context. >+ * >+ * The realm of the principal will be checked if it is ASCII only and upper >+ * cased if it isn't yet. >+ * >+ * @param cred The credential context. >+ * >+ * @param val The principal to set or NULL to reset. >+ * >+ * @param obtained This way the described principal was specified. >+ * >+ * @return true on success, false if the realm is not ASCII or the allocation >+ * failed. >+ */ > _PUBLIC_ bool cli_credentials_set_principal(struct cli_credentials *cred, >- const char *val, >- enum credentials_obtained obtained) >+ const char *val, >+ enum credentials_obtained obtained) > { > if (obtained >= cred->principal_obtained) { > /* If `val = NULL` is passed, principal is reset */ >@@ -991,12 +1006,22 @@ _PUBLIC_ const char *cli_credentials_get_realm(struct cli_credentials *cred) > } > > /** >- * Set the realm for this credentials context, and force it to >- * uppercase for the sanity of our local kerberos libraries >+ * @brief Set the realm for this credentials context. >+ * >+ * The realm be checked if it is ASCII only and upper cased if it isn't yet. >+ * >+ * @param cred The credential context. >+ * >+ * @param val The realm to set or NULL to reset. >+ * >+ * @param obtained This way the described realm was specified. >+ * >+ * @return true on success, false if the realm is not ASCII or the allocation >+ * failed. > */ > _PUBLIC_ bool cli_credentials_set_realm(struct cli_credentials *cred, >- const char *val, >- enum credentials_obtained obtained) >+ const char *val, >+ enum credentials_obtained obtained) > { > if (obtained >= cred->realm_obtained) { > /* If `val = NULL` is passed, realm is reset */ >-- >2.50.1 >
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=18694">View the attachment on a separate page</a>.</b>
View Attachment As Raw
Flags:
ab
:
review?
(
gd
)
pfilipensky
:
review+
Actions:
View
Attachments on
bug 15893
: 18694