Re: Ignore /tmp/.Xauth* in daily

2021-03-06 Thread Vadim Zhukov
сб, 6 мар. 2021 г. в 23:14, Matthieu Herrb :
>
> On Sat, Mar 06, 2021 at 09:52:58PM +0300, Vadim Zhukov wrote:
> > сб, 6 мар. 2021 г. в 21:30, Theo de Raadt :
> > >
> > > Matthieu Herrb  wrote:
> > >
> > > > Linux, systemd and XDG have inventend this /run/user/$uid tmpfs that
> > > > is created automagically and they use that in place of /tmp for
> > > > volatile things that don't beloing to $HOME, but this is not a can of
> > > > worms I want to open now.
> > >
> > > Awesome, another directory to drop stuff and run a filesystem out of space
> > > with unclear consequences...
> > >
> > > This does not fit with our direction either.
> >
> > So this code appeared in X11R4. There was no VCS repo, I suppose, so no 
> > history.
> >
> > There are basically four cases why xdm may fail to create ~/.Xauthority:
> >
> > a) home directory doesn't exist
> > b) home directory is non-writeable due to permissions
> > c) /home is full
> > d) /home is on NFS and there are locking/network issues.
> >
> > I'm not sure if (a) is a valid case. (b) is a variant of my case, as I
> > said, I can live without this feature. In the case of (c) users
> > (non-admins) won't be able to do something anyway. Can't speak for NFS
> > (I've quit the job where /home on NFS has been set up a few years ago)
> > so no opinion on (d).
> >
>
> I think 4 his not an issue anymore.the locking mecanism used by xauth
> is working with all current NFS implementations (including
> OpenBSD's).
>
> Here is a patch to remve the backup authorization file. Unfortunatly
> there is no simple way to display an explicit error message. One will
> need to check the xenodm.log file.
>
> Xsession can be patched too to remove the fallback to /tmp/xes- log
> file if ~/.xsession-errors cannot be writen. This will be a separate
> diff.
>
> Index: include/dm.h
> ===
> RCS file: /cvs/OpenBSD/xenocara/app/xenodm/include/dm.h,v
> retrieving revision 1.15
> diff -u -p -u -r1.15 dm.h
> --- include/dm.h10 Jan 2021 09:18:30 -  1.15
> +++ include/dm.h6 Mar 2021 17:53:44 -
> @@ -122,7 +122,6 @@ struct display {
> char**authNames;/* authorization protocol names */
> unsigned short  *authNameLens;  /* authorization protocol name lens */
> char*clientAuthFile;/* client specified auth file */
> -   char*userAuthDir;   /* backup directory for tickets */
> int authComplain;   /* complain when no auth for XDMCP */
>
> /* information potentially derived from resources */
> Index: man/xenodm.man
> ===
> RCS file: /cvs/OpenBSD/xenocara/app/xenodm/man/xenodm.man,v
> retrieving revision 1.11
> diff -u -p -u -r1.11 xenodm.man
> --- man/xenodm.man  15 Aug 2019 16:23:33 -  1.11
> +++ man/xenodm.man  6 Mar 2021 17:53:44 -
> @@ -582,18 +582,6 @@ to occur, during which time the new auth
>  The default is
>  .Cm false ,
>  which will work for all MIT servers.
> -.It Ic DisplayManager. Ns Ar DISPLAY Ns Ic .userAuthDir
> -When
> -.Nm
> -is unable to write to the usual user authorization file
> -.Pq Pa $HOME/.Xauthority ,
> -it creates a unique file name in this directory and points the environment
> -variable
> -.Ev XAUTHORITY
> -at the created file.
> -It uses
> -.Pa /tmp
> -by default.
>  .El
>  .Sh CONFIGURATION FILE
>  First, the
> Index: xenodm/auth.c
> ===
> RCS file: /cvs/OpenBSD/xenocara/app/xenodm/xenodm/auth.c,v
> retrieving revision 1.15
> diff -u -p -u -r1.15 auth.c
> --- xenodm/auth.c   1 Jan 2021 18:09:07 -   1.15
> +++ xenodm/auth.c   6 Mar 2021 17:53:44 -
> @@ -752,7 +752,7 @@ void
>  SetUserAuthorization (struct display *d, struct verify_info *verify)
>  {
>  FILE   *old = NULL, *new;
> -char   home_name[1024], backup_name[1024], new_name[1024];
> +char   home_name[1024], new_name[1024];
>  char   *name = NULL;
>  char   *home;
>  char   *envname = NULL;
> @@ -762,7 +762,6 @@ SetUserAuthorization (struct display *d,
>  struct statstatb;
>  inti;
>  intmagicCookie;
> -intfd;
>
>  Debug ("SetUserAuthorization\n");
>  auths = d->authorizations;
> @@ -793,45 +792,10 @@ SetUserAuthorization (struct display *d,
> }
> }
> if (lockStatus != LOCK_SUCCESS) {
> -   snprintf (backup_name, sizeof(backup_name),
> - "%s/.XauthXX", d->userAuthDir);
> -   fd = mkstemp (backup_name);
> -   if (fd >= 0) {
> -   old = fdopen (fd, "r");
> -   if (old == NULL)
> -   (void) close(fd);
> -   }
> -
> -   if (old != NULL)
> -   {
> -   lockStatus = XauLockAuth (backup_name, 1, 2, 

Re: Ignore /tmp/.Xauth* in daily

2021-03-06 Thread Matthieu Herrb
On Sat, Mar 06, 2021 at 09:52:58PM +0300, Vadim Zhukov wrote:
> сб, 6 мар. 2021 г. в 21:30, Theo de Raadt :
> >
> > Matthieu Herrb  wrote:
> >
> > > Linux, systemd and XDG have inventend this /run/user/$uid tmpfs that
> > > is created automagically and they use that in place of /tmp for
> > > volatile things that don't beloing to $HOME, but this is not a can of
> > > worms I want to open now.
> >
> > Awesome, another directory to drop stuff and run a filesystem out of space
> > with unclear consequences...
> >
> > This does not fit with our direction either.
> 
> So this code appeared in X11R4. There was no VCS repo, I suppose, so no 
> history.
> 
> There are basically four cases why xdm may fail to create ~/.Xauthority:
> 
> a) home directory doesn't exist
> b) home directory is non-writeable due to permissions
> c) /home is full
> d) /home is on NFS and there are locking/network issues.
> 
> I'm not sure if (a) is a valid case. (b) is a variant of my case, as I
> said, I can live without this feature. In the case of (c) users
> (non-admins) won't be able to do something anyway. Can't speak for NFS
> (I've quit the job where /home on NFS has been set up a few years ago)
> so no opinion on (d).
> 

I think 4 his not an issue anymore.the locking mecanism used by xauth
is working with all current NFS implementations (including
OpenBSD's).

Here is a patch to remve the backup authorization file. Unfortunatly
there is no simple way to display an explicit error message. One will
need to check the xenodm.log file.

Xsession can be patched too to remove the fallback to /tmp/xes- log
file if ~/.xsession-errors cannot be writen. This will be a separate
diff.

Index: include/dm.h
===
RCS file: /cvs/OpenBSD/xenocara/app/xenodm/include/dm.h,v
retrieving revision 1.15
diff -u -p -u -r1.15 dm.h
--- include/dm.h10 Jan 2021 09:18:30 -  1.15
+++ include/dm.h6 Mar 2021 17:53:44 -
@@ -122,7 +122,6 @@ struct display {
char**authNames;/* authorization protocol names */
unsigned short  *authNameLens;  /* authorization protocol name lens */
char*clientAuthFile;/* client specified auth file */
-   char*userAuthDir;   /* backup directory for tickets */
int authComplain;   /* complain when no auth for XDMCP */
 
/* information potentially derived from resources */
Index: man/xenodm.man
===
RCS file: /cvs/OpenBSD/xenocara/app/xenodm/man/xenodm.man,v
retrieving revision 1.11
diff -u -p -u -r1.11 xenodm.man
--- man/xenodm.man  15 Aug 2019 16:23:33 -  1.11
+++ man/xenodm.man  6 Mar 2021 17:53:44 -
@@ -582,18 +582,6 @@ to occur, during which time the new auth
 The default is
 .Cm false ,
 which will work for all MIT servers.
-.It Ic DisplayManager. Ns Ar DISPLAY Ns Ic .userAuthDir
-When
-.Nm
-is unable to write to the usual user authorization file
-.Pq Pa $HOME/.Xauthority ,
-it creates a unique file name in this directory and points the environment
-variable
-.Ev XAUTHORITY
-at the created file.
-It uses
-.Pa /tmp
-by default.
 .El
 .Sh CONFIGURATION FILE
 First, the
Index: xenodm/auth.c
===
RCS file: /cvs/OpenBSD/xenocara/app/xenodm/xenodm/auth.c,v
retrieving revision 1.15
diff -u -p -u -r1.15 auth.c
--- xenodm/auth.c   1 Jan 2021 18:09:07 -   1.15
+++ xenodm/auth.c   6 Mar 2021 17:53:44 -
@@ -752,7 +752,7 @@ void
 SetUserAuthorization (struct display *d, struct verify_info *verify)
 {
 FILE   *old = NULL, *new;
-char   home_name[1024], backup_name[1024], new_name[1024];
+char   home_name[1024], new_name[1024];
 char   *name = NULL;
 char   *home;
 char   *envname = NULL;
@@ -762,7 +762,6 @@ SetUserAuthorization (struct display *d,
 struct statstatb;
 inti;
 intmagicCookie;
-intfd;
 
 Debug ("SetUserAuthorization\n");
 auths = d->authorizations;
@@ -793,45 +792,10 @@ SetUserAuthorization (struct display *d,
}
}
if (lockStatus != LOCK_SUCCESS) {
-   snprintf (backup_name, sizeof(backup_name),
- "%s/.XauthXX", d->userAuthDir);
-   fd = mkstemp (backup_name);
-   if (fd >= 0) {
-   old = fdopen (fd, "r");
-   if (old == NULL)
-   (void) close(fd);
-   }
-
-   if (old != NULL)
-   {
-   lockStatus = XauLockAuth (backup_name, 1, 2, 10);
-   Debug ("backup lock is %d\n", lockStatus);
-   if (lockStatus == LOCK_SUCCESS) {
-   if (openFiles (backup_name, new_name, sizeof(new_name),
-   , )
-   && (old != NULL) && (new != 

Re: Ignore /tmp/.Xauth* in daily

2021-03-06 Thread Vadim Zhukov
сб, 6 мар. 2021 г. в 21:30, Theo de Raadt :
>
> Matthieu Herrb  wrote:
>
> > Linux, systemd and XDG have inventend this /run/user/$uid tmpfs that
> > is created automagically and they use that in place of /tmp for
> > volatile things that don't beloing to $HOME, but this is not a can of
> > worms I want to open now.
>
> Awesome, another directory to drop stuff and run a filesystem out of space
> with unclear consequences...
>
> This does not fit with our direction either.

So this code appeared in X11R4. There was no VCS repo, I suppose, so no history.

There are basically four cases why xdm may fail to create ~/.Xauthority:

a) home directory doesn't exist
b) home directory is non-writeable due to permissions
c) /home is full
d) /home is on NFS and there are locking/network issues.

I'm not sure if (a) is a valid case. (b) is a variant of my case, as I
said, I can live without this feature. In the case of (c) users
(non-admins) won't be able to do something anyway. Can't speak for NFS
(I've quit the job where /home on NFS has been set up a few years ago)
so no opinion on (d).

-- 
  WBR,
  Vadim Zhukov



Re: Ignore /tmp/.Xauth* in daily

2021-03-06 Thread Theo de Raadt
Matthieu Herrb  wrote:

> Linux, systemd and XDG have inventend this /run/user/$uid tmpfs that
> is created automagically and they use that in place of /tmp for
> volatile things that don't beloing to $HOME, but this is not a can of
> worms I want to open now.

Awesome, another directory to drop stuff and run a filesystem out of space
with unclear consequences...

This does not fit with our direction either.



Re: Ignore /tmp/.Xauth* in daily

2021-03-06 Thread Matthieu Herrb
On Sat, Mar 06, 2021 at 08:50:57PM +0300, Vadim Zhukov wrote:
> 
> Well, I do... nothing? I can show my .xsession, there's nothing
> special. I run cwm, FWIW, and do not start anything heavy from the
> beginning, to have a working desktop ASAP. KDE and GNOME won't work
> this way for sure. :-)

ok, I was wrong. My testing account did have a .xsession from a
previous test that caued the failure. With the real default Xsession
it still starts.

But I tend to agree with Theo that these backup strategies to write to
/tmp when one cannot write to $HOME are wrong.

Linux, systemd and XDG have inventend this /run/user/$uid tmpfs that
is created automagically and they use that in place of /tmp for
volatile things that don't beloing to $HOME, but this is not a can of
worms I want to open now.

-- 
Matthieu Herrb



Re: Ignore /tmp/.Xauth* in daily

2021-03-06 Thread Theo de Raadt
Vadim Zhukov  wrote:

> сб, 6 мар. 2021 г. в 20:53, Theo de Raadt :
> >
> > Vadim Zhukov  wrote:
> >
> > > > The backup dir can be configured to something else, but it needs to be
> > > > writeable by the user whois login in. It could be a subdir of /tmp (if
> > > > the rc.d script takes care of creating it) or I can remove that
> > > > feature from xenodm and fail the login if /home is not writeable.
> > >
> > > I've sent a diff for subdir of /tmp already. ;-)
> >
> > Sure, but the creation of a directory introduces new concerns.
> >
> > Why must non-readable $HOME work, and is the trade-off for placing
> > "keys" in /tmp worthwhile.
> >
> > It made sense to someone 30 years ago.  Does it make sense now?
> 
> Please correct me if I wrong: you said "non-readable $HOME" a few
> times during discussion, did you mean "read-only $HOME" instead?

non-writeable.

I'll start working on diffs to many parts of OpenBSD, that if they
cannot write files, they just throw them in /tmp

/sarc

*WHY* does X do this.  *WHY* do you think it is smart?

Justify it.  And I mean justify it beyond "I've accidentally been using
this".



Re: Ignore /tmp/.Xauth* in daily

2021-03-06 Thread Vadim Zhukov
сб, 6 мар. 2021 г. в 20:53, Theo de Raadt :
>
> Vadim Zhukov  wrote:
>
> > > The backup dir can be configured to something else, but it needs to be
> > > writeable by the user whois login in. It could be a subdir of /tmp (if
> > > the rc.d script takes care of creating it) or I can remove that
> > > feature from xenodm and fail the login if /home is not writeable.
> >
> > I've sent a diff for subdir of /tmp already. ;-)
>
> Sure, but the creation of a directory introduces new concerns.
>
> Why must non-readable $HOME work, and is the trade-off for placing
> "keys" in /tmp worthwhile.
>
> It made sense to someone 30 years ago.  Does it make sense now?

Please correct me if I wrong: you said "non-readable $HOME" a few
times during discussion, did you mean "read-only $HOME" instead?

-- 
  WBR,
  Vadim Zhukov



Re: Ignore /tmp/.Xauth* in daily

2021-03-06 Thread Vadim Zhukov
сб, 6 мар. 2021 г. в 20:50, Theo de Raadt :
>
> Matthieu Herrb  wrote:
>
> > On Sat, Mar 06, 2021 at 09:56:34AM -0700, Theo de Raadt wrote:
> > > Matthieu Herrb  wrote:
> > >
> > > > On Fri, Mar 05, 2021 at 09:10:32PM +0300, Vadim Zhukov wrote:
> > > > > чт, 4 мар. 2021 г. в 02:02, Vadim Zhukov :
> > > > > >
> > > > > > Hello all.
> > > > > >
> > > > > > Since xenodm has DEF_USER_AUTH_DIR set to "/tmp", we need to ignore
> > > > > > /tmp/.Xauth* in daily cleanup, don't we?
> > > > > >
> > > > > > Found the hard way a few minutes ago on my X240.
> > > > >
> > > > > Thanks sthen@, I've realized this happens only when xenodm could not
> > > > > create ~/.Xauthority. In my case this happens because my laptop starts
> > > > > with /home mounted read-only, but there may be others. Mattieu, the
> > > > > xenodm logic itself is correct, right? If yes, anyone brave enough to
> > > > > okay the diff below then? :-)
> > > >
> > > > Hi,
> > > >
> > > > Yes I think the xenodm logic (inherithed from xdm) is correct.
> > > >
> > > > Althoug in my experience, when an X session cnnot write to $HOME it
> > > > generally doesn't get very far (iirc not beeing able to write to
> > > > .xsession-errors used to be fatal)...
> >
> > I tried to log in with a read-only /home and it failed as I remembered
> > it. Vadim are your doing something special in you X session to make
> > that work   ?
> >
> >
> > > >
> > > > Anyways ok to skip that directory if it exists in daily.
> > >
> > > It is not a directory -- it is a file.
> > >
> > > I don't understand how this file is created.  Well-known names in /tmp
> > > are raceable -- therefore we and others increasingly use directories 
> > > containing
> > > files as a safer pattern.  Where is the code that creates this file?  Is 
> > > it
> > > safe?  I am suspicious.
> >
> > It's created by mkstemp(3) in
> > /usr/xenocara/app/xenodm/xenodm/auth.c:798
> >
> > >
> > > I strongly disagree with the pattern ".Xauth*".  It should be EXACT.  If
> > > someone else creates a file called .Xauthsadflkjdsaf, it should not be
> > > deleted.
> > >
> > > As a final point, is this strategy of considering /tmp a safe place 
> > > acceptable
> > > at all?  If $HOME doesn't work, why not just have X fail to work correctly
> > > and consider this "fail over to /tmp" a junk idea from the past?
> >
> > The backup dir can be configured to something else, but it needs to be
> > writeable by the user whois login in. It could be a subdir of /tmp (if
> > the rc.d script takes care of creating it) or I can remove that
> > feature from xenodm and fail the login if /home is not writeable.
>
> My question is more basic:  Why must there be a backup dir.
>
> Why not just fail hard.
>
> Should we change ssh so that if the home directory is non-readable, it
> store ssh keys and other ephmeral information in /tmp?   No surely not.

But you can still log in to read-only home via SSH.

> So why does X need to follow this pattern?  Is there a strong justification,
> or is it simply a decision made 30 years ago?

I feel myself having too little experience to judge. But: previously I
used "startx" and I was happy to type "mrw /home; startx" after
logging in (mrw is small script doing mount -uorw). Then startx was
broken and now I'm forced to use xenodm. So I have it enabled, and I
log in graphically from the start. If we'll drop /tmp failover, I'll
have to switch consoles and log in to text console in addition. Not a
big deal, of course.

-- 
  WBR,
  Vadim Zhukov



Re: Ignore /tmp/.Xauth* in daily

2021-03-06 Thread Theo de Raadt
Vadim Zhukov  wrote:

> > The backup dir can be configured to something else, but it needs to be
> > writeable by the user whois login in. It could be a subdir of /tmp (if
> > the rc.d script takes care of creating it) or I can remove that
> > feature from xenodm and fail the login if /home is not writeable.
> 
> I've sent a diff for subdir of /tmp already. ;-)

Sure, but the creation of a directory introduces new concerns.

Why must non-readable $HOME work, and is the trade-off for placing
"keys" in /tmp worthwhile.

It made sense to someone 30 years ago.  Does it make sense now?



Re: Ignore /tmp/.Xauth* in daily

2021-03-06 Thread Vadim Zhukov
сб, 6 мар. 2021 г. в 20:41, Matthieu Herrb :
>
> On Sat, Mar 06, 2021 at 09:56:34AM -0700, Theo de Raadt wrote:
> > Matthieu Herrb  wrote:
> >
> > > On Fri, Mar 05, 2021 at 09:10:32PM +0300, Vadim Zhukov wrote:
> > > > чт, 4 мар. 2021 г. в 02:02, Vadim Zhukov :
> > > > >
> > > > > Hello all.
> > > > >
> > > > > Since xenodm has DEF_USER_AUTH_DIR set to "/tmp", we need to ignore
> > > > > /tmp/.Xauth* in daily cleanup, don't we?
> > > > >
> > > > > Found the hard way a few minutes ago on my X240.
> > > >
> > > > Thanks sthen@, I've realized this happens only when xenodm could not
> > > > create ~/.Xauthority. In my case this happens because my laptop starts
> > > > with /home mounted read-only, but there may be others. Mattieu, the
> > > > xenodm logic itself is correct, right? If yes, anyone brave enough to
> > > > okay the diff below then? :-)
> > >
> > > Hi,
> > >
> > > Yes I think the xenodm logic (inherithed from xdm) is correct.
> > >
> > > Althoug in my experience, when an X session cnnot write to $HOME it
> > > generally doesn't get very far (iirc not beeing able to write to
> > > .xsession-errors used to be fatal)...
>
> I tried to log in with a read-only /home and it failed as I remembered
> it. Vadim are your doing something special in you X session to make
> that work   ?

Well, I do... nothing? I can show my .xsession, there's nothing
special. I run cwm, FWIW, and do not start anything heavy from the
beginning, to have a working desktop ASAP. KDE and GNOME won't work
this way for sure. :-)

For .xsession-errors, there is /tmp-based handling in
/etc/X11/xenodm/Xsession already. Oh, now I know where the disk space
on /tmp gets lost from time to time...

> > > Anyways ok to skip that directory if it exists in daily.
> >
> > It is not a directory -- it is a file.
> >
> > I don't understand how this file is created.  Well-known names in /tmp
> > are raceable -- therefore we and others increasingly use directories 
> > containing
> > files as a safer pattern.  Where is the code that creates this file?  Is it
> > safe?  I am suspicious.
>
> It's created by mkstemp(3) in
> /usr/xenocara/app/xenodm/xenodm/auth.c:798
>
> >
> > I strongly disagree with the pattern ".Xauth*".  It should be EXACT.  If
> > someone else creates a file called .Xauthsadflkjdsaf, it should not be
> > deleted.
> >
> > As a final point, is this strategy of considering /tmp a safe place 
> > acceptable
> > at all?  If $HOME doesn't work, why not just have X fail to work correctly
> > and consider this "fail over to /tmp" a junk idea from the past?
>
> The backup dir can be configured to something else, but it needs to be
> writeable by the user whois login in. It could be a subdir of /tmp (if
> the rc.d script takes care of creating it) or I can remove that
> feature from xenodm and fail the login if /home is not writeable.

I've sent a diff for subdir of /tmp already. ;-)

--
  WBR,
  Vadim Zhukov



Re: Ignore /tmp/.Xauth* in daily

2021-03-06 Thread Theo de Raadt
Matthieu Herrb  wrote:

> On Sat, Mar 06, 2021 at 09:56:34AM -0700, Theo de Raadt wrote:
> > Matthieu Herrb  wrote:
> > 
> > > On Fri, Mar 05, 2021 at 09:10:32PM +0300, Vadim Zhukov wrote:
> > > > чт, 4 мар. 2021 г. в 02:02, Vadim Zhukov :
> > > > >
> > > > > Hello all.
> > > > >
> > > > > Since xenodm has DEF_USER_AUTH_DIR set to "/tmp", we need to ignore
> > > > > /tmp/.Xauth* in daily cleanup, don't we?
> > > > >
> > > > > Found the hard way a few minutes ago on my X240.
> > > > 
> > > > Thanks sthen@, I've realized this happens only when xenodm could not
> > > > create ~/.Xauthority. In my case this happens because my laptop starts
> > > > with /home mounted read-only, but there may be others. Mattieu, the
> > > > xenodm logic itself is correct, right? If yes, anyone brave enough to
> > > > okay the diff below then? :-)
> > > 
> > > Hi,
> > > 
> > > Yes I think the xenodm logic (inherithed from xdm) is correct.
> > > 
> > > Althoug in my experience, when an X session cnnot write to $HOME it
> > > generally doesn't get very far (iirc not beeing able to write to
> > > .xsession-errors used to be fatal)...
> 
> I tried to log in with a read-only /home and it failed as I remembered
> it. Vadim are your doing something special in you X session to make
> that work   ?
>
> 
> > > 
> > > Anyways ok to skip that directory if it exists in daily.
> > 
> > It is not a directory -- it is a file.
> > 
> > I don't understand how this file is created.  Well-known names in /tmp
> > are raceable -- therefore we and others increasingly use directories 
> > containing
> > files as a safer pattern.  Where is the code that creates this file?  Is it
> > safe?  I am suspicious.
> 
> It's created by mkstemp(3) in
> /usr/xenocara/app/xenodm/xenodm/auth.c:798
> 
> > 
> > I strongly disagree with the pattern ".Xauth*".  It should be EXACT.  If
> > someone else creates a file called .Xauthsadflkjdsaf, it should not be
> > deleted.
> > 
> > As a final point, is this strategy of considering /tmp a safe place 
> > acceptable
> > at all?  If $HOME doesn't work, why not just have X fail to work correctly
> > and consider this "fail over to /tmp" a junk idea from the past?
> 
> The backup dir can be configured to something else, but it needs to be
> writeable by the user whois login in. It could be a subdir of /tmp (if
> the rc.d script takes care of creating it) or I can remove that
> feature from xenodm and fail the login if /home is not writeable.

My question is more basic:  Why must there be a backup dir.

Why not just fail hard.

Should we change ssh so that if the home directory is non-readable, it
store ssh keys and other ephmeral information in /tmp?   No surely not.

So why does X need to follow this pattern?  Is there a strong justification,
or is it simply a decision made 30 years ago?



Re: Ignore /tmp/.Xauth* in daily

2021-03-06 Thread Matthieu Herrb
On Sat, Mar 06, 2021 at 09:56:34AM -0700, Theo de Raadt wrote:
> Matthieu Herrb  wrote:
> 
> > On Fri, Mar 05, 2021 at 09:10:32PM +0300, Vadim Zhukov wrote:
> > > чт, 4 мар. 2021 г. в 02:02, Vadim Zhukov :
> > > >
> > > > Hello all.
> > > >
> > > > Since xenodm has DEF_USER_AUTH_DIR set to "/tmp", we need to ignore
> > > > /tmp/.Xauth* in daily cleanup, don't we?
> > > >
> > > > Found the hard way a few minutes ago on my X240.
> > > 
> > > Thanks sthen@, I've realized this happens only when xenodm could not
> > > create ~/.Xauthority. In my case this happens because my laptop starts
> > > with /home mounted read-only, but there may be others. Mattieu, the
> > > xenodm logic itself is correct, right? If yes, anyone brave enough to
> > > okay the diff below then? :-)
> > 
> > Hi,
> > 
> > Yes I think the xenodm logic (inherithed from xdm) is correct.
> > 
> > Althoug in my experience, when an X session cnnot write to $HOME it
> > generally doesn't get very far (iirc not beeing able to write to
> > .xsession-errors used to be fatal)...

