linux-cifs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Shyam Prasad N <nspmangalore@gmail.com>
To: "Aurélien Aptel" <aaptel@suse.com>
Cc: CIFS <linux-cifs@vger.kernel.org>,
	samba-technical@lists.samba.org,
	Pavel Shilovsky <piastryyy@gmail.com>,
	Steve French <smfrench@gmail.com>,
	sribhat.msa@outlook.com
Subject: Re: [PATCH][SMB3] mount.cifs integration with PAM
Date: Wed, 23 Sep 2020 17:36:48 +0530	[thread overview]
Message-ID: <CANT5p=r0Jix9EuuF8gJzQBGHLp0Y-Oogxzju7_2cJog_jF2fjg@mail.gmail.com> (raw)
In-Reply-To: <87mu1yc6gw.fsf@suse.com>

[-- Attachment #1: Type: text/plain, Size: 1484 bytes --]

I tried to use this as a fallback option, but there's an issue. During
the first mount attempt, cifs.upcall pushes the credentials lookup
failure to the keyring. So on the next attempt, request_key() doesn't
even call into the userspace, since there's already an entry there. So
we'll probably have to call it under a new child process on the
fallback attempt.

@Aurélien Aptel Do you think that it is worth the effort? or should we
just keep force_pam as a mount option?

Also, I'll test this out with DFS once I figure out how to set it up. :)
Re-attaching the patch with some minor changes with just the
"force_pam" mount option.

Regards,
Shyam

On Thu, Sep 10, 2020 at 3:13 PM Aurélien Aptel <aaptel@suse.com> wrote:
>
> Shyam Prasad N <nspmangalore@gmail.com> writes:
> > Your understanding is correct. We could also go for a hybrid approach,
> > where we fallback to option b when option a fails to authenticate.
> > But for now, I'll resubmit a patch with option a as a fallback when
> > regular mount fails, just like you had suggested.
>
> Please try DFS setups as well. On DFS links a sub-mount is made from the
> kernel and mount.cifs is not called.
>
> Cheers,
> --
> Aurélien Aptel / SUSE Labs Samba Team
> GPG: 1839 CB5F 9F5B FB9B AA97  8C99 03C8 A49B 521B D5D3
> SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, DE
> GF: Felix Imendörffer, Mary Higgins, Sri Rasiah HRB 247165 (AG München)



-- 
-Shyam

[-- Attachment #2: 0001-mount.cifs-Have-an-option-to-authenticate-against-PA.patch --]
[-- Type: application/octet-stream, Size: 9450 bytes --]

From 6ad829f2e0e3d0390580d1a2a0bf74b7f8a321aa Mon Sep 17 00:00:00 2001
From: Shyam Prasad N <sprasad@microsoft.com>
Date: Thu, 13 Aug 2020 08:53:08 -0700
Subject: [PATCH] mount.cifs: Have an option to authenticate against PAM.

Authentication against PAM has two benefits:
- The PAM module (winbind or sssd) will perform the house-keeping of
the krb5 tickets and make sure they aren't expired.
- The mount.cifs utility need not rely on sshd or su to authenticate
against PAM and arrange the krb5 TGT. In case of ssh with private keys,
ssh doesn't involve PAM. So krb5 TGT may be missing.

This introduces a new PAM application named "cifs".
The user may use this to customize the PAM stack for just this application.
Otherwise, the default PAM configuration is used.
Also, to disable this feature at the time of build, one can run
configure with --enable-krb5pam=no.
---
 Makefile.am  |   2 +-
 configure.ac |  27 +++++++++
 mount.cifs.c | 157 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 182 insertions(+), 4 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index fe9cd34..51c5c47 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -4,7 +4,7 @@ ACLOCAL_AMFLAGS = -I aclocal
 root_sbindir = $(ROOTSBINDIR)
 root_sbin_PROGRAMS = mount.cifs
 mount_cifs_SOURCES = mount.cifs.c mtab.c resolve_host.c util.c
-mount_cifs_LDADD = $(LIBCAP) $(CAPNG_LDADD) $(RT_LDADD)
+mount_cifs_LDADD = $(LIBCAP) $(LIBPAM) $(CAPNG_LDADD) $(RT_LDADD)
 include_HEADERS = cifsidmap.h
 rst_man_pages = mount.cifs.8
 
diff --git a/configure.ac b/configure.ac
index 22e78ef..fa8af0f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -55,6 +55,11 @@ AC_ARG_ENABLE(pam,
 	enable_pam=$enableval,
 	enable_pam="maybe")
 
+AC_ARG_ENABLE(krb5pam,
+    [AS_HELP_STRING([--enable-krb5pam],[Add PAM authentication support when using sec=krb5 @<:@default=yes@])],,
+    enable_krb5pam=$enableval
+    enable_krb5pam="yes")
+
 AC_ARG_ENABLE(systemd,
 	[AS_HELP_STRING([--enable-systemd],[Enable systemd specific behavior for mount.cifs @<:@default=yes@:>@])],
 	enable_systemd=$enableval,
@@ -241,6 +246,27 @@ if test $enable_pam != "no"; then
 			])
 fi
 
+if test "x$enable_krb5pam" = "xno"; then
+	enable_krb5pam="no"
+else
+	AC_CHECK_LIB([pam], [pam_start], enable_krb5pam="yes", enable_krb5pam="no", )
+	AC_CHECK_HEADERS([security/pam_appl.h], ,
+			 [
+				if test $enable_krb5pam = "yes"; then
+					AC_MSG_ERROR([security/pam_appl.h not found, consider installing keyutils-libs-devel.])
+				else
+					AC_MSG_WARN([security/pam_appl.h not found, consider installing pam-devel. Disabling cifscreds PAM module.])
+					enable_krb5pam="no"
+				fi
+			 ])
+fi
+
+if test "$enable_krb5pam" = "yes"; then
+	AC_DEFINE([HAVE_KRB5PAM],[1], [Define if PAM auth is enabled for krb5])
+	LIBPAM=-lpam
+	AC_SUBST(LIBPAM)
+fi
+
 # ugly, but I'm not sure how to check for functions in a library that's not in $LIBS
 cu_saved_libs=$LIBS
 LIBS="$LIBS $KRB5_LDADD"
@@ -288,6 +314,7 @@ AM_CONDITIONAL(CONFIG_CIFSACL, [test "$enable_cifsacl" != "no"])
 AM_CONDITIONAL(CONFIG_SMBINFO, [test "$enable_smbinfo" != "no"])
 AM_CONDITIONAL(CONFIG_PYTHON_TOOLS, [test "$enable_pythontools" != "no"])
 AM_CONDITIONAL(CONFIG_PAM, [test "$enable_pam" != "no"])
+AM_CONDITIONAL(CONFIG_KRB5PAM, [test "$enable_krb5pam" != "no"])
 AM_CONDITIONAL(CONFIG_PLUGIN, [test "$enable_cifsidmap" != "no" -o "$enable_cifsacl" != "no"])
 
 LIBCAP_NG_PATH
diff --git a/mount.cifs.c b/mount.cifs.c
index 4feb397..4a0d03f 100644
--- a/mount.cifs.c
+++ b/mount.cifs.c
@@ -46,6 +46,9 @@
 #include <time.h>
 #include <sys/mman.h>
 #include <sys/wait.h>
+#ifdef HAVE_KRB5PAM
+#include <security/pam_appl.h>
+#endif /* HAVE_KRB5PAM */
 #ifdef HAVE_SYS_FSUID_H
 #include <sys/fsuid.h>
 #endif /* HAVE_SYS_FSUID_H */
@@ -163,6 +166,7 @@
 #define OPT_BKUPGID    31
 #define OPT_NOFAIL     32
 #define OPT_SNAPSHOT   33
+#define OPT_FORCE_PAM  34
 
 #define MNT_TMP_FILE "/.mtab.cifs.XXXXXX"
 
@@ -189,6 +193,8 @@ struct parsed_mount_info {
 	unsigned int verboseflag:1;
 	unsigned int nofail:1;
 	unsigned int got_domain:1;
+	unsigned int is_krb5:1;
+	unsigned int force_pam:1;
 };
 
 static const char *thisprogram;
@@ -713,6 +719,8 @@ static int parse_opt_token(const char *token)
 		return OPT_PASS;
 	if (strcmp(token, "sec") == 0)
 		return OPT_SEC;
+	if (strcmp(token, "force_pam") == 0)
+		return OPT_FORCE_PAM;
 	if (strcmp(token, "ip") == 0 ||
 		strcmp(token, "addr") == 0)
 		return OPT_IP;
@@ -891,12 +899,19 @@ parse_options(const char *data, struct parsed_mount_info *parsed_info)
 
 		case OPT_SEC:
 			if (value) {
-				if (!strncmp(value, "none", 4) ||
-				    !strncmp(value, "krb5", 4))
+				if (!strncmp(value, "none", 4))
 					parsed_info->got_password = 1;
+				if (!strncmp(value, "krb5", 4))
+					parsed_info->is_krb5 = 1;
 			}
 			break;
 
+#ifdef HAVE_KRB5PAM
+		case OPT_FORCE_PAM:
+			parsed_info->force_pam = 1;
+			goto nocopy;
+#endif /* HAVE_KRB5PAM */
+			
 		case OPT_IP:
 			if (!value || !*value) {
 				fprintf(stderr,
@@ -1809,6 +1824,119 @@ get_password(const char *prompt, char *input, int capacity)
 	return input;
 }
 
+#ifdef HAVE_KRB5PAM
+#define PAM_CIFS_SERVICE "cifs"
+
+static int
+pam_auth_krb5_conv(int n, const struct pam_message **msg,
+    struct pam_response **resp, void *data)
+{
+    struct parsed_mount_info *parsed_info;
+	struct pam_response *reply;
+	int i;
+
+	*resp = NULL;
+
+    parsed_info = data;
+    if (parsed_info == NULL)
+		return (PAM_CONV_ERR);
+
+	if (n <= 0 || n > PAM_MAX_NUM_MSG)
+		return (PAM_CONV_ERR);
+
+	if ((reply = calloc(n, sizeof(*reply))) == NULL)
+		return (PAM_CONV_ERR);
+
+	for (i = 0; i < n; ++i) {
+		switch (msg[i]->msg_style) {
+		case PAM_PROMPT_ECHO_OFF:
+            if ((reply[i].resp = (char *) malloc(MOUNT_PASSWD_SIZE + 1)) == NULL)
+                goto fail;
+
+            if (parsed_info->got_password && parsed_info->password != NULL) {
+                strncpy(reply[i].resp, parsed_info->password, MOUNT_PASSWD_SIZE + 1);
+            } else if (get_password(msg[i]->msg, reply[i].resp, MOUNT_PASSWD_SIZE + 1) == NULL) {
+                goto fail;
+            }
+            reply[i].resp[MOUNT_PASSWD_SIZE] = '\0';
+
+			reply[i].resp_retcode = PAM_SUCCESS;
+			break;
+		case PAM_PROMPT_ECHO_ON:
+			fprintf(stderr, "%s: ", msg[i]->msg);
+            if ((reply[i].resp = (char *) malloc(MOUNT_PASSWD_SIZE + 1)) == NULL)
+                goto fail;
+
+			if (fgets(reply[i].resp, MOUNT_PASSWD_SIZE + 1, stdin) == NULL)
+                goto fail;
+
+            reply[i].resp[MOUNT_PASSWD_SIZE] = '\0';
+
+			reply[i].resp_retcode = PAM_SUCCESS;
+			break;
+		case PAM_ERROR_MSG:
+		case PAM_TEXT_INFO:
+			fprintf(stderr, "%s: ", msg[i]->msg);
+
+			reply[i].resp_retcode = PAM_SUCCESS;
+			break;
+		default:
+			goto fail;
+		}
+	}
+	*resp = reply;
+	return (PAM_SUCCESS);
+
+ fail:
+	for(i = 0; i < n; i++) {
+        if (reply[i].resp)
+            free(reply[i].resp);
+	}
+	free(reply);
+	return (PAM_CONV_ERR);
+}
+
+static int 
+pam_auth_krb5_user(struct parsed_mount_info *parsed_info)
+{
+    int rc = -1;
+    pam_handle_t *pamh = NULL;
+    struct pam_conv pam_conv = {
+        .conv = pam_auth_krb5_conv,
+        .appdata_ptr = (void *) parsed_info
+    };
+    
+    fprintf(stdout, "Authenticating as user: %s\n", parsed_info->username);
+    rc = pam_start(PAM_CIFS_SERVICE, parsed_info->username, &pam_conv, &pamh);
+    if (rc != PAM_SUCCESS) {
+        fprintf(stderr, "Error starting PAM transaction: %s\n", pam_strerror(pamh, rc));
+        return rc;
+    }
+
+    rc = pam_authenticate(pamh, 0);
+    if (rc != PAM_SUCCESS) {
+        fprintf(stderr, "Error in authenticating user with PAM: %s\n", pam_strerror(pamh, rc));
+        goto end;
+    }
+
+    rc = pam_acct_mgmt(pamh, 0);
+    if (rc != PAM_SUCCESS) {
+        fprintf(stderr, "User account invalid: %s\n", pam_strerror(pamh, rc));
+        goto end;
+    }
+
+    rc = pam_setcred(pamh, PAM_ESTABLISH_CRED);
+    if (rc != PAM_SUCCESS) {
+        fprintf(stderr, "Error in setting PAM credentials: %s\n", pam_strerror(pamh, rc));
+        goto end;
+    }
+
+end:
+    pam_end(pamh, rc);
+    return rc;
+}
+#endif /* HAVE_KRB5PAM */
+
 static int
 assemble_mountinfo(struct parsed_mount_info *parsed_info,
 		   const char *thisprogram, const char *mountpoint,
@@ -1891,7 +2019,30 @@ assemble_mountinfo(struct parsed_mount_info *parsed_info,
 		parsed_info->got_user = 1;
 	}
 
-	if (!parsed_info->got_password) {
+#ifdef HAVE_KRB5PAM
+	if (parsed_info->is_krb5 && parsed_info->force_pam) {
+		/* 
+		 * Attempt to authenticate with PAM. 
+		 * If PAM is configured properly, let it get the krb5 tickets necessary for the mount.
+		 * Even if this fails, it could be the case of PAM not configured properly. 
+		 * In that case, retain the current behavior. So this is just a best-effort.
+		 */
+		rc = pam_auth_krb5_user(parsed_info);
+		if (rc) {
+			fprintf(stderr, "Attempt to authenticate user with " \
+					"PAM unsuccessful. Still, proceeding with mount.\n");
+            /*
+             * Even if this is a failure, fallthrough and see if cifs.ko can still 
+             * authenticate the user.
+             */
+		}
+
+		parsed_info->got_password = 1;
+	}
+#endif /* HAVE_KRB5PAM */
+
+	/* If sec=krb5, then password is collected by other means */
+	if (!parsed_info->is_krb5 && !parsed_info->got_password) {
 		char tmp_pass[MOUNT_PASSWD_SIZE + 1];
 		char *prompt = NULL;
 
-- 
2.25.1


  reply	other threads:[~2020-09-23 12:07 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-14  5:45 [PATCH][SMB3] mount.cifs integration with PAM Shyam Prasad N
2020-08-14  9:52 ` Aurélien Aptel
     [not found]   ` <CANT5p=oeY91u17DPe6WO75Eq_bjzrVC0kmAErrZ=h3S1qh-Wxw@mail.gmail.com>
2020-08-17  8:48     ` Aurélien Aptel
     [not found]       ` <CANT5p=rxp3iQMgxaM_mn3RE3B+zezWr3o8zpkFyWUR27CpeVCA@mail.gmail.com>
2020-09-09 11:04         ` Shyam Prasad N
2020-09-09 14:13           ` Aurélien Aptel
2020-09-09 17:25             ` Shyam Prasad N
2020-09-10  9:43               ` Aurélien Aptel
2020-09-23 12:06                 ` Shyam Prasad N [this message]
2020-09-23 13:56                   ` Aurélien Aptel
2020-09-24 10:39                     ` Shyam Prasad N
2020-11-09 23:42                       ` Pavel Shilovsky
2020-11-10 13:20                         ` Shyam Prasad N
2020-11-10 19:22                           ` Pavel Shilovsky
2020-11-27 10:43                             ` Shyam Prasad N
2020-12-14 18:03                               ` Stefan Metzmacher
     [not found]                                 ` <CANT5p=rYiY0xE-35swsFKVitZD2yTchRiReyA0wVvY+mU_qKEw@mail.gmail.com>
2021-01-30 14:24                                   ` Shyam Prasad N
2021-02-01 10:51                                     ` Aurélien Aptel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CANT5p=r0Jix9EuuF8gJzQBGHLp0Y-Oogxzju7_2cJog_jF2fjg@mail.gmail.com' \
    --to=nspmangalore@gmail.com \
    --cc=aaptel@suse.com \
    --cc=linux-cifs@vger.kernel.org \
    --cc=piastryyy@gmail.com \
    --cc=samba-technical@lists.samba.org \
    --cc=smfrench@gmail.com \
    --cc=sribhat.msa@outlook.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).