bug#57386: 29.0.50; support make-symbolic-link without admin priviledge on Windows

bug#57386: 29.0.50; support make-symbolic-link without admin priviledge on Windows

[Kiên Nguyễn Quang]

[-- Attachment #1.1: Type: text/plain, Size: 190 bytes --]

The CreateSymbolicLink Win32API has a flag that allows creating sym-links
without admin privileges SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE.
This patch added that support for w32 Emacs.

[-- Attachment #1.2: Type: text/html, Size: 296 bytes --]

[-- Attachment #2: 0001-symlink-allow-w32-user-to-create-symlink-without-adm.patch --]
[-- Type: application/octet-stream, Size: 1451 bytes --]

From 4c5c2b942473c6737ec9f6ad0ee55583ee985875 Mon Sep 17 00:00:00 2001
From: Kien Nguyen <kien.n.quang@gmail.com>
Date: Wed, 24 Aug 2022 23:19:35 +0900
Subject: [PATCH] allow w32 user to create symlink without admin privileged

* src/w32.c (symlink): allow w32 user to create symlink without admin privileged

---
 src/w32.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/w32.c b/src/w32.c
index cbcfcdd4f..23c0c8396 100644
--- a/src/w32.c
+++ b/src/w32.c
@@ -5992,12 +5992,15 @@ sys_umask (int mode)
 #ifndef SYMBOLIC_LINK_FLAG_DIRECTORY
 #define SYMBOLIC_LINK_FLAG_DIRECTORY 0x1
 #endif
+#ifndef SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE
+#define SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE 0x2
+#endif
 
 int
 symlink (char const *filename, char const *linkname)
 {
   char linkfn[MAX_UTF8_PATH], *tgtfn;
-  DWORD flags = 0;
+  DWORD flags = SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE;
   int dir_access, filename_ends_in_slash;
 
   /* Diagnostics follows Posix as much as possible.  */
@@ -6055,7 +6058,7 @@ symlink (char const *filename, char const *linkname)
      directory.  */
   filename_ends_in_slash = IS_DIRECTORY_SEP (filename[strlen (filename) - 1]);
   if (dir_access == 0 || filename_ends_in_slash)
-    flags = SYMBOLIC_LINK_FLAG_DIRECTORY;
+    flags |= SYMBOLIC_LINK_FLAG_DIRECTORY;
 
   tgtfn = (char *)map_w32_filename (filename, NULL);
   if (filename_ends_in_slash)
-- 
2.37.0.windows.1

bug#57386: 29.0.50; support make-symbolic-link without admin priviledge on Windows

[Eli Zaretskii]

> From: Kiên Nguyễn Quang
>  <kien.n.quang@gmail.com>
> Date: Thu, 25 Aug 2022 00:33:58 +0900
> 
> The CreateSymbolicLink Win32API has a flag that allows creating sym-links without admin privileges
> SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE.
> This patch added that support for w32 Emacs.

Thanks, but we cannot use that flag unconditionally, because it is not
supported on all Windows versions.  The addition of the flags should
be conditioned on the version of Windows on which Emacs runs.

bug#57386: 29.0.50; support make-symbolic-link without admin priviledge on Windows

[Kiên Nguyễn Quang]

[-- Attachment #1.1: Type: text/plain, Size: 1003 bytes --]

You're right, there's a stupid check against parameters on the version that
doesn't support the new flag.
Since from Windows 10, the Windows version checking is not recommended (and
not accurate) anymore, I think we can just retry if the function is called
with invalid parameters.
That will probably slow down the down-level Windows versions, but the
difference should not be too much.

On Thu, Aug 25, 2022 at 1:00 AM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Kiên Nguyễn Quang
> >  <kien.n.quang@gmail.com>
> > Date: Thu, 25 Aug 2022 00:33:58 +0900
> >
> > The CreateSymbolicLink Win32API has a flag that allows creating
> sym-links without admin privileges
> > SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE.
> > This patch added that support for w32 Emacs.
>
> Thanks, but we cannot use that flag unconditionally, because it is not
> supported on all Windows versions.  The addition of the flags should
> be conditioned on the version of Windows on which Emacs runs.
>

[-- Attachment #1.2: Type: text/html, Size: 1460 bytes --]

[-- Attachment #2: 0001-symlink-allow-w32-user-to-create-symlink-without-adm.patch --]
[-- Type: application/octet-stream, Size: 2088 bytes --]

From 3ade8436b9c4a0c8d71a252068abf370ebe848ba Mon Sep 17 00:00:00 2001
From: Kien Nguyen <kien.n.quang@gmail.com>
Date: Wed, 24 Aug 2022 23:19:35 +0900
Subject: [PATCH] symlink: allow w32 non-admin user to create symlink

* src/w32.c (symlink): allow w32 user to create symlink without admin privileged

---
 src/w32.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/src/w32.c b/src/w32.c
index cbcfcdd4f..407befbda 100644
--- a/src/w32.c
+++ b/src/w32.c
@@ -5992,12 +5992,15 @@ sys_umask (int mode)
 #ifndef SYMBOLIC_LINK_FLAG_DIRECTORY
 #define SYMBOLIC_LINK_FLAG_DIRECTORY 0x1
 #endif
+#ifndef SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE
+#define SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE 0x2
+#endif
 
 int
 symlink (char const *filename, char const *linkname)
 {
   char linkfn[MAX_UTF8_PATH], *tgtfn;
-  DWORD flags = 0;
+  DWORD flags = SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE;
   int dir_access, filename_ends_in_slash;
 
   /* Diagnostics follows Posix as much as possible.  */
@@ -6055,7 +6058,7 @@ symlink (char const *filename, char const *linkname)
      directory.  */
   filename_ends_in_slash = IS_DIRECTORY_SEP (filename[strlen (filename) - 1]);
   if (dir_access == 0 || filename_ends_in_slash)
-    flags = SYMBOLIC_LINK_FLAG_DIRECTORY;
+    flags |= SYMBOLIC_LINK_FLAG_DIRECTORY;
 
   tgtfn = (char *)map_w32_filename (filename, NULL);
   if (filename_ends_in_slash)
@@ -6069,6 +6072,16 @@ symlink (char const *filename, char const *linkname)
       if (errno != ENOSYS)
 	{
 	  DWORD w32err = GetLastError ();
+	  /* Remove the flag and try again if the API is not supported */
+	  if (w32err == ERROR_INVALID_PARAMETER)
+	    {
+	      errno = 0;
+	      flags &= ~SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE;
+	      if (create_symbolic_link (linkfn, tgtfn, flags))
+		goto success;
+
+	      w32err = GetLastError ();
+	    }
 
 	  switch (w32err)
 	    {
@@ -6104,6 +6117,8 @@ symlink (char const *filename, char const *linkname)
 	}
       return -1;
     }
+
+ success:
   return 0;
 }
 
-- 
2.37.0.windows.1

bug#57386: 29.0.50; support make-symbolic-link without admin priviledge on Windows

[Eli Zaretskii]

> From: Kiên Nguyễn Quang <kien.n.quang@gmail.com>
> Date: Thu, 25 Aug 2022 03:05:37 +0900
> Cc: 57386@debbugs.gnu.org
> 
> You're right, there's a stupid check against parameters on the version that doesn't support the new flag.
> Since from Windows 10, the Windows version checking is not recommended (and not accurate) anymore, I
> think we can just retry if the function is called with invalid parameters.
> That will probably slow down the down-level Windows versions, but the difference should not be too much.

Unfortunately, invalid parameters could cause Emacs to abort,
depending on how it was linked.  So I still think checking the version
of Windows is the way to go.  That MS don't recommend that doesn't
mean we must abide by their recommendations.

bug#57386: 29.0.50; support make-symbolic-link without admin priviledge on Windows

[Kiên Nguyễn Quang]

[-- Attachment #1: Type: text/plain, Size: 1322 bytes --]

In my new patch, if we received the ERROR_INVALID_PARAMETER, we will try to
create the symbolic link again without the new flag.
Isn't that ok for now?
The Windows version API has returned the same thing from the start of
Windows 10 so we pretty much can't detect the Windows version by using it.
Unless we read the information directly from the registry and parse it by
ourselves, which is something I would like to avoid.

On Thu, Aug 25, 2022 at 3:26 AM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Kiên Nguyễn Quang <kien.n.quang@gmail.com>
> > Date: Thu, 25 Aug 2022 03:05:37 +0900
> > Cc: 57386@debbugs.gnu.org
> >
> > You're right, there's a stupid check against parameters on the version
> that doesn't support the new flag.
> > Since from Windows 10, the Windows version checking is not recommended
> (and not accurate) anymore, I
> > think we can just retry if the function is called with invalid
> parameters.
> > That will probably slow down the down-level Windows versions, but the
> difference should not be too much.
>
> Unfortunately, invalid parameters could cause Emacs to abort,
> depending on how it was linked.  So I still think checking the version
> of Windows is the way to go.  That MS don't recommend that doesn't
> mean we must abide by their recommendations.
>

[-- Attachment #2: Type: text/html, Size: 1832 bytes --]

bug#57386: 29.0.50; support make-symbolic-link without admin priviledge on Windows

[Eli Zaretskii]

> From: Kiên Nguyễn Quang <kien.n.quang@gmail.com>
> Date: Thu, 25 Aug 2022 10:30:01 +0900
> Cc: 57386@debbugs.gnu.org
> 
> In my new patch, if we received the ERROR_INVALID_PARAMETER, we will try to create the symbolic link
> again without the new flag.
> Isn't that ok for now?

Sorry, no.  I want to avoid the ERROR_INVALID_PARAMETER error
entirely, for the reasons I explained up-thread.  That error happens
only if the underlying Windows version is too old and doesn't support
this flag, so a version check will avoid it.  If the Windows version
is new enough, that error will never happen, even if the Developer
Option is not activated; instead, the call will simply fail (with
ERROR_PRIVILEGE_NOT_HELD) as if the flag were never used.  So checking
the Windows version will allow us not to trigger the invalid parameter
error, and will also avoid calling the API twice.

> The Windows version API has returned the same thing from the start of Windows 10 so we pretty much
> can't detect the Windows version by using it.
> Unless we read the information directly from the registry and parse it by ourselves, which is something I
> would like to avoid.

That's a separate problem, which we will resolve if and when it
becomes relevant.  For now, this problem doesn't exist, since AFAIK
there are no features yet which we want to use that are available only
in Windows 11 and later.  So please assume that version checks in
Emacs are reliable, and always will be, no matter what MS says about
that.

Thanks.

bug#57386: 29.0.50; support make-symbolic-link without admin priviledge on Windows

[Kiên Nguyễn Quang]

[-- Attachment #1: Type: text/plain, Size: 897 bytes --]

>
>
> That's a separate problem, which we will resolve if and when it
> becomes relevant.  For now, this problem doesn't exist, since AFAIK
> there are no features yet which we want to use that are available only
> in Windows 11 and later.  So please assume that version checks in
> Emacs are reliable, and always will be, no matter what MS says about
> that.
>
> Thanks.
>

The new flag is available starting from Windows 10 Creator update, which
means two (or 3? I don't know) updates after the first Windows 10 release.
Since the version returned for all Windows 10 are the same, you see, that's
the problem.

I think the error is already caught and should be replaced with the new one
if we retry calling the API, can you elaborate more on how it can still
affect Emacs?
Is there somewhere Emacs is calling GetLastError without invoking the API
so the stored error is mistakenly used?

Thanks

[-- Attachment #2: Type: text/html, Size: 1256 bytes --]

bug#57386: 29.0.50; support make-symbolic-link without admin priviledge on Windows

[Eli Zaretskii]

> From: Kiên Nguyễn Quang <kien.n.quang@gmail.com>
> Date: Thu, 25 Aug 2022 17:52:50 +0900
> Cc: 57386@debbugs.gnu.org
> 
>  That's a separate problem, which we will resolve if and when it
>  becomes relevant.  For now, this problem doesn't exist, since AFAIK
>  there are no features yet which we want to use that are available only
>  in Windows 11 and later.  So please assume that version checks in
>  Emacs are reliable, and always will be, no matter what MS says about
>  that.
> 
>  Thanks.
> 
> The new flag is available starting from Windows 10 Creator update, which means two (or 3? I don't know)
> updates after the first Windows 10 release.
> Since the version returned for all Windows 10 are the same, you see, that's the problem.

If that's the problem (is anyone who has Windows 10 still likely to
use those old releases, given the automatic updates in Windows 10 that
cannot be turned off? and if they do, do they indeed get
ERROR_INVALID_PARAMETER?), we can handle that as well.  We have
already an API for accessing the Registry (see w32-read-registry and
its C counterpart w32_read_registry), and report-emacs-bug already
uses it to obtain the full description of the version, the build
number and all.  If needed, we can use a similar technique to make an
accurate version test.

> I think the error is already caught and should be replaced with the new one if we retry calling the API, can you
> elaborate more on how it can still affect Emacs?

When you call an API with a parameter that is invalid, you are risking
an exception, depending on the API and the build (debug or not).  This
has various unpleasant consequences; in the worst case, the Emacs
process could be terminated.  As documented, the problem is limited to
CRT functions, but our general policy is to avoid that even when using
the Win32 APIs.  And version check is a simple enough way of avoiding
that, so I see no reason not to do it here.

bug#57386: 29.0.50; support make-symbolic-link without admin priviledge on Windows

[Kiên Nguyễn Quang]

[-- Attachment #1.1: Type: text/plain, Size: 1140 bytes --]

>
>
>  (is anyone who has Windows 10 still likely to
> use those old releases, given the automatic updates in Windows 10 that
> cannot be turned off? and if they do, do they indeed get
> ERROR_INVALID_PARAMETER?)
>
There is LTS branch of Windows that gets updated regularly and doesn't have
to update to a newer release build.
Those users (mostly enterprise users/devs) will get the
ERROR_INVALID_PARAMETER, I've tested that.

> When you call an API with a parameter that is invalid, you are risking
> an exception, depending on the API and the build (debug or not).  This
> has various unpleasant consequences; in the worst case, the Emacs
> process could be terminated.  As documented, the problem is limited to
> CRT functions, but our general policy is to avoid that even when using
> the Win32 APIs.  And version check is a simple enough way of avoiding
> that, so I see no reason not to do it here.

Okay, that makes sense. Although I think that's the problem of CRT APIs
only, as API should never throw exceptions, which is a foreign concept to
ABI and requires an exact library match to handle correctly

The new patch is attached.

[-- Attachment #1.2: Type: text/html, Size: 1563 bytes --]

[-- Attachment #2: 0002-Make-native-comp-debug-0-for-Windows.patch --]
[-- Type: application/octet-stream, Size: 879 bytes --]

From 8ab79a8b30cb55c7a41fe48bc7a6002bb62be44d Mon Sep 17 00:00:00 2001
From: Kien Nguyen <kien.n.quang@gmail.com>
Date: Sat, 15 May 2021 10:57:25 +0900
Subject: [PATCH] Make native-comp-debug 0 for Windows

* lisp/emacs-lisp/comp.el (native-comp-debug): dont make debug native-comp for Windows

---
 lisp/emacs-lisp/comp.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/emacs-lisp/comp.el b/lisp/emacs-lisp/comp.el
index 3e7f17ef1c..85d0cea212 100644
--- a/lisp/emacs-lisp/comp.el
+++ b/lisp/emacs-lisp/comp.el
@@ -55,7 +55,7 @@ native-comp-speed
   :safe #'integerp
   :version "28.1")
 
-(defcustom native-comp-debug (if (eq 'windows-nt system-type) 1 0)
+(defcustom native-comp-debug 0
   "Debug level for native compilation, a number between 0 and 3.
 This is intended for debugging the compiler itself.
   0 no debug output.
-- 
2.31.1.windows.1

bug#57386: 29.0.50; support make-symbolic-link without admin priviledge on Windows

[Eli Zaretskii]

> From: Kiên Nguyễn Quang <kien.n.quang@gmail.com>
> Date: Thu, 25 Aug 2022 18:51:30 +0900
> Cc: 57386@debbugs.gnu.org
> 
> > When you call an API with a parameter that is invalid, you are risking
> > an exception, depending on the API and the build (debug or not).  This
> > has various unpleasant consequences; in the worst case, the Emacs
> > process could be terminated.  As documented, the problem is limited to
> > CRT functions, but our general policy is to avoid that even when using
> > the Win32 APIs.  And version check is a simple enough way of avoiding
> > that, so I see no reason not to do it here.
> 
> Okay, that makes sense. Although I think that's the problem of CRT APIs only, as API should never throw
> exceptions, which is a foreign concept to ABI and requires an exact library match to handle correctly
> 
> The new patch is attached.

Thanks, but I think you sent a wrong patch?

bug#57386: 29.0.50; support make-symbolic-link without admin priviledge on Windows

[Kiên Nguyễn Quang]

[-- Attachment #1.1: Type: text/plain, Size: 1146 bytes --]

Oops. Here it is.

On Thu, Aug 25, 2022 at 6:57 PM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Kiên Nguyễn Quang <kien.n.quang@gmail.com>
> > Date: Thu, 25 Aug 2022 18:51:30 +0900
> > Cc: 57386@debbugs.gnu.org
> >
> > > When you call an API with a parameter that is invalid, you are risking
> > > an exception, depending on the API and the build (debug or not).  This
> > > has various unpleasant consequences; in the worst case, the Emacs
> > > process could be terminated.  As documented, the problem is limited to
> > > CRT functions, but our general policy is to avoid that even when using
> > > the Win32 APIs.  And version check is a simple enough way of avoiding
> > > that, so I see no reason not to do it here.
> >
> > Okay, that makes sense. Although I think that's the problem of CRT APIs
> only, as API should never throw
> > exceptions, which is a foreign concept to ABI and requires an exact
> library match to handle correctly
> >
> > The new patch is attached.
>
> Thanks, but I think you sent a wrong patch?
>


-- 
 Nguyen Quang Kien - グエン クアン キエン
 Software Developer @ MSD

[-- Attachment #1.2: Type: text/html, Size: 1873 bytes --]

[-- Attachment #2: 0001-symlink-allow-w32-user-to-create-symlink-without-adm.patch --]
[-- Type: application/octet-stream, Size: 1538 bytes --]

From d108561e58a3a5117b259bf03cb790db1407e670 Mon Sep 17 00:00:00 2001
From: Kien Nguyen <kien.n.quang@gmail.com>
Date: Wed, 24 Aug 2022 23:19:35 +0900
Subject: [PATCH] symlink: allow w32 non-admin user to create symlink

* src/w32.c (symlink): allow w32 user to create symlink without admin privileged

---
 src/w32.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/src/w32.c b/src/w32.c
index cbcfcdd4f..5e5ed980f 100644
--- a/src/w32.c
+++ b/src/w32.c
@@ -5992,12 +5992,17 @@ sys_umask (int mode)
 #ifndef SYMBOLIC_LINK_FLAG_DIRECTORY
 #define SYMBOLIC_LINK_FLAG_DIRECTORY 0x1
 #endif
+#ifndef SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE
+#define SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE 0x2
+#endif
 
 int
 symlink (char const *filename, char const *linkname)
 {
   char linkfn[MAX_UTF8_PATH], *tgtfn;
-  DWORD flags = 0;
+  /* The new flag is supported from build 14972 */
+  DWORD flags = (w32_build_number >= 14972) ?
+    SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE : 0;
   int dir_access, filename_ends_in_slash;
 
   /* Diagnostics follows Posix as much as possible.  */
@@ -6055,7 +6060,7 @@ symlink (char const *filename, char const *linkname)
      directory.  */
   filename_ends_in_slash = IS_DIRECTORY_SEP (filename[strlen (filename) - 1]);
   if (dir_access == 0 || filename_ends_in_slash)
-    flags = SYMBOLIC_LINK_FLAG_DIRECTORY;
+    flags |= SYMBOLIC_LINK_FLAG_DIRECTORY;
 
   tgtfn = (char *)map_w32_filename (filename, NULL);
   if (filename_ends_in_slash)
-- 
2.37.0.windows.1

bug#57386: 29.0.50; support make-symbolic-link without admin priviledge on Windows

[Eli Zaretskii]

> From: Kiên Nguyễn Quang <kien.n.quang@gmail.com>
> Date: Thu, 25 Aug 2022 19:32:13 +0900
> Cc: 57386@debbugs.gnu.org
> 
> Oops. Here it is.

Thanks.  I installed this (with minor changes), but please in the
future make sure your commit log messages follow our conventions (as
described in CONTRIBUTE), because this patch was rejected by our Git
commit hooks, and I had to apply it "by hand".

bug#57386: 29.0.50; support make-symbolic-link without admin priviledge on Windows

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Kiên Nguyễn Quang <kien.n.quang@gmail.com> writes:

> There is LTS branch of Windows that gets updated regularly and doesn't
> have to update to a newer release build.  Those users (mostly
> enterprise users/devs) will get the ERROR_INVALID_PARAMETER, I've
> tested that.

What about Windows 95 and 98? Will reading the registry to obtain the
API version return correct values there as well?

Thanks.

bug#57386: 29.0.50; support make-symbolic-link without admin priviledge on Windows

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: Eli Zaretskii <eliz@gnu.org>,  57386@debbugs.gnu.org
> Date: Thu, 25 Aug 2022 19:35:39 +0800
> 
> Kiên Nguyễn Quang <kien.n.quang@gmail.com> writes:
> 
> > There is LTS branch of Windows that gets updated regularly and doesn't
> > have to update to a newer release build.  Those users (mostly
> > enterprise users/devs) will get the ERROR_INVALID_PARAMETER, I've
> > tested that.
> 
> What about Windows 95 and 98? Will reading the registry to obtain the
> API version return correct values there as well?

It should, although I never had a chance of testing that.

But the patch eventually submitted doesn't access the Registry at all.