I tried to log in with a read-only /home and it failed as I remembered
it. Vadim are your doing something special in you X session to make
that work   ?

> > 
> > Anyways ok to skip that directory if it exists in daily.
> 
> It is not a directory -- it is a file.
> 
> I don't understand how this file is created.  Well-known names in /tmp
> are raceable -- therefore we and others increasingly use directories 
> containing
> files as a safer pattern.  Where is the code that creates this file?  Is it
> safe?  I am suspicious.

It's created by mkstemp(3) in
/usr/xenocara/app/xenodm/xenodm/auth.c:798

> 
> I strongly disagree with the pattern ".Xauth*".  It should be EXACT.  If
> someone else creates a file called .Xauthsadflkjdsaf, it should not be
> deleted.
> 
> As a final point, is this strategy of considering /tmp a safe place acceptable
> at all?  If $HOME doesn't work, why not just have X fail to work correctly
> and consider this "fail over to /tmp" a junk idea from the past?

The backup dir can be configured to something else, but it needs to be
writeable by the user whois login in. It could be a subdir of /tmp (if
the rc.d script takes care of creating it) or I can remove that
feature from xenodm and fail the login if /home is not writeable.

-- 
Matthieu Herrb



Re: Ignore /tmp/.Xauth* in daily

2021-03-06 Thread Theo de Raadt
Matthieu Herrb  wrote:

