Module Name:    src
Committed By:   martin
Date:           Wed Jun 21 22:04:13 UTC 2023

Modified Files:
        src/lib/libpam/modules/pam_krb5 [netbsd-8]: pam_krb5.8 pam_krb5.c

Log Message:
Pull up following revision(s) (requested by riastradh in ticket #1844):

        lib/libpam/modules/pam_krb5/pam_krb5.c: revision 1.31
        lib/libpam/modules/pam_krb5/pam_krb5.8: revision 1.13

pam_krb5: Refuse to operate without a key to verify tickets.

New allow_kdc_spoof overrides this to restore previous behaviour
which was vulnerable to KDC spoofing, because without a host or
service key, pam_krb5 can't distinguish the legitimate KDC from a
spoofed one.

This way, having pam_krb5 enabled isn't dangerous even if you create
an empty /etc/krb5.conf to use client SSO without any host services.

Perhaps this should use krb5_verify_init_creds(3) instead, and
thereby respect the rather obscurely named krb5.conf option
verify_ap_req_nofail like the Linux pam_krb5 does, but:
- verify_ap_req_nofail is default-off (i.e., vulnerable by default),
- changing verify_ap_req_nofail to default-on would probably affect
  more things and therefore be riskier,
- allow_kdc_spoof is a much clearer way to spell the idea,
- this patch is a smaller semantic change and thus less risky, and
- a security change with compatibility issues shouldn't have a
  workaround that might introduce potentially worse security issues
  or more compatibility issues.

Perhaps this should use krb5_verify_user(3) with secure=1 instead,
for simplicity, but it's not clear how to do that without first
prompting for the password -- which we shouldn't do at all if we
later decide we won't be able to use it anyway -- and without
repeating a bunch of the logic here anyway to pick the service name.

References about verify_ap_req_nofail:
- mit-krb5 discussion about verify_ap_req_nofail:
  https://mailman.mit.edu/pipermail/krbdev/2011-January/009778.html
- Oracle has the default-secure setting in their krb5 system:
  https://docs.oracle.com/cd/E26505_01/html/E27224/setup-148.html
  
https://docs.oracle.com/cd/E26505_01/html/816-5174/krb5.conf-4.html#REFMAN4krb5.conf-4
  https://docs.oracle.com/cd/E19253-01/816-4557/gihyu/
- Heimdal issue on verify_ap_req_nofail default:
  https://github.com/heimdal/heimdal/issues/1129


To generate a diff of this commit:
cvs rdiff -u -r1.11 -r1.11.40.1 src/lib/libpam/modules/pam_krb5/pam_krb5.8
cvs rdiff -u -r1.26 -r1.26.18.1 src/lib/libpam/modules/pam_krb5/pam_krb5.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/lib/libpam/modules/pam_krb5/pam_krb5.8
diff -u src/lib/libpam/modules/pam_krb5/pam_krb5.8:1.11 src/lib/libpam/modules/pam_krb5/pam_krb5.8:1.11.40.1
--- src/lib/libpam/modules/pam_krb5/pam_krb5.8:1.11	Tue Dec  2 22:52:06 2008
+++ src/lib/libpam/modules/pam_krb5/pam_krb5.8	Wed Jun 21 22:04:13 2023
@@ -1,4 +1,4 @@
-.\" $NetBSD: pam_krb5.8,v 1.11 2008/12/02 22:52:06 reed Exp $
+.\" $NetBSD: pam_krb5.8,v 1.11.40.1 2023/06/21 22:04:13 martin Exp $
 .\" $FreeBSD: src/lib/libpam/modules/pam_krb5/pam_krb5.8,v 1.6 2001/11/24 23:41:32 dd Exp $
 .\"
 .\" Copyright (c) Frank Cusack, 1999-2001. All rights reserved.
@@ -142,6 +142,21 @@ and
 .Ql %p ,
 to designate the current process ID; can be used in
 .Ar name .
+.It Cm allow_kdc_spoof
+Allow
+.Nm
+to succeed even if there is no host or service key available in a
+keytab to authenticate the Kerberos KDC's ticket.
+If there is no such key, for example on a host with no keytabs,
+.Nm
+will fail immediately without prompting the user.
+.Pp
+.Sy Warning :
+If the host has not been configured with a keytab from the KDC, setting
+this option makes it vulnerable to malicious KDCs, e.g. via DNS
+flooding, because
+.Nm
+has no way to distinguish the legitimate KDC from a spoofed KDC.
 .El
 .Ss Kerberos 5 Account Management Module
 The Kerberos 5 account management component

Index: src/lib/libpam/modules/pam_krb5/pam_krb5.c
diff -u src/lib/libpam/modules/pam_krb5/pam_krb5.c:1.26 src/lib/libpam/modules/pam_krb5/pam_krb5.c:1.26.18.1
--- src/lib/libpam/modules/pam_krb5/pam_krb5.c:1.26	Sat Dec 28 18:04:03 2013
+++ src/lib/libpam/modules/pam_krb5/pam_krb5.c	Wed Jun 21 22:04:13 2023
@@ -1,4 +1,4 @@
-/*	$NetBSD: pam_krb5.c,v 1.26 2013/12/28 18:04:03 christos Exp $	*/
+/*	$NetBSD: pam_krb5.c,v 1.26.18.1 2023/06/21 22:04:13 martin Exp $	*/
 
 /*-
  * This pam_krb5 module contains code that is:
@@ -53,7 +53,7 @@
 #ifdef __FreeBSD__
 __FBSDID("$FreeBSD: src/lib/libpam/modules/pam_krb5/pam_krb5.c,v 1.22 2005/01/24 16:49:50 rwatson Exp $");
 #else
-__RCSID("$NetBSD: pam_krb5.c,v 1.26 2013/12/28 18:04:03 christos Exp $");
+__RCSID("$NetBSD: pam_krb5.c,v 1.26.18.1 2023/06/21 22:04:13 martin Exp $");
 #endif
 
 #include <sys/types.h>
@@ -85,7 +85,12 @@ __RCSID("$NetBSD: pam_krb5.c,v 1.26 2013
 
 static void	log_krb5(krb5_context, krb5_error_code, struct syslog_data *,
     const char *, ...) __printflike(4, 5);
-static int	verify_krb_v5_tgt(krb5_context, krb5_ccache, char *, int);
+static int	verify_krb_v5_tgt_begin(krb5_context, char *, int,
+    const char **, krb5_principal *, char[static BUFSIZ], struct syslog_data *);
+static int	verify_krb_v5_tgt(krb5_context, krb5_ccache, char *, int,
+    const char *, krb5_principal, char[static BUFSIZ], struct syslog_data *);
+static void	verify_krb_v5_tgt_cleanup(krb5_context, int,
+    const char *, krb5_principal, char[static BUFSIZ], struct syslog_data *);
 static void	cleanup_cache(pam_handle_t *, void *, int);
 static const	char *compat_princ_component(krb5_context, krb5_principal, int);
 static void	compat_free_data_contents(krb5_context, krb5_data *);
@@ -100,6 +105,7 @@ static void	compat_free_data_contents(kr
 #define PAM_OPT_RENEWABLE	"renewable"
 #define PAM_OPT_NO_CCACHE	"no_ccache"
 #define PAM_OPT_REUSE_CCACHE	"reuse_ccache"
+#define PAM_OPT_ALLOW_KDC_SPOOF	"allow_kdc_spoof"
 
 /*
  * authentication management
@@ -110,6 +116,11 @@ pam_sm_authenticate(pam_handle_t *pamh, 
 {
 	krb5_error_code krbret;
 	krb5_context pam_context;
+	int debug;
+	const char *auth_service;
+	krb5_principal auth_princ;
+	char auth_phost[BUFSIZ];
+	struct syslog_data auth_data = SYSLOG_DATA_INIT;
 	krb5_creds creds;
 	krb5_principal princ;
 	krb5_ccache ccache;
@@ -144,18 +155,46 @@ pam_sm_authenticate(pam_handle_t *pamh, 
 
 	PAM_LOG("Got service: %s", (const char *)service);
 
+	if ((srvdup = strdup(service)) == NULL) {
+		retval = PAM_BUF_ERR;
+		goto cleanup6;
+	}
+
 	krbret = krb5_init_context(&pam_context);
 	if (krbret != 0) {
 		PAM_VERBOSE_ERROR("Kerberos 5 error");
-		return (PAM_SERVICE_ERR);
+		retval = PAM_SERVICE_ERR;
+		goto cleanup5;
 	}
 
 	PAM_LOG("Context initialised");
 
+	debug = openpam_get_option(pamh, PAM_OPT_DEBUG) ? 1 : 0;
+	krbret = verify_krb_v5_tgt_begin(pam_context, srvdup, debug,
+	    &auth_service, &auth_princ, auth_phost, &auth_data);
+	if (krbret != 0) {	/* failed to find key */
+		/* Keytab or service key does not exist */
+		if (debug)
+			log_krb5(pam_context, krbret, &auth_data,
+			    "pam_krb5: verify_krb_v5_tgt: "
+			    "krb5_kt_read_service_key");
+		/*
+		 * Give up now because we can't authenticate the KDC
+		 * with a keytab, unless the administrator asked to
+		 * have the traditional behaviour of being vulnerable
+		 * to spoofed KDCs.
+		 */
+		if (!openpam_get_option(pamh, PAM_OPT_ALLOW_KDC_SPOOF)) {
+			retval = PAM_SERVICE_ERR;
+			goto cleanup4;
+		}
+	}
+
 	krbret = krb5_get_init_creds_opt_alloc(pam_context, &opts);
 	if (krbret != 0) {
 		PAM_VERBOSE_ERROR("Kerberos 5 error");
-		return (PAM_SERVICE_ERR);
+		retval = PAM_SERVICE_ERR;
+		goto cleanup4;
 	}
 
 	if (openpam_get_option(pamh, PAM_OPT_FORWARDABLE))
