CVS commit: [netbsd-8] src/lib/libpam/modules/pam_krb5

2023-10-02 Thread Martin Husemann
Module Name:src
Committed By:   martin
Date:   Mon Oct  2 13:09:01 UTC 2023

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

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

lib/libpam/modules/pam_krb5/pam_krb5.c: revision 1.32

pam_krb5: Fix PR lib/57631.

Loose ends in the fix for NetBSD-SA2023-006 that weren't caught by
review or, somehow, by my own testing.  Evidently we need automatic
tests for this pam business.


To generate a diff of this commit:
cvs rdiff -u -r1.26.18.1 -r1.26.18.2 \
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.c
diff -u src/lib/libpam/modules/pam_krb5/pam_krb5.c:1.26.18.1 src/lib/libpam/modules/pam_krb5/pam_krb5.c:1.26.18.2
--- src/lib/libpam/modules/pam_krb5/pam_krb5.c:1.26.18.1	Wed Jun 21 22:04:13 2023
+++ src/lib/libpam/modules/pam_krb5/pam_krb5.c	Mon Oct  2 13:09:01 2023
@@ -1,4 +1,4 @@
-/*	$NetBSD: pam_krb5.c,v 1.26.18.1 2023/06/21 22:04:13 martin Exp $	*/
+/*	$NetBSD: pam_krb5.c,v 1.26.18.2 2023/10/02 13:09:01 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.18.1 2023/06/21 22:04:13 martin Exp $");
+__RCSID("$NetBSD: pam_krb5.c,v 1.26.18.2 2023/10/02 13:09:01 martin Exp $");
 #endif
 
 #include 
@@ -341,7 +341,6 @@ pam_sm_authenticate(pam_handle_t *pamh, 
 	krbret = verify_krb_v5_tgt(pam_context, ccache, srvdup,
 	debug,
 	auth_service, auth_princ, auth_phost, _data);
-	free(srvdup);
 	if (krbret == -1) {
 		PAM_VERBOSE_ERROR("Kerberos 5 error");
 		krb5_cc_destroy(pam_context, ccache);
@@ -940,6 +939,7 @@ verify_krb_v5_tgt_begin(krb5_context con
 	const char *services[3], **service;
 
 	*servicep = NULL;
+	*princp = NULL;
 
 	if (debug)
 		openlog_r("pam_krb5", LOG_PID, LOG_AUTHPRIV, datap);
@@ -982,6 +982,8 @@ verify_krb_v5_tgt_begin(krb5_context con
 		);
 		if (retval != 0)
 			continue;
+		*servicep = *service;
+		*princp = princ;
 		break;
 	}
 	if (keyblock)



CVS commit: [netbsd-8] src/lib/libpam/modules/pam_krb5

2023-10-02 Thread Martin Husemann
Module Name:src
Committed By:   martin
Date:   Mon Oct  2 13:09:01 UTC 2023

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

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

lib/libpam/modules/pam_krb5/pam_krb5.c: revision 1.32

pam_krb5: Fix PR lib/57631.

Loose ends in the fix for NetBSD-SA2023-006 that weren't caught by
review or, somehow, by my own testing.  Evidently we need automatic
tests for this pam business.


To generate a diff of this commit:
cvs rdiff -u -r1.26.18.1 -r1.26.18.2 \
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.



CVS commit: [netbsd-8] src/lib/libpam/modules/pam_krb5

2023-06-21 Thread Martin Husemann
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.



CVS commit: [netbsd-8] src/lib/libpam/modules/pam_krb5

2023-06-21 Thread Martin Husemann
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 
@@ -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