public inbox for cygwin-apps@cygwin.com
 help / color / mirror / Atom feed
* openssh AuthorizedKeysFile
@ 2018-04-06 20:37 Achim Gratz
  2018-04-09  7:52 ` Corinna Vinschen
  0 siblings, 1 reply; 5+ messages in thread
From: Achim Gratz @ 2018-04-06 20:37 UTC (permalink / raw)
  To: cygwin-apps


I've got a new server for Cygwin @work and wanted to get the sshd to run
with StrictMode on (it's been off on the old server).  Long story short,
some accounts used for administrative tasks are contrained so that I
need to store the authorized_keys file directly on the server, so I
added /etc/ssh/%u/authorized_keys in front of the default
.ssh/authorized_keys.  Unfortunately that only works if the same
administrative account has been used to install Cygwin itself, lest sshd
declares the directory /etc/ssh unsafe (or use StrictMode=no).  I found
this patch that seems to address exactly the same situation:

https://github.com/pierresouchay/cygwin_patches/blob/master/openssh.patch

The code has since been refactored and a similar change would need to be
applied elsewhere.  Interestingly enough there is some special handling
to _not_ check all the leading path components for the home directory
(otherwise it wouldn't work at all).  In my reading of the refactored
code it seems that the same effect could be achieved by defining
PLATFORM_SYS_DIR_UID appropriately (although I would prefer if that was
configurable somewhere in a file).  But it seems that for Cygwin that
symbol doesn't get defined at all?


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Wavetables for the Waldorf Blofeld:
http://Synth.Stromeko.net/Downloads.html#BlofeldUserWavetables

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: openssh AuthorizedKeysFile
  2018-04-06 20:37 openssh AuthorizedKeysFile Achim Gratz
@ 2018-04-09  7:52 ` Corinna Vinschen
  2018-04-09 10:59   ` Corinna Vinschen
  2018-04-09 17:32   ` Achim Gratz
  0 siblings, 2 replies; 5+ messages in thread
From: Corinna Vinschen @ 2018-04-09  7:52 UTC (permalink / raw)
  To: cygwin-apps

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