> On Fri, Mar 05, 2021 at 09:10:32PM +0300, Vadim Zhukov wrote:
> > чт, 4 мар. 2021 г. в 02:02, Vadim Zhukov :
> > >
> > > Hello all.
> > >
> > > Since xenodm has DEF_USER_AUTH_DIR set to "/tmp", we need to ignore
> > > /tmp/.Xauth* in daily cleanup, don't we?
> > >
> > > Found the hard way a few minutes ago on my X240.
> > 
> > Thanks sthen@, I've realized this happens only when xenodm could not
> > create ~/.Xauthority. In my case this happens because my laptop starts
> > with /home mounted read-only, but there may be others. Mattieu, the
> > xenodm logic itself is correct, right? If yes, anyone brave enough to
> > okay the diff below then? :-)
> 
> Hi,
> 
> Yes I think the xenodm logic (inherithed from xdm) is correct.
> 
> Althoug in my experience, when an X session cnnot write to $HOME it
> generally doesn't get very far (iirc not beeing able to write to
> .xsession-errors used to be fatal)...
> 
> Anyways ok to skip that directory if it exists in daily.

It is not a directory -- it is a file.

I don't understand how this file is created.  Well-known names in /tmp
are raceable -- therefore we and others increasingly use directories containing
files as a safer pattern.  Where is the code that creates this file?  Is it
safe?  I am suspicious.

