Merged into next. Please let me know if something needs to be fixed. Thanks!
--
Best regards,
Pavel Shilovsky
чт, 24 сент. 2020 г. в 03:39, Shyam Prasad N <nspmangalore@xxxxxxxxx>:
>
> Hi Aurélien,
>
> I've implemented most of your review comments. Also fixed the issue.
>
> On Wed, Sep 23, 2020 at 7:26 PM Aurélien Aptel <aaptel@xxxxxxxx> wrote:
> >
> > Shyam Prasad N <nspmangalore@xxxxxxxxx> 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)
>
>
>
> --
> -Shyam
