openssh unix dev - Oct 2021 - [PATCH] Support ambient capability vector in Linux PAM

Hello everyone,

originating from this discussion

   https://github.com/shadow-maint/shadow/pull/408

and the work recently done in Linux libcap

   https://bugzilla.kernel.org/show_bug.cgi?id=214377#c3

I would like to propose this patch to support the ambient
capability vector in Linux PAM + libcap-2.58+.

Background for this is that the setuid() systemcall drops
all ambient capabilities for obvious security reasons. So,
to support the ambient vector by using pam_cap.so in any
login procedure, capabilites have to be set _after_ the
last call to setuid(), which leaves the PAM cleanup code
path as the only option.

Calling pam_end() with PAM_DATA_SILENT is documented in

http://www.linux-pam.org/Linux-PAM-html/adg-interface-by-app-expected.html#adg-pam_end

Concerned about portability I am unsure if testing for
PAM_DATA_SILENT is sufficient or if __LINUX_PAM__ should be
preferred.

Cheers,
	Bj?rn
--
diff --git a/auth-pam.c b/auth-pam.c
index 29034e40..a6544133 100644
--- a/auth-pam.c
+++ b/auth-pam.c
@@ -683,6 +683,22 @@ sshpam_cleanup(void)
         sshpam_handle = NULL;
  }

+void
+sshpam_child_cleanup(void)
+{
+       if (sshpam_handle == NULL)
+               return;
+       debug("PAM: child cleanup");
+#ifdef PAM_DATA_SILENT
+       /*
+        * Child codepath cleanup for Linux PAM.
+        * Support Linux libcap-2.58+ pam_cap.so ambient vector.
+        */
+       pam_end(sshpam_handle, PAM_SUCCESS | PAM_DATA_SILENT);
+#endif
+       sshpam_handle = NULL;
+}
+
  static int
  sshpam_init(struct ssh *ssh, Authctxt *authctxt)
  {
diff --git a/auth-pam.h b/auth-pam.h
index 9fcea270..049858a5 100644
--- a/auth-pam.h
+++ b/auth-pam.h
@@ -39,6 +39,7 @@ char ** fetch_pam_child_environment(void);
  void free_pam_environment(char **);
  void sshpam_thread_cleanup(void);
  void sshpam_cleanup(void);
+void sshpam_child_cleanup(void);
  int sshpam_auth_passwd(Authctxt *, const char *);
  int sshpam_get_maxtries_reached(void);
  void sshpam_set_maxtries_reached(int);
diff --git a/session.c b/session.c
index 5f423f9f..19a28527 100644
--- a/session.c
+++ b/session.c
@@ -1589,6 +1589,10 @@ do_child(struct ssh *ssh, Session *s, const char 
*command)
          */
         child_close_fds(ssh);

+#ifdef USE_PAM
+       sshpam_child_cleanup();
+#endif
+
         /*
          * Must take new environment into use so that .ssh/rc,
          * /etc/ssh/sshrc and xauth are run in the proper environment.

On Fri, 1 Oct 2021, Bj?rn Fischer wrote:
> Hello everyone,
> 
> originating from this discussion
> 
>    https://github.com/shadow-maint/shadow/pull/408
> 
> and the work recently done in Linux libcap
> 
>    https://bugzilla.kernel.org/show_bug.cgi?id=214377#c3
> 
> I would like to propose this patch to support the ambient
> capability vector in Linux PAM + libcap-2.58+.
> 
> Background for this is that the setuid() systemcall drops
> all ambient capabilities for obvious security reasons. So,
> to support the ambient vector by using pam_cap.so in any
> login procedure, capabilites have to be set _after_ the
> last call to setuid(), which leaves the PAM cleanup code
> path as the only option.
> 
> Calling pam_end() with PAM_DATA_SILENT is documented in
> 
>
http://www.linux-pam.org/Linux-PAM-html/adg-interface-by-app-expected.html#adg-pam_end
> 
> Concerned about portability I am unsure if testing for
> PAM_DATA_SILENT is sufficient or if __LINUX_PAM__ should be
> preferred.
I guess my only concern is that this would cause pam_end() to
potentially be called multiple times, once in the parent process
(without PAM_DATA_SILENT) and zero to many times in child session
processes. 

E.g. a forwarding-only session might have no child session process,
whereas a multiplexed connection might have many child processes,
all of which will share the same pam_handle.

How will PAM cope with this?

-d