On Apr  6 22:37, Achim Gratz wrote:
> 
> I've got a new server for Cygwin @work and wanted to get the sshd to run
> with StrictMode on (it's been off on the old server).  Long story short,
> some accounts used for administrative tasks are contrained so that I
> need to store the authorized_keys file directly on the server, so I
> added /etc/ssh/%u/authorized_keys in front of the default
> .ssh/authorized_keys.  Unfortunately that only works if the same
> administrative account has been used to install Cygwin itself, lest sshd
> declares the directory /etc/ssh unsafe (or use StrictMode=no).

What exactly doesn't work?  If it's the ownership of the dirs and
files, chown will do the trick, no?

> I found
> this patch that seems to address exactly the same situation:
> 
> https://github.com/pierresouchay/cygwin_patches/blob/master/openssh.patch
> 
> The code has since been refactored and a similar change would need to be
> applied elsewhere.  Interestingly enough there is some special handling
> to _not_ check all the leading path components for the home directory
> (otherwise it wouldn't work at all).  In my reading of the refactored
> code it seems that the same effect could be achieved by defining
> PLATFORM_SYS_DIR_UID appropriately (although I would prefer if that was
> configurable somewhere in a file).  But it seems that for Cygwin that
> symbol doesn't get defined at all?

No, so far it's a special feature for AIX and HP/UX only.  On these
platforms certain dirs and files are owned by the bin user with uid 2.

The problem on Cygwin is that we don't have a fixed uid owning the
entire system paths.  It always depends on the account used to create
the system dirs, which can vary from installation to installation.  What
you could do is adding a passwd entry with uid 0 for the account
installing Cygwin and make sure that the files are always owned by this
account (chown).

The only other way to fix this would be to define PLATFORM_SYS_DIR_UID
to be a function call on Cygwin, which checks the account for... what?
To be an admin account?  That sounds quite relaxed, but I don't see
any other way.

Something like this in configure.ac:

  AC_DEFINE([PLATFORM_SYS_DIR_UID], cygwin_valid_sys_dir_owner(), [System dirs owned by admin account])

and a bit of extra code in openbsd-compat/bsd-cygwin_util.c along the
lines of:

  /* return uid if uid is a valid system dir-owning uid */
  cygwin_valid_sys_dir_owner (uid_t uid)
  {
    struct pw = getpwuid (uid);
    getgrouplist (pw->pw_name, pw->pw_gid, grplist, &ngroups);
    if (544 in grplist)
      return 1
    return 0;
  }


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: openssh AuthorizedKeysFile
  2018-04-09  7:52 ` Corinna Vinschen
@ 2018-04-09 10:59   ` Corinna Vinschen
  2018-04-09 14:09     ` Corinna Vinschen
  2018-04-09 17:32   ` Achim Gratz
  1 sibling, 1 reply; 5+ messages in thread
From: Corinna Vinschen @ 2018-04-09 10:59 UTC (permalink / raw)
  To: cygwin-apps

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

On Apr  9 09:52, Corinna Vinschen wrote:
> On Apr  6 22:37, Achim Gratz wrote:
> > 
> > I've got a new server for Cygwin @work and wanted to get the sshd to run
> > with StrictMode on (it's been off on the old server).  Long story short,
> > some accounts used for administrative tasks are contrained so that I
> > need to store the authorized_keys file directly on the server, so I
> > added /etc/ssh/%u/authorized_keys in front of the default
> > .ssh/authorized_keys.  Unfortunately that only works if the same
> > administrative account has been used to install Cygwin itself, lest sshd
> > declares the directory /etc/ssh unsafe (or use StrictMode=no).
> 
> What exactly doesn't work?  If it's the ownership of the dirs and
> files, chown will do the trick, no?
> 
> >   In my reading of the refactored
> > code it seems that the same effect could be achieved by defining
> > PLATFORM_SYS_DIR_UID appropriately (although I would prefer if that was
> > configurable somewhere in a file).  But it seems that for Cygwin that
> > symbol doesn't get defined at all?
> 
> No, so far it's a special feature for AIX and HP/UX only.  On these
> platforms certain dirs and files are owned by the bin user with uid 2.
> 
> The problem on Cygwin is that we don't have a fixed uid owning the
> entire system paths.  It always depends on the account used to create
> the system dirs, which can vary from installation to installation.  What
> you could do is adding a passwd entry with uid 0 for the account
> installing Cygwin and make sure that the files are always owned by this
> account (chown).
> 
> The only other way to fix this would be to define PLATFORM_SYS_DIR_UID
> to be a function call on Cygwin, which checks the account for... what?
> To be an admin account?  That sounds quite relaxed, but I don't see
> any other way.
> 
> Something like this [...]

Please try if this patch to openssh will do the trick for you.  I only
tested that it builds, but not if it works as desired.

From 6b493f7e9f5ab7c64fa56c84ea727d3d06a12c0f Mon Sep 17 00:00:00 2001
From: Corinna Vinschen <vinschen@redhat.com>
Date: Mon, 9 Apr 2018 12:56:31 +0200
Subject: [PATCH] cygwin: add function call to provide OS-specific
 PLATFORM_SYS_DIR_UID

---
 configure.ac                     |  1 +
 openbsd-compat/bsd-cygwin_util.c | 25 +++++++++++++++++++++++++
 openbsd-compat/bsd-cygwin_util.h |  1 +
 3 files changed, 27 insertions(+)

diff --git a/configure.ac b/configure.ac
index 663062bef142..a5f68c367c92 100644
--- a/configure.ac
+++ b/configure.ac
@@ -626,6 +626,7 @@ case "$host" in
 		file descriptor passing])
 	AC_DEFINE([SSH_IOBUFSZ], [65535], [Windows is sensitive to read buffer size])
 	AC_DEFINE([FILESYSTEM_NO_BACKSLASH], [1], [File names may not contain backslash characters])
+	AC_DEFINE([PLATFORM_SYS_DIR_UID], cygwin_valid_sys_dir_owner(uid), [System dirs owned by admin account])
 	# Cygwin defines optargs, optargs as declspec(dllimport) for historical
 	# reasons which cause compile warnings, so we disable those warnings.
 	OSSH_CHECK_CFLAG_COMPILE([-Wno-attributes])
diff --git a/openbsd-compat/bsd-cygwin_util.c b/openbsd-compat/bsd-cygwin_util.c
index 398a5f617af5..0f5bb1a4448a 100644
--- a/openbsd-compat/bsd-cygwin_util.c
+++ b/openbsd-compat/bsd-cygwin_util.c
@@ -33,6 +33,7 @@
 #ifdef HAVE_CYGWIN
 
 #include <sys/types.h>
+#include <grp.h>
 #include <fcntl.h>
 #include <string.h>
 #include <unistd.h>
@@ -116,4 +117,28 @@ free_windows_environment(char **p)
 	free(p);
 }
 