I strongly disagree with the pattern ".Xauth*".  It should be EXACT.  If
someone else creates a file called .Xauthsadflkjdsaf, it should not be
deleted.

As a final point, is this strategy of considering /tmp a safe place acceptable
at all?  If $HOME doesn't work, why not just have X fail to work correctly
and consider this "fail over to /tmp" a junk idea from the past?


> > 
> > > Index: daily
> > > ===
> > > RCS file: /cvs/src/etc/daily,v
> > > retrieving revision 1.95
> > > diff -u -p -r1.95 daily
> > > --- daily   20 Oct 2020 22:42:29 -  1.95
> > > +++ daily   3 Mar 2021 22:58:28 -
> > > @@ -49,7 +49,7 @@ if [ -d /tmp -a ! -L /tmp ]; then
> > > cd /tmp && {
> > > find -x . \
> > > \( -path './ssh-*' -o -path ./.X11-unix -o -path ./.ICE-unix \
> > > -   -o -path './tmux-*' \) \
> > > +   -o -path './tmux-*' -o -path './.Xauth*' \) \
> > > -prune -o -type f -atime +7 -delete 2>/dev/null
> > > find -x . -type d -mtime +1 ! -path ./vi.recover ! -path 
> > > ./.X11-unix \
> > > ! -path ./.ICE-unix ! -name . \
> > 
> > -- 
> >   WBR,
> >   Vadim Zhukov
> 
> -- 
> Matthieu Herrb
> 



