From 6350f796fdd1c7519c35a9cd71b33b6dafc5ed3a Mon Sep 17 00:00:00 2001
From: NIIBE Yutaka <gniibe@fsij.org>
Date: Wed, 17 May 2023 10:55:17 +0900
Subject: w32: Cleaner semantics for PID and Process handle.

* src/assuan-defs.h (struct assuan_context_s): Introduce SERVER_PROC
member.  Clarify the use of PROCESS_ID and PID.
Introduce assuan_pid_t for internal use of process id or handle.
* src/assuan.h.in: Follow the change.
* src/assuan.c (assuan_new_ext): Likewise.
* src/client.c (_assuan_client_finish): Likewise.
* src/assuan-pipe-connect.c (pipe_connect): Likewise.
* src/server.c (_assuan_server_finish): Likewise.
* src/system-posix.c: Likewise.
* src/system-w32.c: Likewise.
* src/system.c: Likewise.
* src/assuan-pipe-server.c (assuan_init_pipe_server)
[HAVE_W32_SYSTEM]: Exclude the use of _assuan_pipe_connect_pid.
* src/assuan-socket-server.c (accept_connection_bottom)
[HAVE_W32_SYSTEM]: Exclude the use of the member peercred.pid.
* src/assuan-socket.c (_assuan_sock_check_nonce): Support Cygwin
Unix domain emulation for having valid client process ID.
* src/context.c (assuan_get_pid): Clarify the use cases.
* src/posix-types.inc.h, src/w32-types.inc.h: Introduce assuan_pid_t.

--

GnuPG-bug-id: 6487
Signed-off-by: NIIBE Yutaka <gniibe@fsij.org>
---
 src/assuan-defs.h          | 48 +++++++++++++++++++++++++++++++++++++++++-----
 src/assuan-pipe-connect.c  |  4 ++--
 src/assuan-pipe-server.c   |  4 ++++
 src/assuan-socket-server.c |  2 ++
 src/assuan-socket.c        |  2 ++
 src/assuan.c               |  7 +++++++
 src/assuan.h.in            | 19 +++++++++---------
 src/client.c               |  6 +++---
 src/context.c              | 22 ++++++++++++++++++++-
 src/posix-types.inc.h      |  1 +
 src/server.c               | 10 ++++++----
 src/system-posix.c         |  6 +++---
 src/system-w32.c           |  8 ++++----
 src/system.c               | 10 +++++-----
 src/w32-types.inc.h        |  6 ++++++
 15 files changed, 119 insertions(+), 36 deletions(-)

diff --git a/src/assuan-defs.h b/src/assuan-defs.h
index e55e7c7..faf9aae 100644
--- a/src/assuan-defs.h
+++ b/src/assuan-defs.h
@@ -179,9 +179,47 @@ struct assuan_context_s
 
   int max_accepts;  /* If we can not handle more than one connection,
 		       set this to 1, otherwise to -1.  */
-  pid_t pid;	  /* The pid of the peer. */
+
+  /*
+   * Process reference (PID on POSIX, Process Handle on Windows).
+   * Internal use, only valid for client with pipe.
+   */
+  assuan_pid_t server_proc;
+
+  /*
+   * NOTE: There are two different references for the process:
+   *
+   * (1) Process ID which is valid on a system.
+   * (2) Process handle which is private to the process that get it.
+   *
+   * POSIX system only has (1).
+   * Windows system has both of (1) and (2).
+   */
+
 #if defined(HAVE_W32_SYSTEM)
-  int process_id;     /* process ID of the server for FD passing.  */
+  /*
+   * The process ID of the peer.
+   *
+   * client with pipe: Used internally for FD passing.
+   * client with socket: Used internally for FD passing.
+   *
+   * server with pipe: Not valid.
+   * server with socket: Valid for Cygwin Unix domain socket emulation.
+   *
+   */
+  int process_id;
+#else
+  /*
+   * The pid of the peer.
+   *
+   * client with pipe: Not valid.
+   * client with socket: Not valid.
+   *
+   * server with pipe: Valid (by env _assuan_pipe_connect_pid).
+   * server with socket: Valid on a system with SO_PEERCRED/etc.
+   *
+   */
+  pid_t pid;
 #endif
   assuan_fd_t listen_fd;  /* The fd we are listening on (used by
                              socket servers) */
@@ -265,14 +303,14 @@ int _assuan_recvmsg (assuan_context_t ctx, assuan_fd_t fd,
 		     assuan_msghdr_t msg, int flags);
 int _assuan_sendmsg (assuan_context_t ctx, assuan_fd_t fd,
 		     assuan_msghdr_t msg, int flags);