+/* Check if current account is administrative account (aka member of
+ * group 544 "Administrators")
+ */
+uid_t
+cygwin_valid_sys_dir_owner(uid_t uid)
+{
+	int ngrps = 0;
+	gid_t *grps = NULL;
+	struct passwd *pw;
+
+	pw = getpwuid(uid);
+	if (!pw)
+	  return 0;
+
+	if (getgrouplist(pw->pw_name, pw->pw_gid, grps, &ngrps) < 0) {
+		grps = (gid_t *) alloca(sizeof (gid_t) * ngrps);
+		if (getgrouplist(pw->pw_name, pw->pw_gid, grps, &ngrps) < 0)
+			return 0;
+		while (--ngrps >= 0)
+			if (grps[ngrps] == 544)
+			  return uid;
+	}
+	return 0;
+}
 #endif /* HAVE_CYGWIN */
diff --git a/openbsd-compat/bsd-cygwin_util.h b/openbsd-compat/bsd-cygwin_util.h
index 9cef694b9a7c..e2d53f47defe 100644
--- a/openbsd-compat/bsd-cygwin_util.h
+++ b/openbsd-compat/bsd-cygwin_util.h
@@ -44,6 +44,7 @@ typedef void *HANDLE;
    windows headers, so we have to define them here explicitely. */
 extern HANDLE cygwin_logon_user (const struct passwd *, const char *);
 extern void cygwin_set_impersonation_token (const HANDLE);
+extern uid_t cygwin_valid_sys_dir_owner(uid_t uid);
 
 #include <sys/cygwin.h>
 #include <io.h>
-- 
2.14.3


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: openssh AuthorizedKeysFile
  2018-04-09 10:59   ` Corinna Vinschen
@ 2018-04-09 14:09     ` Corinna Vinschen
  0 siblings, 0 replies; 5+ messages in thread
From: Corinna Vinschen @ 2018-04-09 14:09 UTC (permalink / raw)
  To: cygwin-apps

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

On Apr  9 12:59, Corinna Vinschen wrote:
> On Apr  9 09:52, Corinna Vinschen wrote:
> > The problem on Cygwin is that we don't have a fixed uid owning the
> > entire system paths.  It always depends on the account used to create
> > the system dirs, which can vary from installation to installation.  What
> > you could do is adding a passwd entry with uid 0 for the account
> > installing Cygwin and make sure that the files are always owned by this
> > account (chown).
> > 
> > The only other way to fix this would be to define PLATFORM_SYS_DIR_UID
> > to be a function call on Cygwin, which checks the account for... what?
> > To be an admin account?  That sounds quite relaxed, but I don't see
> > any other way.
> > 
> > Something like this [...]
> 
> Please try if this patch to openssh will do the trick for you.  I only
> tested that it builds, but not if it works as desired.

On second thought...

This is dangerous.  Every path owned by any member of the admins group
is treated as safe path.  I.e., even the home dir of any admin user
is safe.

That doesn't sound desirable...


Corinna

