Re: [PATCH][SMB3] mount.cifs integration with PAM

Aurélien Aptel
2020-09-23T13:56:31Z

Shyam Prasad N <nspmangalore@gmail.com> writes:
> 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.

You will need 2 Windows VM. DFS is basically symlinks across
servers. The DFS root VM will host the links (standalone namespace) and
you have to make them point to shares on the 2nd VM. You don't need to
setup replication to test.

When you mount the root in cifs.ko and access a link, the server will
reply that the file is remote. cifs.ko then does an FSCTL on the link to
resolve the target it points to and then connects to the target and
mounts it under the link seemlessly.

Regarding the patch:

* need to update the man page with option and explanation

I have some comments with the style, I know it's annoying.. but it
would be best to keep the same across the code.

* use the existing indent style (tabs) and avoid adding trailing whitespaces.
* no () for return statements
* no casting for memory allocation
* if (X) free(X) => free(X)

Below some comments about pam_auth_krb5_conv():

> @@ -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)

Do we need to remove the trailing \n from the buffer?

> + goto fail;
> +
> + reply[i].resp[MOUNT_PASSWD_SIZE] = '\0';
> +
> + reply[i].resp_retcode = PAM_SUCCESS;
> + break;
> + case PAM_ERROR_MSG:

Shouldn't this PAM_ERROR_MSG case goto fail?

> + 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(NULL) is a no-op, remove the checks.

> + }
> + free(reply);
> + return (PAM_CONV_ERR);
> +}

I gave this a try with a properly configured system joined to AD from
local root account:

aaptel$ ./configure
...
checking krb5.h usability... yes
checking krb5.h presence... yes
checking for krb5.h... yes
checking krb5/krb5.h usability... yes
checking krb5/krb5.h presence... yes
checking for krb5/krb5.h... yes
checking for keyvalue in krb5_keyblock... no
...
checking keyutils.h usability... yes
checking keyutils.h presence... yes
checking for keyutils.h... yes
checking for krb5_init_context in -lkrb5... yes
...
checking for WBCLIENT... yes
checking for wbcSidsToUnixIds in -lwbclient... yes
...
checking for keyutils.h... (cached) yes
checking security/pam_appl.h usability... yes
checking security/pam_appl.h presence... yes
checking for security/pam_appl.h... yes
checking for pam_start in -lpam... yes
checking for security/pam_appl.h... (cached) yes
checking for krb5_auth_con_getsendsubkey... yes
checking for krb5_principal_get_realm... no
checking for krb5_free_unparsed_name... yes
checking for krb5_auth_con_setaddrs... yes
checking for krb5_auth_con_set_req_cksumtype... yes
...
aaptel$ make
....(ok)

Without force_pam:

root# ./mount.cifs -v //adnuc.nuc.test/data /x -o sec=krb5,username=administrator,domain=NUC
mount.cifs kernel mount options: ip=192.168.2.111,unc=\\adnuc.nuc.test\data,sec=krb5,user=administrator,domain=NUC
mount error(2): No such file or directory
Refer to the mount.cifs(8) manual page (e.g. man mount.cifs) and kernel log messages (dmesg)

With force_pam:

root# ./mount.cifs -v //adnuc.nuc.test/data /x -o sec=krb5,username=administrator,domain=NUC,force_pam
Authenticating as user: administrator
Error in authenticating user with PAM: Authentication failure
Attempt to authenticate user with PAM unsuccessful. Still, proceeding with mount.

=> no further message but mount failed and no msg in dmesg, it didn't
reach the mount() call

Not sure what is going on. Does the domain need to be passed to PAM?

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)