Re: Ignore /tmp/.Xauth* in daily

2021-03-06 Thread Matthieu Herrb
On Fri, Mar 05, 2021 at 09:10:32PM +0300, Vadim Zhukov wrote:
> чт, 4 мар. 2021 г. в 02:02, Vadim Zhukov :
> >
> > Hello all.
> >
> > Since xenodm has DEF_USER_AUTH_DIR set to "/tmp", we need to ignore
> > /tmp/.Xauth* in daily cleanup, don't we?
> >
> > Found the hard way a few minutes ago on my X240.
> 
> Thanks sthen@, I've realized this happens only when xenodm could not
> create ~/.Xauthority. In my case this happens because my laptop starts
> with /home mounted read-only, but there may be others. Mattieu, the
> xenodm logic itself is correct, right? If yes, anyone brave enough to
> okay the diff below then? :-)

Hi,

Yes I think the xenodm logic (inherithed from xdm) is correct.

Althoug in my experience, when an X session cnnot write to $HOME it
generally doesn't get very far (iirc not beeing able to write to
.xsession-errors used to be fatal)...

Anyways ok to skip that directory if it exists in daily.

> 
> > Index: daily
> > ===
> > RCS file: /cvs/src/etc/daily,v
> > retrieving revision 1.95
> > diff -u -p -r1.95 daily
> > --- daily   20 Oct 2020 22:42:29 -  1.95
> > +++ daily   3 Mar 2021 22:58:28 -
> > @@ -49,7 +49,7 @@ if [ -d /tmp -a ! -L /tmp ]; then
> > cd /tmp && {
> > find -x . \
> > \( -path './ssh-*' -o -path ./.X11-unix -o -path ./.ICE-unix \
> > -   -o -path './tmux-*' \) \
> > +   -o -path './tmux-*' -o -path './.Xauth*' \) \
> > -prune -o -type f -atime +7 -delete 2>/dev/null
> > find -x . -type d -mtime +1 ! -path ./vi.recover ! -path 
> > ./.X11-unix \
> > ! -path ./.ICE-unix ! -name . \
> 
> -- 
>   WBR,
>   Vadim Zhukov