> 
> From 6b493f7e9f5ab7c64fa56c84ea727d3d06a12c0f Mon Sep 17 00:00:00 2001
> From: Corinna Vinschen <vinschen@redhat.com>
> Date: Mon, 9 Apr 2018 12:56:31 +0200
> Subject: [PATCH] cygwin: add function call to provide OS-specific
>  PLATFORM_SYS_DIR_UID
> 
> ---
>  configure.ac                     |  1 +
>  openbsd-compat/bsd-cygwin_util.c | 25 +++++++++++++++++++++++++
>  openbsd-compat/bsd-cygwin_util.h |  1 +
>  3 files changed, 27 insertions(+)
> 
> diff --git a/configure.ac b/configure.ac
> index 663062bef142..a5f68c367c92 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -626,6 +626,7 @@ case "$host" in
>  		file descriptor passing])
>  	AC_DEFINE([SSH_IOBUFSZ], [65535], [Windows is sensitive to read buffer size])
>  	AC_DEFINE([FILESYSTEM_NO_BACKSLASH], [1], [File names may not contain backslash characters])
> +	AC_DEFINE([PLATFORM_SYS_DIR_UID], cygwin_valid_sys_dir_owner(uid), [System dirs owned by admin account])
>  	# Cygwin defines optargs, optargs as declspec(dllimport) for historical
>  	# reasons which cause compile warnings, so we disable those warnings.
>  	OSSH_CHECK_CFLAG_COMPILE([-Wno-attributes])
> diff --git a/openbsd-compat/bsd-cygwin_util.c b/openbsd-compat/bsd-cygwin_util.c
> index 398a5f617af5..0f5bb1a4448a 100644
> --- a/openbsd-compat/bsd-cygwin_util.c
> +++ b/openbsd-compat/bsd-cygwin_util.c
> @@ -33,6 +33,7 @@
>  #ifdef HAVE_CYGWIN
>  
>  #include <sys/types.h>
> +#include <grp.h>
>  #include <fcntl.h>
>  #include <string.h>
>  #include <unistd.h>
> @@ -116,4 +117,28 @@ free_windows_environment(char **p)
>  	free(p);
>  }
>  
> +/* Check if current account is administrative account (aka member of
> + * group 544 "Administrators")
> + */
> +uid_t
> +cygwin_valid_sys_dir_owner(uid_t uid)
> +{
> +	int ngrps = 0;
> +	gid_t *grps = NULL;
> +	struct passwd *pw;
> +
> +	pw = getpwuid(uid);
> +	if (!pw)
> +	  return 0;
> +
> +	if (getgrouplist(pw->pw_name, pw->pw_gid, grps, &ngrps) < 0) {
> +		grps = (gid_t *) alloca(sizeof (gid_t) * ngrps);
> +		if (getgrouplist(pw->pw_name, pw->pw_gid, grps, &ngrps) < 0)
> +			return 0;
> +		while (--ngrps >= 0)
> +			if (grps[ngrps] == 544)
> +			  return uid;
> +	}
> +	return 0;
> +}
>  #endif /* HAVE_CYGWIN */
> diff --git a/openbsd-compat/bsd-cygwin_util.h b/openbsd-compat/bsd-cygwin_util.h
> index 9cef694b9a7c..e2d53f47defe 100644
> --- a/openbsd-compat/bsd-cygwin_util.h
> +++ b/openbsd-compat/bsd-cygwin_util.h
> @@ -44,6 +44,7 @@ typedef void *HANDLE;
>     windows headers, so we have to define them here explicitely. */
>  extern HANDLE cygwin_logon_user (const struct passwd *, const char *);
>  extern void cygwin_set_impersonation_token (const HANDLE);
> +extern uid_t cygwin_valid_sys_dir_owner(uid_t uid);
>  
>  #include <sys/cygwin.h>
>  #include <io.h>
> -- 
> 2.14.3
> 
> 
> Corinna
> 
> -- 
> Corinna Vinschen                  Please, send mails regarding Cygwin to
> Cygwin Maintainer                 cygwin AT cygwin DOT com
> Red Hat



-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: openssh AuthorizedKeysFile
  2018-04-09  7:52 ` Corinna Vinschen
  2018-04-09 10:59   ` Corinna Vinschen
@ 2018-04-09 17:32   ` Achim Gratz
  1 sibling, 0 replies; 5+ messages in thread
From: Achim Gratz @ 2018-04-09 17:32 UTC (permalink / raw)
  To: cygwin-apps

Corinna Vinschen writes:
> What exactly doesn't work?  If it's the ownership of the dirs and
> files, chown will do the trick, no?

Well, the whole Cygwin directory is owned by an administrative domain
account, so unless I can tell sshd that this account is the equivalent
of root it will always complain about some directory up the chain.

> No, so far it's a special feature for AIX and HP/UX only.  On these
> platforms certain dirs and files are owned by the bin user with uid 2.

I've figured the same reading the comments.

> The problem on Cygwin is that we don't have a fixed uid owning the
> entire system paths.

One system path is enough, so I'd rather have a way to configure some
directory tree to be treated like the user's home (i.e. check the
directory permissions only up to some certain point).

> It always depends on the account used to create the system dirs, which
> can vary from installation to installation.  What you could do is
> adding a passwd entry with uid 0 for the account installing Cygwin and
> make sure that the files are always owned by this account (chown).

I'd rather avoid messing with the uid since I'd want that to show up
correctly in other places if possible.  But that would at least be
something I can try without recompiling sshd or anything else.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Wavetables for the Terratec KOMPLEXER:
http://Synth.Stromeko.net/Downloads.html#KomplexerWaves

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2018-04-09 17:32 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-06 20:37 openssh AuthorizedKeysFile Achim Gratz
2018-04-09  7:52 ` Corinna Vinschen
2018-04-09 10:59   ` Corinna Vinschen
2018-04-09 14:09     ` Corinna Vinschen
2018-04-09 17:32   ` Achim Gratz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).