@@ -299,12 +338,9 @@ pam_sm_authenticate(pam_handle_t *pamh, 
 	PAM_LOG("Credentials stashed");
 
 	/* Verify them */
-	if ((srvdup = strdup(service)) == NULL) {
-		retval = PAM_BUF_ERR;
-		goto cleanup;
-	}
 	krbret = verify_krb_v5_tgt(pam_context, ccache, srvdup,
-	    openpam_get_option(pamh, PAM_OPT_DEBUG) ? 1 : 0);
+	    debug,
+	    auth_service, auth_princ, auth_phost, &auth_data);
 	free(srvdup);
 	if (krbret == -1) {
 		PAM_VERBOSE_ERROR("Kerberos 5 error");
@@ -354,11 +390,16 @@ cleanup3:
 
 	if (opts)
 		krb5_get_init_creds_opt_free(pam_context, opts);
+cleanup4:
+	verify_krb_v5_tgt_cleanup(pam_context, debug,
+	    auth_service, auth_princ, auth_phost, &auth_data);
 
 	krb5_free_context(pam_context);
+cleanup5:
+	free(srvdup);
 
-	PAM_LOG("Done cleanup3");
-
+	PAM_LOG("Done cleanup5");
+cleanup6:
 	if (retval != PAM_SUCCESS)
 		PAM_VERBOSE_ERROR("Kerberos 5 refuses you");
 
@@ -884,26 +925,24 @@ log_krb5(krb5_context ctx, krb5_error_co
  * the local keytab doesn't have it), and we cannot find another
  * service we do have, let her in.
  *
- * Returns 1 for confirmation, -1 for failure, 0 for uncertainty.
+ * - verify_krb_v5_tgt_begin returns a krb5 error code.
+ * - verify_krb_v5_tgt returns 0 on success, -1 on failure.
  */
 /* ARGSUSED */
 static int
-verify_krb_v5_tgt(krb5_context context, krb5_ccache ccache,
-    char *pam_service, int debug)
+verify_krb_v5_tgt_begin(krb5_context context, char *pam_service, int debug,
+    const char **servicep, krb5_principal *princp, char phost[static BUFSIZ],
+    struct syslog_data *datap)
 {
 	krb5_error_code retval;
 	krb5_principal princ;
 	krb5_keyblock *keyblock;
-	krb5_data packet;
-	krb5_auth_context auth_context = NULL;
-	char phost[BUFSIZ];
 	const char *services[3], **service;
-	struct syslog_data data = SYSLOG_DATA_INIT;
 
-	packet.data = 0;
+	*servicep = NULL;
 
 	if (debug)
-		openlog_r("pam_krb5", LOG_PID, LOG_AUTHPRIV, &data);
+		openlog_r("pam_krb5", LOG_PID, LOG_AUTHPRIV, datap);
 
 	/* If possible we want to try and verify the ticket we have
 	 * received against a keytab.  We will try multiple service
@@ -923,11 +962,11 @@ verify_krb_v5_tgt(krb5_context context, 
 		retval = krb5_sname_to_principal(context, NULL, *service,
 		    KRB5_NT_SRV_HST, &princ);
 		if (retval != 0 && debug)
-			log_krb5(context, retval, &data,
+			log_krb5(context, retval, datap,
 			    "pam_krb5: verify_krb_v5_tgt: "
 			    "krb5_sname_to_principal");
 		if (retval != 0)
-			return -1;
+			return (retval);
 
 		/* Extract the name directly. */
 		strncpy(phost, compat_princ_component(context, princ, 1),
@@ -945,21 +984,30 @@ verify_krb_v5_tgt(krb5_context context, 
 			continue;
 		break;
 	}
-	if (retval != 0) {	/* failed to find key */
-		/* Keytab or service key does not exist */
-		if (debug)
-			log_krb5(context, retval, &data,
-			    "pam_krb5: verify_krb_v5_tgt: "
-			    "krb5_kt_read_service_key");
-		retval = 0;
-		goto cleanup;
-	}
 	if (keyblock)
 		krb5_free_keyblock(context, keyblock);
 
+	return (retval);
+}
+
+static int
+verify_krb_v5_tgt(krb5_context context, krb5_ccache ccache,
+    char *pam_service, int debug,
+    const char *service, krb5_principal princ, char phost[static BUFSIZ],
+    struct syslog_data *datap)
+{
+	krb5_error_code retval;
+	krb5_auth_context auth_context = NULL;
+	krb5_data packet;
+
+	if (service == NULL)
+		return (0);	/* uncertain, can't authenticate KDC */
+
+	packet.data = 0;
+
 	/* Talk to the kdc and construct the ticket. */
 	auth_context = NULL;
-	retval = krb5_mk_req(context, &auth_context, 0, *service, phost,
+	retval = krb5_mk_req(context, &auth_context, 0, service, phost,
 		NULL, ccache, &packet);
 	if (auth_context) {
 		krb5_auth_con_free(context, auth_context);
@@ -967,7 +1015,7 @@ verify_krb_v5_tgt(krb5_context context, 
 	}
 	if (retval) {
 		if (debug)
-			log_krb5(context, retval, &data,
+			log_krb5(context, retval, datap,
 			    "pam_krb5: verify_krb_v5_tgt: "
 			    "krb5_mk_req");
 		retval = -1;
@@ -979,7 +1027,7 @@ verify_krb_v5_tgt(krb5_context context, 
 	    NULL, NULL);
 	if (retval) {
 		if (debug)
-			log_krb5(context, retval, &data,
+			log_krb5(context, retval, datap,
 			    "pam_krb5: verify_krb_v5_tgt: "
 			    "krb5_rd_req");
 		retval = -1;
@@ -988,16 +1036,25 @@ verify_krb_v5_tgt(krb5_context context, 
 		retval = 1;
 
 cleanup:
-	if (debug)
-		closelog_r(&data);
 	if (packet.data)
 		compat_free_data_contents(context, &packet);
 	if (auth_context) {
 		krb5_auth_con_free(context, auth_context);
 		auth_context = NULL;	/* setup for rd_req */
 	}
-	krb5_free_principal(context, princ);
-	return retval;
+	return (retval);
+}
+
+static void
+verify_krb_v5_tgt_cleanup(krb5_context context, int debug,
+    const char *service, krb5_principal princ, char phost[static BUFSIZ],
+    struct syslog_data *datap)
+{
+
+	if (service)
+		krb5_free_principal(context, princ);
+	if (debug)
+		closelog_r(datap);
 }
 
 /* Free the memory for cache_name. Called by pam_end() */

Reply via email to