-- 
Matthieu Herrb



Re: Ignore /tmp/.Xauth* in daily

2021-03-05 Thread Vadim Zhukov
чт, 4 мар. 2021 г. в 02:02, Vadim Zhukov :
>
> Hello all.
>
> Since xenodm has DEF_USER_AUTH_DIR set to "/tmp", we need to ignore
> /tmp/.Xauth* in daily cleanup, don't we?
>
> Found the hard way a few minutes ago on my X240.

Thanks sthen@, I've realized this happens only when xenodm could not
create ~/.Xauthority. In my case this happens because my laptop starts
with /home mounted read-only, but there may be others. Mattieu, the
xenodm logic itself is correct, right? If yes, anyone brave enough to
okay the diff below then? :-)

> Index: daily
> ===
> RCS file: /cvs/src/etc/daily,v
> retrieving revision 1.95
> diff -u -p -r1.95 daily
> --- daily   20 Oct 2020 22:42:29 -  1.95
> +++ daily   3 Mar 2021 22:58:28 -
> @@ -49,7 +49,7 @@ if [ -d /tmp -a ! -L /tmp ]; then
> cd /tmp && {
> find -x . \
> \( -path './ssh-*' -o -path ./.X11-unix -o -path ./.ICE-unix \
> -   -o -path './tmux-*' \) \
> +   -o -path './tmux-*' -o -path './.Xauth*' \) \
> -prune -o -type f -atime +7 -delete 2>/dev/null
> find -x . -type d -mtime +1 ! -path ./vi.recover ! -path ./.X11-unix \
> ! -path ./.ICE-unix ! -name . \

-- 
  WBR,
  Vadim Zhukov



Ignore /tmp/.Xauth* in daily

2021-03-03 Thread Vadim Zhukov
Hello all.

Since xenodm has DEF_USER_AUTH_DIR set to "/tmp", we need to ignore
/tmp/.Xauth* in daily cleanup, don't we?

Found the hard way a few minutes ago on my X240.

Okay?

--
WBR,
  Vadim Zhukov


Index: daily
===
RCS file: /cvs/src/etc/daily,v
retrieving revision 1.95
diff -u -p -r1.95 daily
--- daily   20 Oct 2020 22:42:29 -  1.95
+++ daily   3 Mar 2021 22:58:28 -
@@ -49,7 +49,7 @@ if [ -d /tmp -a ! -L /tmp ]; then
cd /tmp && {
find -x . \
\( -path './ssh-*' -o -path ./.X11-unix -o -path ./.ICE-unix \
-   -o -path './tmux-*' \) \
+   -o -path './tmux-*' -o -path './.Xauth*' \) \
-prune -o -type f -atime +7 -delete 2>/dev/null
find -x . -type d -mtime +1 ! -path ./vi.recover ! -path ./.X11-unix \
! -path ./.ICE-unix ! -name . \