smbd: Simplify smbd_dirptr_get_entry()
authorVolker Lendecke <vl@samba.org>
Mon, 13 Nov 2023 12:46:51 +0000 (13:46 +0100)
committerVolker Lendecke <vl@samba.org>
Wed, 15 Nov 2023 05:10:35 +0000 (05:10 +0000)
This uses the much simpler openat_pathef_fsp_lcomp, avoiding
non_widelink_open where we don't need it. The only case where we still
have to call openat_pathref_fsp() in its full capacity is to find out
whether a symlink we found is dangling or not.

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
source3/smbd/dir.c

index b0a0dbae27313ac64b962e3c9cb199b7b4178a7e..5d37dbafa7af6c6dde262720b210eee46bb89e53 100644 (file)
@@ -551,10 +551,10 @@ bool smbd_dirptr_get_entry(TALLOC_CTX *ctx,
                char *dname = NULL;
                char *fname = NULL;
                struct smb_filename *smb_fname = NULL;
-               struct open_symlink_err *symlink_err = NULL;
                uint32_t mode = 0;
                bool get_dosmode = get_dosmode_in;
                bool toplevel_dotdot;
+               bool visible;
                bool ok;
 
                dname = dptr_ReadDirName(ctx, dirptr);
@@ -589,44 +589,48 @@ bool smbd_dirptr_get_entry(TALLOC_CTX *ctx,
 
                toplevel_dotdot = toplevel && ISDOTDOT(dname);
 
-               if (ISDOT(dname) || ISDOTDOT(dname)) {
-
-                       const char *dotname = dname;
+               smb_fname = synthetic_smb_fname(talloc_tos(),
+                                               toplevel_dotdot ? "." : dname,
+                                               NULL,
+                                               NULL,
+                                               dir_fname->twrp,
+                                               dir_fname->flags);
+               if (smb_fname == NULL) {
+                       TALLOC_FREE(dname);
+                       return false;
+               }
 
-                       if (toplevel_dotdot) {
-                               /*
-                                * Handle ".." in toplevel like "." to not
-                                * leak info from outside the share.
-                                */
-                               dotname = ".";
-                       }
+               /*
+                * UCF_POSIX_PATHNAMES to avoid the readdir fallback
+                * if we get raced between readdir and unlink.
+                */
+               status = openat_pathref_fsp_lcomp(dir_hnd->fsp,
+                                                 smb_fname,
+                                                 UCF_POSIX_PATHNAMES);
+               if (!NT_STATUS_IS_OK(status)) {
+                       DBG_DEBUG("Could not open %s: %s\n",
+                                 dname,
+                                 nt_errstr(status));
+                       TALLOC_FREE(smb_fname);
+                       TALLOC_FREE(fname);
+                       TALLOC_FREE(dname);
+                       continue;
+               }
 
-                       smb_fname = synthetic_smb_fname(talloc_tos(),
-                                                       dotname,
-                                                       NULL,
-                                                       NULL,
-                                                       dir_fname->twrp,
-                                                       dir_fname->flags);
-                       if (smb_fname == NULL) {
-                               TALLOC_FREE(dname);
-                               return false;
-                       }
+               visible = is_visible_fsp(smb_fname->fsp);
+               if (!visible) {
+                       TALLOC_FREE(smb_fname);
+                       TALLOC_FREE(fname);
+                       TALLOC_FREE(dname);
+                       continue;
+               }
 
-                       status = openat_pathref_fsp(dir_hnd->fsp, smb_fname);
-                       if (!NT_STATUS_IS_OK(status)) {
-                               DBG_INFO("Could not open \"..\": %s\n",
-                                        nt_errstr(status));
-                               TALLOC_FREE(smb_fname);
-                               TALLOC_FREE(dname);
-                               continue;
+               if (!S_ISLNK(smb_fname->st.st_ex_mode)) {
+                       if (get_dosmode) {
+                               mode = fdos_mode(smb_fname->fsp);
+                               smb_fname->st = smb_fname->fsp->fsp_name->st;
                        }
 
-                       mode = fdos_mode(smb_fname->fsp);
-
-                       /*
-                        * Don't leak INO/DEV/User SID/Group SID about
-                        * the containing directory of the share.
-                        */
                        if (toplevel_dotdot) {
                                /*
                                 * Ensure posix fileid and sids are hidden
@@ -640,123 +644,74 @@ bool smbd_dirptr_get_entry(TALLOC_CTX *ctx,
                        goto done;
                }
 
-               status = openat_pathref_fsp_nosymlink(talloc_tos(),
-                                                     conn,
-                                                     dir_hnd->fsp,
-                                                     dname,
-                                                     dir_fname->twrp,
-                                                     posix,
-                                                     &smb_fname,
-                                                     &symlink_err);
-
-               if (NT_STATUS_IS_OK(status)) {
-                       bool visible = is_visible_fsp(smb_fname->fsp);
-                       if (!visible) {
-                               TALLOC_FREE(smb_fname);
-                               TALLOC_FREE(dname);
-                               TALLOC_FREE(fname);
-                               continue;
-                       }
-               } else if (NT_STATUS_EQUAL(status,
-                                          NT_STATUS_STOPPED_ON_SYMLINK)) {
-                       struct symlink_reparse_struct *reparse =
-                               symlink_err->reparse;
-                       const char *target = reparse->substitute_name;
-                       bool is_msdfs_link;
-
-                       is_msdfs_link = lp_host_msdfs();
-                       is_msdfs_link &= lp_msdfs_root(SNUM(conn));
-                       is_msdfs_link &= (strncmp(target, "msdfs:", 6) == 0);
-
-                       if (is_msdfs_link) {
-                               char *path = NULL;
-
-                               path = full_path_from_dirfsp_at_basename(
-                                       talloc_tos(),
-                                       dir_hnd->fsp,
-                                       dname);
-                               if (path == NULL) {
-                                       return false;
-                               }
+               if (lp_host_msdfs() && lp_msdfs_root(SNUM(conn)) &&
+                   is_msdfs_link(dir_hnd->fsp, smb_fname))
+               {
+                       DBG_INFO("Masquerading msdfs link %s as a directory\n",
+                                smb_fname->base_name);
 
-                               smb_fname =
-                                       synthetic_smb_fname(talloc_tos(),
-                                                           path,
-                                                           NULL,
-                                                           &symlink_err->st,
-                                                           dir_fname->twrp,
-                                                           dir_fname->flags);
-                               TALLOC_FREE(path);
-                               if (smb_fname == NULL) {
-                                       return false;
-                               }
+                       smb_fname->st.st_ex_mode = (smb_fname->st.st_ex_mode &
+                                                   ~S_IFMT) |
+                                                  S_IFDIR;
 
-                               DBG_INFO("Masquerading msdfs link %s as a"
-                                        "directory\n",
-                                        smb_fname->base_name);
+                       mode = dos_mode_msdfs(conn, dname, &smb_fname->st);
+                       get_dosmode = false;
+                       ask_sharemode = false;
+                       goto done;
+               }
 
-                               smb_fname->st.st_ex_mode =
-                                       (smb_fname->st.st_ex_mode & ~S_IFMT) |
-                                       S_IFDIR;
+               if (posix) {
+                       /*
+                        * Posix always wants to see symlinks,
+                        * dangling or not. We've done the
+                        * openat_pathref_fsp() to fill in
+                        * smb_fname->fsp just in case it's not
+                        * dangling.
+                        */
+                       mode = FILE_ATTRIBUTE_NORMAL;
+                       get_dosmode = false;
+                       ask_sharemode = false;
+                       goto done;
+               }
 
-                               mode = dos_mode_msdfs(conn,
-                                                     dname,
-                                                     &smb_fname->st);
-                               get_dosmode = false;
-                               ask_sharemode = false;
-                               goto done;
-                       }
+               if (!lp_follow_symlinks(SNUM(conn))) {
+                       /*
+                        * Hide symlinks not followed
+                        */
+                       TALLOC_FREE(smb_fname);
+                       TALLOC_FREE(fname);
+                       TALLOC_FREE(dname);
+                       continue;
+               }
 
-                       smb_fname = synthetic_smb_fname(talloc_tos(),
-                                                       dname,
-                                                       NULL,
-                                                       &symlink_err->st,
-                                                       dir_fname->twrp,
-                                                       dir_fname->flags);
-                       if (smb_fname == NULL) {
-                               return false;
-                       }
+               /*
+                * We have to find out if it's a dangling
+                * symlink. Use the fat logic behind
+                * openat_pathref_fsp().
+                */
 
-                       status = openat_pathref_fsp(dir_hnd->fsp, smb_fname);
+               {
+                       struct files_struct *fsp = smb_fname->fsp;
+                       smb_fname_fsp_unlink(smb_fname);
+                       fd_close(fsp);
+                       file_free(NULL, fsp);
+               }
 
-                       if (posix) {
-                               /*
-                                * Posix always wants to see symlinks,
-                                * dangling or not. We've done the
-                                * openat_pathref_fsp() to fill in
-                                * smb_fname->fsp just in case it's
-                                * not dangling.
-                                */
-                               mode = FILE_ATTRIBUTE_NORMAL;
-                               get_dosmode = false;
-                               ask_sharemode = false;
-                               goto done;
-                       }
+               status = openat_pathref_fsp(dir_hnd->fsp, smb_fname);
 
-                       if (!NT_STATUS_IS_OK(status)) {
-                               /*
-                                * Dangling symlink. Hide.
-                                */
-                               TALLOC_FREE(smb_fname);
-                               TALLOC_FREE(fname);
-                               TALLOC_FREE(dname);
-                               continue;
-                       }
-               } else {
-                       DBG_NOTICE("Could not open %s: %s\n",
-                                  dname,
-                                  nt_errstr(status));
-                       TALLOC_FREE(dname);
-                       TALLOC_FREE(fname);
+               if (!NT_STATUS_IS_OK(status)) {
+                       /*
+                        * Dangling symlink. Hide.
+                        */
                        TALLOC_FREE(smb_fname);
+                       TALLOC_FREE(fname);
+                       TALLOC_FREE(dname);
                        continue;
                }
 
                if (get_dosmode) {
                        mode = fdos_mode(smb_fname->fsp);
-                       if (smb_fname->fsp != NULL) {
-                               smb_fname->st = smb_fname->fsp->fsp_name->st;
-                       }
+                       smb_fname->st = smb_fname->fsp->fsp_name->st;
                }
 
        done: