This one needs some 🍿...

Windows, ah, Windows

So, on Windows, directories and files are somewhat more distinct than they are on Unix. The Windows CreateFile API (confusingly, this function is also responsible for opening existing files) will return handles to directories, but only if the strangely named FILE_FLAG_BACKUP_SEMANTICS is passed to it.

This is used in Unix.stat and Unix.readlink and #8796 noted that we should be doing it in Unix.utimes for directories. It all looks good, and it's at least what Python and Go do, if not other major cross-platform OS libraries.

But they are all wrong.

It's all in the semantics

Everyone gets duped by the sentence "To open a directory using CreateFile, specify the FILE_FLAG_BACKUP_SEMANTICS flag as part of dwFlagsAndAttributes." but fail to spot the following sentence: "Appropriate security checks still apply when this flag is used without SE_BACKUP_NAME and SE_RESTORE_NAME privileges."

So far, so good - but what happens if we're running as a user who has SE_BACKUP_NAME or SE_RESTORE_NAME? Well, unfortunately that means that our standard library function merrily bypasses all security checks on the files it's reading. It's not a privilege escalation, because the user has the capability, but it's somewhat akin to having a call to sudo embedded in a function for seemingly no reason.

The cure? If we have SeBackupPrivilege (for Unix.stat and Unix.readlink) or SeRestorePrivilege (for Unix.utimes), then we need to drop that privilege when we open the directory, in order to ensure that we don't invoke those powers and access a directory which you would not expect a general purpose function to access.

Some background for why I think this subtlety matters: SeBackupPrivilege and SeRestorePrivilege are intended, as the name implies, for backup programs but the privilege is by default available for the Administrators group. However, there are intentionally various steps you have to go through in your program to invoke the privilege:

• You have to have the right to use it in the first place
• You have to call an API function (AdjustTokenPrivileges, since you asked) to enable the privilege
• For every file you wish to access, you must add FILE_FLAG_BACKUP_SEMANTICS to the CreateFile call

Now, with a triple-lock like that in "the design", I don't think general purpose functions should be accidentally taking advantage of those privileges, however tricky it is to enable them.

The implementation

First of all, the implementation only uses FILE_FLAG_BACKUP_SEMANTICS for directories. If a directory is encountered, then the three functions now analyse the privileges of the security token associated with the thread making the call.

The first commit in this chain attempted to solve this at the process level, but it requires some enormously subtle interlocking to avoid a race condition between multiple calls. I didn't like it.

The second solution, which I dislike less, is to ensure that the thread making the call to CreateFile drops the privilege if necessary. This is far more complicated than a Unix programmer might expect, but about par for the course for the Windows API. Essentially:

• If the thread already has a specific security token, and the privilege is enabled, then it's disabled for the duration of the call. This scenario is very unlikely, because it requires the caller (i.e. the OCaml program running) to have done stuff to make this happen (there is also a highly obscure mode of failure which would cause the function to fail with an exception but, as Donald Knuth would say, "If you have been so devious as to get this message, you will understand it, and you deserve no sympathy.")
• If the thread does not have a specific security token (the normal state of affairs), then the process's token is duplicated and the privilege dropped for the duration of the call. This is in fact impersonation, but the process would be impersonating itself - at the end of the call it simply reverts the impersonation (again, there is an obscure failure mode here, deserving no sympathy).

I have also, whilst here, changed the behaviour of Unix.stat and Unix.readlink to return EACCES instead of ENOENT in some cases (I think this is an error from my 2015 code - you don't expect ENOENT on a file to which you don't have access). That change possibly wants moving to a separate PR.