1

I have created a small framework that provides a unified API to multiple file systems/APIs (namely Win32, Posix, NFS). Said API is somewhat similar to Posix -- to access a file you need to "open" it providing a hint for intended purpose (r, w or rw). Something like open_file("/abc/log.txt", access::rw).

Supporting Win32 API in this framework gives me a headache due to "declarative" nature of Win32 -- you are supposed to know upfront which operations you plan to perform on given handle and pass related dwDesiredAccess into related (Nt)CreateFile() call. Unfortunately framework has no idea what operation client is going to perform (i.e. change-owner, write-attributes, etc) besides generic r/w/rw hint. And I am not willing to let Win32 concepts to leak into my framework (i.e. I don't like adding dwDesiredAccess equivalent into my open_file()).

Here is what I've tried:

1. MAXIMUM_ALLOWED

Idea: Open related handles with MAXIMUM_ALLOWED -- I'll get everything I could and if some right is missing, related operation (e.g. set_mime()) will simply fail with access denied.

Problems:

  • it doesn't work with read-only files or volumes ((Nt)CreateFile() fails with access denied)
  • MSDN warns that if defragmentation is in progress on FAT volume -- trying to open a directory this way will fail
  • in general it seems that using MAXIMUM_ALLOWED is frowned upon for some reason

2. Reopen object when necessary

Idea: Represent r/w/rw though GENERIC_READ and GENERIC_WRITE and for all operations that require additional access (e.g. delete() requires DELETE) reopen the object with required access.

Problems:

  • Reopening object is not cheap
  • changes made via second object can be silently overwritten, for example:
    • set_mtime() reopens the file with FILE_WRITE_ATTRIBUTES|SYNCHRONIZE
    • calls NtSetInformationFile(... FileBasicInformation) to update metadata and closes the handle
    • later original handle gets closed, it causes data flush and silently overwrites ModifiedTime previously set by set_mtime()

3. Duplicate handle instead of reopening object

Idea: same as in previous section, but instead of reopening object -- duplicate original handle (asking for new access):

HANDLE h;
HANDLE hp = GetCurrentProcess();
CHECK_WIN32( DuplicateHandle(hp, hFile, hp, &h, FILE_WRITE_ATTRIBUTES|SYNCHRONIZE, FALSE, 0) );

Problems:

  • Duplicating (and closing) file handle every time I need to perform (a non-plain-read/write) operation seem to be excessive and somewhat expensive
  • DuplicateHandle() documentation warns (without giving any details) that asking for additional access may fail. It has been working fine in all use cases I checked it for (typically asking for things like DELETE/FILE_WRITE_ATTRIBUTES on handles opened with GENERIC_READ), but apparently Win32 API provides no guarantees :-/

... otherwise approach seem to be working.

Bottomline:

I am looking for a way to address MAXIMUM_ALLOWED issues. (Or suggestions for alternative approach, maybe?)

C.M.
  • 3,071
  • 1
  • 14
  • 33
  • About [3](https://learn.microsoft.com/en-us/windows/win32/api/handleapi/nf-handleapi-duplicatehandle): "*in other cases, DuplicateHandle cannot create a handle with more access rights than the original. For example, a file handle created with the GENERIC_READ access right cannot be duplicated so that it has both the GENERIC_READ and GENERIC_WRITE access right*". – dxiv Dec 14 '20 at 22:50
  • @dxiv yeah, I saw this. Unfortunately MSDN doesn't specify any details and in my use cases asking for additional access (i.e. `FILE_WRITE_ATTRIBUTES` or `DELETE` when object is opened with `GENERIC_READ`) so far always worked... I am not comfortable with this approach at all, but I couldn't find anything better :-\ – C.M. Dec 14 '20 at 23:00
  • 1
    ReOpenFile lets you open a new handle to a file with different access rights. – Raymond Chen Dec 15 '20 at 00:15
  • @RaymondChen `ReOpenFile()` creates new kernel file object, I just checked -- it is the same as case #2 with the same problems. – C.M. Dec 15 '20 at 20:12

1 Answers1

0

Edit: Here is another reason why reopening file is not a good idea.

There is no way to use MAXIMUM_ALLOWED reliably -- R/O files and volumes cause it to error. Poorly designed feature.

Another approach is to get minimum access and "expand" it as required (by re-opening file with new dwAccessRequired flag). This does not work:

  • if you open file temporarily some changes made through new handle (e.g. mtime modification) will be wiped out later when original handle is closed (and underlying kernel object flushes data to disk)

  • if you try to replace old handle with new one this means expensive flush (on old handle close) + MT synchronization which means I can't efficiently use my file object from multiple threads (I know that right now due to FILE_SYNCHRONOUS_IO_NONALERT all operations are serialized anyway, but it will be fixed in near term)

Alas, DuplicateHandle() can't grant new access -- so this won't help either.

Basically, all I need is a thread-safe BOOL ExtendAccess(HANDLE h, DWORD dwAdditionalAccess) function. It looks like you can't have it even via NT API -- possible only in kernel mode.

Luckily this framework is always used under privileged account, which means I can enable SE_BACKUP_NAME, use FILE_OPEN_FOR_BACKUP_INTENT, over-request access (with minimal fallback in case of read-only volume) and avoid dealing with restrictive DACLs. Ah, and yes, deal with ReadOnly attribute in delete(). Still haven't decided what to do if user wants to open read-only file for writing...

I ended up with this:

W32Handle open_file_(HANDLE hparent, UNICODE_STRING zwpath, access a, disposition d, truncate t)
{
    ...
    ACCESS_MASK access = [a]() -> ACCESS_MASK {
        switch(a)
        {
        case access::r  : return GENERIC_READ;
        case access::w  : [[fallthrough]];                      // MSDN suggests to use GENERIC_READ with GENERIC_WRITE over network (performance reasons)
        case access::rw : return GENERIC_READ|GENERIC_WRITE;
        }
        UNREACHEABLE;
    }();

    constexpr DWORD write_access = FILE_WRITE_ATTRIBUTES|DELETE|WRITE_OWNER;    // we want to always have these (for apply, unlink, chown, etc)

    access |= write_access;
    access |= SYNCHRONIZE|READ_CONTROL|ACCESS_SYSTEM_SECURITY;                  // add "read DACL/SACL" rights (for full_metadata)

    ULONG flags = FILE_SYNCHRONOUS_IO_NONALERT|FILE_NON_DIRECTORY_FILE|FILE_OPEN_FOR_BACKUP_INTENT;

    OBJECT_ATTRIBUTES oa;
    InitializeObjectAttributes(&oa, &zwpath, 0, hparent, NULL);

    HANDLE h;
    IO_STATUS_BLOCK io;
    NTSTATUS r = ZwCreateFile(&h, access, &oa, &io, NULL, FILE_ATTRIBUTE_NORMAL, FILE_SHARE_VALID_FLAGS, disposition, flags, NULL, 0);
    if (r == STATUS_SUCCESS) return W32Handle(h);

    if (r == STATUS_MEDIA_WRITE_PROTECTED)      // try again without write flags
    {
        access &= ~write_access;
        r = ZwCreateFile(&h, access, &oa, &io, NULL, FILE_ATTRIBUTE_NORMAL, FILE_SHARE_VALID_FLAGS, disposition, flags, NULL, 0);
        if (r == STATUS_SUCCESS) return W32Handle(h);
    }

    HR_THROW_(HRESULT_FROM_NT(r), "%s: Failed to open file", __func__);
}

Overall terrible API, a spaghetti of special cases. I wish I had my own SMB client.

C.M.
  • 3,071
  • 1
  • 14
  • 33