win32_apply.c: better workaround for access denied bug when creating ADS

Removing directory DACLs was not really a good idea, since this state can
too easily remain after extraction.  Instead, since the unexpected
STATUS_ACCESS_DENIED error only occurs when creating an empty named data
stream, we can avoid the error by requesting no permissions on the
handle.  It happens that no permissions are needed anyway.

diff --git a/NEWS b/NEWS
index 30dea7718ea78976eca3aeeb3eb78d75cd681c4d..4bda7ea3f59706f5a6e11b0d0f7696cb5e0962d0 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -1,6 +1,9 @@
 Version 1.9.2-BETA:
        On UNIX, wimlib can now overwrite readonly files when extracting.
 
 Version 1.9.2-BETA:
        On UNIX, wimlib can now overwrite readonly files when extracting.
 

+       On Windows, fixed a bug where wimlib could leave a null DACL (a.k.a. "no
+       NTFS permissions") set on some existing directories after extraction.
+

        On Windows, when applying a WIM image in "WIMBoot mode" when the WOF
        driver is not loaded, wimlib can now correctly register a new WIM file
        with the target volume when the target volume previously had had WIM
        On Windows, when applying a WIM image in "WIMBoot mode" when the WOF
        driver is not loaded, wimlib can now correctly register a new WIM file
        with the target volume when the target volume previously had had WIM

diff --git a/src/win32_apply.c b/src/win32_apply.c
index 6f9e41b98393669f8ad42151ecd9407c4c2c6e88..d9bf63c2fd305b93cf932b1bbff9d0dc996f8c83 100644 (file)
--- a/src/win32_apply.c
+++ b/src/win32_apply.c
@@ -1471,10 +1471,11 @@ retry:
 /*
  * Create a nondirectory file or named data stream at the current path,
  * superseding any that already exists at that path.  If successful, return an
 /*
  * Create a nondirectory file or named data stream at the current path,
  * superseding any that already exists at that path.  If successful, return an

- * open handle to the file or named data stream.
+ * open handle to the file or named data stream with the requested permissions.

  */
 static int
  */
 static int

-supersede_file_or_stream(struct win32_apply_ctx *ctx, HANDLE *h_ret)
+supersede_file_or_stream(struct win32_apply_ctx *ctx, DWORD perms,
+                        HANDLE *h_ret)

 {
        NTSTATUS status;
        bool retried = false;
 {
        NTSTATUS status;
        bool retried = false;


@@ -1483,7 +1484,7 @@ supersede_file_or_stream(struct win32_apply_ctx *ctx, HANDLE *h_ret)
         * FILE_ATTRIBUTE_ENCRYPTED doesn't get set before we want it to be.  */
 retry:
        status = do_create_file(h_ret,
         * FILE_ATTRIBUTE_ENCRYPTED doesn't get set before we want it to be.  */
 retry:
        status = do_create_file(h_ret,

-                               GENERIC_READ | GENERIC_WRITE | DELETE,
+                               perms,

                                NULL,
                                FILE_ATTRIBUTE_SYSTEM,
                                FILE_CREATE,
                                NULL,
                                FILE_ATTRIBUTE_SYSTEM,
                                FILE_CREATE,


@@ -1595,7 +1596,13 @@ create_empty_streams(const struct wim_dentry *dentry,
                        build_extraction_path_with_ads(dentry, ctx,
                                                       strm->stream_name,
                                                       utf16le_len_chars(strm->stream_name));
                        build_extraction_path_with_ads(dentry, ctx,
                                                       strm->stream_name,
                                                       utf16le_len_chars(strm->stream_name));

-                       ret = supersede_file_or_stream(ctx, &h);
+                       /*
+                        * Note: do not request any permissions on the handle.
+                        * Otherwise, we may encounter a Windows bug where the
+                        * parent directory DACL denies read access to the new
+                        * named data stream, even when using backup semantics!
+                        */
+                       ret = supersede_file_or_stream(ctx, 0, &h);

 
                        build_extraction_path(dentry, ctx);
 
 
                        build_extraction_path(dentry, ctx);
 


@@ -1683,25 +1690,6 @@ retry:
                                             sizeof(basic_info),
                                             FileBasicInformation);
                }
                                             sizeof(basic_info),
                                             FileBasicInformation);
                }

-
-               /* Also try to remove the directory's DACL.  This isn't supposed
-                * to be necessary because we *always* use backup semantics.
-                * However, there is a case where NtCreateFile() fails with
-                * STATUS_ACCESS_DENIED when creating a named data stream that
-                * was just deleted, using a directory-relative open.  I have no
-                * idea why Windows is broken in this case.  */
-               if (!(ctx->common.extract_flags & WIMLIB_EXTRACT_FLAG_NO_ACLS)) {
-                       static const SECURITY_DESCRIPTOR_RELATIVE desc = {
-                               .Revision = SECURITY_DESCRIPTOR_REVISION1,
-                               .Control = SE_SELF_RELATIVE | SE_DACL_PRESENT,
-                               .Owner = 0,
-                               .Group = 0,
-                               .Sacl = 0,
-                               .Dacl = 0,
-                       };
-                       NtSetSecurityObject(h, DACL_SECURITY_INFORMATION,
-                                           (void *)&desc);
-               }

        }
 
        if (!dentry_is_root(dentry)) {
        }
 
        if (!dentry_is_root(dentry)) {


@@ -1771,7 +1759,9 @@ create_nondirectory_inode(HANDLE *h_ret, const struct wim_dentry *dentry,
 
        build_extraction_path(dentry, ctx);
 
 
        build_extraction_path(dentry, ctx);
 

-       ret = supersede_file_or_stream(ctx, &h);
+       ret = supersede_file_or_stream(ctx,
+                                      GENERIC_READ | GENERIC_WRITE | DELETE,
+                                      &h);

        if (ret)
                goto out;
 
        if (ret)
                goto out;