-int _assuan_spawn (assuan_context_t ctx, pid_t *r_pid, const char *name,
+int _assuan_spawn (assuan_context_t ctx, assuan_pid_t *r_pid, const char *name,
 		   const char *argv[],
 		   assuan_fd_t fd_in, assuan_fd_t fd_out,
 		   assuan_fd_t *fd_child_list,
 		   void (*atfork) (void *opaque, int reserved),
 		   void *atforkvalue, unsigned int flags);
-pid_t  _assuan_waitpid (assuan_context_t ctx, pid_t pid, int nowait,
-			int *status, int options);
+assuan_pid_t _assuan_waitpid (assuan_context_t ctx, assuan_pid_t pid,
+                              int nowait, int *status, int options);
 int _assuan_socketpair (assuan_context_t ctx, int namespace, int style,
 			int protocol, assuan_fd_t filedes[2]);
 assuan_fd_t _assuan_socket (assuan_context_t ctx, int namespace,
diff --git a/src/assuan-pipe-connect.c b/src/assuan-pipe-connect.c
index 82a0aee..c116d1d 100644
--- a/src/assuan-pipe-connect.c
+++ b/src/assuan-pipe-connect.c
@@ -184,7 +184,7 @@ pipe_connect (assuan_context_t ctx,
   gpg_error_t rc;
   assuan_fd_t rp[2];
   assuan_fd_t wp[2];
-  pid_t pid;
+  assuan_pid_t pid;
   int res;
   struct at_pipe_fork atp;
   unsigned int spawn_flags;
@@ -244,7 +244,7 @@ pipe_connect (assuan_context_t ctx,
   ctx->accept_handler = NULL;
   ctx->inbound.fd  = rp[0];  /* Our inbound is read end of read pipe. */
   ctx->outbound.fd = wp[1];  /* Our outbound is write end of write pipe. */
-  ctx->pid = pid;
+  ctx->server_proc = pid;
 
   rc = initial_handshake (ctx);
   if (rc)
diff --git a/src/assuan-pipe-server.c b/src/assuan-pipe-server.c
index 5c960a6..4858ac5 100644
--- a/src/assuan-pipe-server.c
+++ b/src/assuan-pipe-server.c
@@ -65,8 +65,10 @@ is_valid_socket (const char *s)
 gpg_error_t
 assuan_init_pipe_server (assuan_context_t ctx, assuan_fd_t filedes[2])
 {
+#if !defined(HAVE_W32_SYSTEM)
   const char *s;
   unsigned long ul;
+#endif
   gpg_error_t rc;
   assuan_fd_t infd = ASSUAN_INVALID_FD;
   assuan_fd_t outfd = ASSUAN_INVALID_FD;
@@ -130,11 +132,13 @@ assuan_init_pipe_server (assuan_context_t ctx, assuan_fd_t filedes[2])
 #endif
   ctx->max_accepts = 1;
 
+#if !defined(HAVE_W32_SYSTEM)
   s = getenv ("_assuan_pipe_connect_pid");
   if (s && (ul=strtoul (s, NULL, 10)) && ul)
     ctx->pid = (pid_t)ul;
   else
     ctx->pid = (pid_t)-1;
+#endif
   ctx->accept_handler = NULL;
   ctx->finish_handler = _assuan_server_finish;
   ctx->inbound.fd = infd;
diff --git a/src/assuan-socket-server.c b/src/assuan-socket-server.c
index f5fe038..a260221 100644
--- a/src/assuan-socket-server.c
+++ b/src/assuan-socket-server.c
@@ -139,10 +139,12 @@ accept_connection_bottom (assuan_context_t ctx)
   }
 #endif
 
+#if !defined(HAVE_W32_SYSTEM)
   /* This overrides any already set PID if the function returns
      a valid one. */
   if (ctx->peercred_valid && ctx->peercred.pid != ASSUAN_INVALID_PID)
     ctx->pid = ctx->peercred.pid;
+#endif
 
   ctx->inbound.fd = fd;
   ctx->inbound.eof = 0;
diff --git a/src/assuan-socket.c b/src/assuan-socket.c
index f209457..cbd19fd 100644
--- a/src/assuan-socket.c
+++ b/src/assuan-socket.c
@@ -1432,6 +1432,8 @@ _assuan_sock_check_nonce (assuan_context_t ctx, assuan_fd_t fd,
          we ignore the values because they are not kernel controlled.  */
       if (do_readn (ctx, fd, buffer, 8))
         return -1;
+
+      memcpy (&ctx->process_id, buffer, 4);
       /* Send our credentials: We use the uid and gid we received but
          our own pid.  */
       n = getpid ();
diff --git a/src/assuan.c b/src/assuan.c
index 58498f7..550f057 100644
--- a/src/assuan.c
+++ b/src/assuan.c
@@ -179,6 +179,13 @@ assuan_new_ext (assuan_context_t *r_ctx, gpg_err_source_t err_source,
     ctx->outbound.fd = ASSUAN_INVALID_FD;
     ctx->listen_fd = ASSUAN_INVALID_FD;
 
+#if defined(HAVE_W32_SYSTEM)
+    ctx->process_id = -1;
+#else
+    ctx->pid = ASSUAN_INVALID_PID;
+#endif
+    ctx->server_proc = ASSUAN_INVALID_PID;
+
     *r_ctx = ctx;
 
     return TRACE_SUC1 ("ctx=%p", ctx);
diff --git a/src/assuan.h.in b/src/assuan.h.in
index 2f89f10..10917c9 100644
--- a/src/assuan.h.in
+++ b/src/assuan.h.in
@@ -255,7 +255,7 @@ struct assuan_system_hooks
   /* If NAME is NULL, don't exec, just fork.  FD_CHILD_LIST is
      modified to reflect the value of the FD in the peer process (on
      Windows).  */
-  int (*spawn) (assuan_context_t ctx, pid_t *r_pid, const char *name,
+  int (*spawn) (assuan_context_t ctx, assuan_pid_t *r_pid, const char *name,
 		const char **argv,
 		assuan_fd_t fd_in, assuan_fd_t fd_out,
 		assuan_fd_t *fd_child_list,
@@ -263,8 +263,8 @@ struct assuan_system_hooks
 		void *atforkvalue, unsigned int flags);
 
   /* If action is 0, like waitpid.  If action is 1, just release the PID?  */
-  pid_t (*waitpid) (assuan_context_t ctx, pid_t pid,
-		    int action, int *status, int options);
+  assuan_pid_t (*waitpid) (assuan_context_t ctx, assuan_pid_t pid,
+                           int action, int *status, int options);
   int (*socketpair) (assuan_context_t ctx, int _namespace, int style,
 		     int protocol, assuan_fd_t filedes[2]);
   assuan_fd_t (*socket) (assuan_context_t ctx, int _namespace,
@@ -529,7 +529,7 @@ void assuan_sock_set_system_hooks (assuan_system_hooks_t system_hooks);
 void __assuan_usleep (assuan_context_t ctx, unsigned int usec);
 int __assuan_pipe (assuan_context_t ctx, assuan_fd_t fd[2], int inherit_idx);
 int __assuan_close (assuan_context_t ctx, assuan_fd_t fd);
-int __assuan_spawn (assuan_context_t ctx, pid_t *r_pid, const char *name,
+int __assuan_spawn (assuan_context_t ctx, assuan_pid_t *r_pid, const char *name,
 		    const char **argv, assuan_fd_t fd_in, assuan_fd_t fd_out,
                     assuan_fd_t *fd_child_list,
                     void (*atfork) (void *opaque, int reserved),
@@ -548,8 +548,8 @@ int __assuan_recvmsg (assuan_context_t ctx, assuan_fd_t fd,
                       assuan_msghdr_t msg, int flags);
 int __assuan_sendmsg (assuan_context_t ctx, assuan_fd_t fd,
                       const assuan_msghdr_t msg, int flags);
-pid_t __assuan_waitpid (assuan_context_t ctx, pid_t pid,
-                        int nowait, int *status, int options);
+assuan_pid_t __assuan_waitpid (assuan_context_t ctx, assuan_pid_t pid,
+                               int nowait, int *status, int options);
 
 #ifdef ASSUAN_REALLY_REQUIRE_OLD_WAY_OF_SYSTEM_NPTH
 /* Standard system hooks for nPth.  */
@@ -578,9 +578,10 @@ pid_t __assuan_waitpid (assuan_context_t ctx, pid_t pid,
   { int res; (void) ctx; npth_unprotect();				\
     res = __assuan_sendmsg (ctx, fd, msg, flags);			\
     npth_protect(); return res; }					\
-  static pid_t _assuan_npth_waitpid (assuan_context_t ctx, pid_t pid,	\
-				     int nowait, int *status, int options) \
-  { pid_t res; (void) ctx; npth_unprotect();				\
+  static assuan_pid_t _assuan_npth_waitpid (assuan_context_t ctx,         \
+                                            assuan_pid_t pid, int nowait, \
+                                            int *status, int options)     \
+  { assuan_pid_t res; (void) ctx; npth_unprotect();			\
     res = __assuan_waitpid (ctx, pid, nowait, status, options);		\
     npth_protect(); return res; }					\
   static int _assuan_npth_connect (assuan_context_t ctx, assuan_fd_t sock, \
diff --git a/src/client.c b/src/client.c
index dcb2a1a..9235584 100644
--- a/src/client.c
+++ b/src/client.c
@@ -48,11 +48,11 @@ _assuan_client_finish (assuan_context_t ctx)
       _assuan_close (ctx, ctx->outbound.fd);
       ctx->outbound.fd = ASSUAN_INVALID_FD;
     }
-  if (ctx->pid != ASSUAN_INVALID_PID && ctx->pid)
+  if (ctx->server_proc != ASSUAN_INVALID_PID)
     {
       if (!ctx->flags.is_socket)
-	_assuan_waitpid (ctx, ctx->pid, ctx->flags.no_waitpid, NULL, 0);
-      ctx->pid = ASSUAN_INVALID_PID;
+	_assuan_waitpid (ctx, ctx->server_proc, ctx->flags.no_waitpid, NULL, 0);
+      ctx->server_proc = ASSUAN_INVALID_PID;
     }
 
   _assuan_uds_deinit (ctx);
diff --git a/src/context.c b/src/context.c
index 947df76..e41b60b 100644
--- a/src/context.c
+++ b/src/context.c
@@ -207,10 +207,30 @@ assuan_set_error (assuan_context_t ctx, gpg_error_t err, const char *text)
 pid_t
 assuan_get_pid (assuan_context_t ctx)
 {
+#if defined(HAVE_W32_SYSTEM)
+  TRACE1 (ctx, ASSUAN_LOG_CTX, "assuan_get_pid", ctx,
+	  "pid=%i", ctx ? ctx->process_id : -1);
+#else
   TRACE1 (ctx, ASSUAN_LOG_CTX, "assuan_get_pid", ctx,
 	  "pid=%i", ctx ? ctx->pid : -1);
+#endif
 
-  return (ctx && ctx->pid) ? ctx->pid : ASSUAN_INVALID_PID;
+  if (!ctx)
+    return ASSUAN_INVALID_PID;
+
+  if (ctx->flags.is_server)
+#if defined(HAVE_W32_SYSTEM)
+    return ctx->process_id;
+#else
+    return ctx->pid;
+#endif
+  else
+    /*
+     * This use case of getting internal process reference by the
+     * application should be fixed.  It's here, only for backward
+     * compatibility.
+     */
+    return ctx->server_proc;
 }
 
 
diff --git a/src/posix-types.inc.h b/src/posix-types.inc.h
index e3810b3..03f7007 100644
--- a/src/posix-types.inc.h
+++ b/src/posix-types.inc.h
@@ -21,4 +21,5 @@
 ## This file is included by the mkheader tool.  Lines starting with
 ## a double hash mark are not copied to the destination file.
 typedef struct msghdr *assuan_msghdr_t;
+typedef pid_t assuan_pid_t;
 ##EOF##
diff --git a/src/server.c b/src/server.c
index 5e96798..2ffbdbc 100644
--- a/src/server.c
+++ b/src/server.c
@@ -44,10 +44,12 @@ _assuan_server_finish (assuan_context_t ctx)
       _assuan_close (ctx, ctx->outbound.fd);
       ctx->outbound.fd = ASSUAN_INVALID_FD;
     }
-  if (ctx->pid != ASSUAN_INVALID_PID && ctx->pid)
-    {
-      ctx->pid = ASSUAN_INVALID_PID;
-    }
+
+#if defined(HAVE_W32_SYSTEM)
+  ctx->process_id = -1;
+#else
+  ctx->pid = ASSUAN_INVALID_PID;
+#endif
 
   _assuan_uds_deinit (ctx);
 
diff --git a/src/system-posix.c b/src/system-posix.c
index 0f7732a..5f3a7f2 100644
--- a/src/system-posix.c
+++ b/src/system-posix.c
@@ -266,7 +266,7 @@ get_max_fds (void)
 
 
 int
-__assuan_spawn (assuan_context_t ctx, pid_t *r_pid, const char *name,
+__assuan_spawn (assuan_context_t ctx, assuan_pid_t *r_pid, const char *name,
 		const char **argv,
 		assuan_fd_t fd_in, assuan_fd_t fd_out,
 		assuan_fd_t *fd_child_list,
@@ -394,8 +394,8 @@ __assuan_spawn (assuan_context_t ctx, pid_t *r_pid, const char *name,
 
 /* FIXME: Add some sort of waitpid function that covers GPGME and
    gpg-agent's use of assuan.  */
-pid_t
-__assuan_waitpid (assuan_context_t ctx, pid_t pid, int nowait,
+assuan_pid_t
+__assuan_waitpid (assuan_context_t ctx, assuan_pid_t pid, int nowait,
 		  int *status, int options)
 {
   /* We can't just release the PID, a waitpid is mandatory.  But
diff --git a/src/system-w32.c b/src/system-w32.c
index 28d3564..415cd63 100644
--- a/src/system-w32.c
+++ b/src/system-w32.c
@@ -451,7 +451,7 @@ build_w32_commandline (assuan_context_t ctx, const char * const *argv,
 
 
 int
-__assuan_spawn (assuan_context_t ctx, pid_t *r_pid, const char *name,
+__assuan_spawn (assuan_context_t ctx, assuan_pid_t *r_pid, const char *name,
 		const char **argv,
 		assuan_fd_t fd_in, assuan_fd_t fd_out,
 		assuan_fd_t *fd_child_list,
@@ -579,7 +579,7 @@ __assuan_spawn (assuan_context_t ctx, pid_t *r_pid, const char *name,
   /*                       pi.hProcess, pi.hThread, */
   /*                       (int) pi.dwProcessId, (int) pi.dwThreadId); */
 
-  *r_pid = (pid_t) pi.hProcess;
+  *r_pid = (assuan_pid_t) pi.hProcess;
 
   /* No need to modify peer process, as we don't change the handle
      names.  However this also means we are not safe, as we inherit
@@ -594,8 +594,8 @@ __assuan_spawn (assuan_context_t ctx, pid_t *r_pid, const char *name,
 
 /* FIXME: Add some sort of waitpid function that covers GPGME and
    gpg-agent's use of assuan.  */
-pid_t
-__assuan_waitpid (assuan_context_t ctx, pid_t pid, int nowait,
+assuan_pid_t
+__assuan_waitpid (assuan_context_t ctx, assuan_pid_t pid, int nowait,
 		  int *status, int options)
 {
   CloseHandle ((HANDLE) pid);
diff --git a/src/system.c b/src/system.c
index 6638ece..9807d9d 100644
--- a/src/system.c
+++ b/src/system.c
@@ -391,7 +391,7 @@ _assuan_sendmsg (assuan_context_t ctx, assuan_fd_t fd, assuan_msghdr_t msg,
    FD_CHILD_LIST as given (no remapping), which must be inheritable.
    On Unix, call ATFORK with ATFORKVALUE after fork and before exec.  */
 int
-_assuan_spawn (assuan_context_t ctx, pid_t *r_pid, const char *name,
+_assuan_spawn (assuan_context_t ctx, assuan_pid_t *r_pid, const char *name,
 	       const char **argv,
 	       assuan_fd_t fd_in, assuan_fd_t fd_out,
 	       assuan_fd_t *fd_child_list,
@@ -448,12 +448,12 @@ _assuan_spawn (assuan_context_t ctx, pid_t *r_pid, const char *name,
 
 /* FIXME: Add some sort of waitpid function that covers GPGME and
    gpg-agent's use of assuan.  */
-pid_t
-_assuan_waitpid (assuan_context_t ctx, pid_t pid, int action,
+assuan_pid_t
+_assuan_waitpid (assuan_context_t ctx, assuan_pid_t pid, int action,
 		 int *status, int options)
 {
 #if DEBUG_SYSIO
-  pid_t res;
+  assuan_pid_t res;
   TRACE_BEG4 (ctx, ASSUAN_LOG_SYSIO, "_assuan_waitpid", ctx,
 	      "pid=%i, action=%i, status=%p, options=%i",
 	      pid, action, status, options);
@@ -471,7 +471,7 @@ _assuan_waitpid (assuan_context_t ctx, pid_t pid, int action,
     return (ctx->system.waitpid) (ctx, pid, action, status, options);
   else
     {
-      pid_t res;
+      assuan_pid_t res;
       _assuan_pre_syscall ();
       res = __assuan_waitpid (ctx, pid, action, status, options);
       _assuan_post_syscall ();
diff --git a/src/w32-types.inc.h b/src/w32-types.inc.h
index 552ffd0..b60cd2c 100644
--- a/src/w32-types.inc.h
+++ b/src/w32-types.inc.h
@@ -32,4 +32,10 @@ typedef void *assuan_msghdr_t;
 # endif
 #endif
 
+#ifdef _WIN64
+ typedef unsigned long long assuan_pid_t;
+#else
+ typedef unsigned long assuan_pid_t;
+#endif
+
 ##EOF##
-- 
cgit v1.2.1