Failing {lib,}gnome-keyring tests: How to make mlock/munlock available to non-root?

2014-10-15 Thread Svante Signell
Hi,

Currently libgnome-keyring and gnome-keyring secmem related tests fail,
falling back to mlock/munlock requiring root permissions to lock
memory. 
In the code we have:
  err = __get_privileged_ports (&hostpriv, NULL);
  if (err)
return __hurd_fail (EPERM);

How to make mock/munlock available to the user (process owner), not only
root? It seems that POSIX allows both root-only and non-root.
http://pubs.opengroup.org/onlinepubs/9699919799/functions/mlock.html
"Appropriate privileges are required to lock process memory with
mlock()."

Linux has a non-root implementation and kFreeBSD is moving towards one
in kernel 10.x, see https://bugs.debian.org/628383

Thanks!





Re: Failing {lib,}gnome-keyring tests: How to make mlock/munlock available to non-root?

2014-10-15 Thread Svante Signell
On Wed, 2014-10-15 at 09:52 +0200, Svante Signell wrote:
> Hi,
> 
> Currently libgnome-keyring and gnome-keyring secmem related tests fail,
> falling back to mlock/munlock requiring root permissions to lock
> memory. 
> In the code we have:
>   err = __get_privileged_ports (&hostpriv, NULL);
>   if (err)
> return __hurd_fail (EPERM);
> 
> How to make mock/munlock available to the user (process owner), not only
> root? It seems that POSIX allows both root-only and non-root.
> http://pubs.opengroup.org/onlinepubs/9699919799/functions/mlock.html
> "Appropriate privileges are required to lock process memory with
> mlock()."
> 
> Linux has a non-root implementation and kFreeBSD is moving towards one
> in kernel 10.x, see https://bugs.debian.org/628383

See also https://lists.debian.org/debian-devel/2014/10/msg00201.html for
a discussion on the topic.




Re: Failing {lib,}gnome-keyring tests: How to make mlock/munlock available to non-root?

2014-10-15 Thread Samuel Thibault
Svante Signell, le Wed 15 Oct 2014 09:57:21 +0200, a écrit :
> See also https://lists.debian.org/debian-devel/2014/10/msg00201.html for
> a discussion on the topic.

I can't understand why you proposed to use setuid in order to keep
secrets, but oh well.

To get mlock available to user should be a matter of making gnumach
accept vm_wire calls with hostpriv == 0. The amount of such locked
memory shall however be accounted and limited. The default on my Linux
system is 64KB.

Samuel



Re: Failing {lib,}gnome-keyring tests: How to make mlock/munlock available to non-root?

2014-10-15 Thread Samuel Thibault
Samuel Thibault, le Wed 15 Oct 2014 10:06:33 +0200, a écrit :
> The amount of such locked memory shall however be accounted and
> limited. The default on my Linux system is 64KB.

And it should be configurable per-process through ulimit -l.

Samuel



Re: Failing {lib,}gnome-keyring tests: How to make mlock/munlock available to non-root?

2014-10-15 Thread Svante Signell
On Wed, 2014-10-15 at 10:06 +0200, Samuel Thibault wrote:
> Svante Signell, le Wed 15 Oct 2014 09:57:21 +0200, a écrit :
> > See also https://lists.debian.org/debian-devel/2014/10/msg00201.html for
> > a discussion on the topic.
> 
> I can't understand why you proposed to use setuid in order to keep
> secrets, but oh well.

I did not seriously propose to use setuid, it was mostly a way to get
answers about security issues of setuid programs, and I got plenty of
them.

> To get mlock available to user should be a matter of making gnumach
> accept vm_wire calls with hostpriv == 0. The amount of such locked
> memory shall however be accounted and limited. The default on my Linux
> system is 64KB.

Isn't it dangerous to remove/special case on 
   if (host == HOST_NULL)
return KERN_INVALID_HOST;
in vm_wire.c?

And where to place the defaults and ulimit checks, vm_wire.c or
mlock.c/munlock.c?

BTW: ulimit() is obsolete, one should use getrlimit() and setrlimit()
nowadays, according to the manpage.




Re: Failing {lib,}gnome-keyring tests: How to make mlock/munlock available to non-root?

2014-10-15 Thread Samuel Thibault
Svante Signell, le Wed 15 Oct 2014 10:56:41 +0200, a écrit :
> On Wed, 2014-10-15 at 10:06 +0200, Samuel Thibault wrote:
> > Svante Signell, le Wed 15 Oct 2014 09:57:21 +0200, a écrit :
> > > See also https://lists.debian.org/debian-devel/2014/10/msg00201.html for
> > > a discussion on the topic.
> > 
> > I can't understand why you proposed to use setuid in order to keep
> > secrets, but oh well.
> 
> I did not seriously propose to use setuid,

Well, it did look like you were doing it.

> > To get mlock available to user should be a matter of making gnumach
> > accept vm_wire calls with hostpriv == 0. The amount of such locked
> > memory shall however be accounted and limited. The default on my Linux
> > system is 64KB.
> 
> Isn't it dangerous to remove/special case on 
>if (host == HOST_NULL)
> return KERN_INVALID_HOST;
> in vm_wire.c?

It isn't if the amount of wirable memory is limited, thus my talking
about the limitation.

> And where to place the defaults and ulimit checks, vm_wire.c or
> mlock.c/munlock.c?

In whatever actually does the wiring inside gnumach.

> BTW: ulimit() is obsolete, one should use getrlimit() and setrlimit()
> nowadays, according to the manpage.

I didn't talk about ulimit() but ulimit -l, which does use
getrlimit()/setrlimit().

Samuel



bug in puts(3) affecting /hurd/init

2014-10-15 Thread Justus Winter
Hello,

there is a strange bug in puts(3) affecting /hurd/init (or
/hurd/startup if you live on the edge).  It is also sometimes
affecting printf(3), as it is replaced by gcc with a call to puts
under certain conditions (e.g. with a format string ending in '\n'
without any conversion specifiers).

Here is a screenshot of /hurd/startup patched with the patch below:

[... gnumach stuff...]
task loaded: exec /hurd/exec

start ext2fs: Hurd server bootstrap: ext2fs[device:hd0s1] exec startup proc 
auth.

INIT: version 2.88 booting
Using makefile-style concurrent boot in runlevel S.
[...]

Note the '.' after "auth", followed by a newline.  The extra newline
is printed by our runsystem script, as a workaround for this bug.

This is what the shutdown is supposed to look like:

[...]
Deactivating swap...swapoff: /dev/hd0s5: 177152k swap space
done.
Unmounting local filesystems...done.
mount: cannot remount /: Device or resource busy
Will now halt.
startup: notifying pfinet of halt...done
startup: notifying tmpfs none of halt...done
startup: notifying tmpfs none of halt...done
startup: notifying tmpfs none of halt...done
startup: notifying ext2fs device:hd0s1 of halt...done
startup: halting Mach (flags 0x8)...

In contrast, here is a shutdown with the unpatched /hurd/init:

root@debian:~# halt-hurd
init: notifying pfinet of shutdown...init: notifying tmpfs none of 
shutdown...init: notifying tmpfs none of shutdown...init: notifying tmpfs none 
of shutdown...init: notifying ext2fs device:hd0s1 of shutdown...init: halting 
Mach (flags 0x8)...

Note that contrary to my comment in the patch below it isn't only
swallowing the newline, but the whole string (i.e. ".\n" and
"done\n").

I've no idea whats wrong, or why it is only affecting /hurd/startup.

Maybe it's the way /hurd/startup setups stdout.  Here's the code:

  /* Fetch a port to the bootstrap filesystem, the host priv and
 master device ports, and the console.  */
  if (task_get_bootstrap_port (mach_task_self (), &bootport)
  || fsys_getpriv (bootport, &host_priv, &device_master, &fstask)
  || device_open (device_master, D_WRITE, "console", &consdev))
crash_mach ();
[...]
  stderr = stdout = mach_open_devstream (consdev, "w");
  stdin = mach_open_devstream (consdev, "r");
  if (stdout == NULL || stdin == NULL)
crash_mach ();
  setbuf (stdout, NULL);

In contrast, here is what libdiskfs does:

  err = get_privileged_ports (NULL, &dev);
  assert_perror (err);
  err = device_open (dev, D_READ|D_WRITE, "console", &cons);
  mach_port_deallocate (mach_task_self (), dev);
  assert_perror (err);
  stdin = mach_open_devstream (cons, "r");
  stdout = stderr = mach_open_devstream (cons, "w");

So, startup calls setbuf while libdiskfs does not.  I once removed
that call, it didn't affect the bug.

Also, startup only passes D_WRITE to device_open, while libdiskfs uses
D_READ|D_WRITE.  This is probably a bug, but I would expect it do
affect reading from stdin (if anything), not writing to stdout.

Thoughts?
Justus

diff --git a/startup/startup.c b/startup/startup.c
index ff58270..e1f07a2 100644
--- a/startup/startup.c
+++ b/startup/startup.c
@@ -1663,3 +1663,10 @@ S_fsys_forward (mach_port_t server, mach_port_t 
requestor,
 {
   return EOPNOTSUPP;
 }
+
+/* XXX: puts is broken, it doesn't print the newline.  */
+int
+puts (const char *s)
+{
+  return printf ("%s%c", s, '\n') == 0? EOF: 1;
+}



Re: bug in puts(3) affecting /hurd/init

2014-10-15 Thread Samuel Thibault
Does the puts() call return some error?

Justus Winter, le Wed 15 Oct 2014 16:46:50 +0200, a écrit :
> So, startup calls setbuf while libdiskfs does not.

Does libdiskfs really do some puts that is working?

> Also, startup only passes D_WRITE to device_open, while libdiskfs uses
> D_READ|D_WRITE.  This is probably a bug, but I would expect it do
> affect reading from stdin (if anything), not writing to stdout.

I don't see why it would have an effect on the output part indeed.

Samuel



Re: bug in puts(3) affecting /hurd/init

2014-10-15 Thread Samuel Thibault
Samuel Thibault, le Thu 16 Oct 2014 01:07:13 +0200, a écrit :
> Does the puts() call return some error?

I mean perhaps _IO_puts stops early because _IO_vtable_offset or
_IO_fwide() fail.

Samuel



Re: [PATCH 8/8] startup: bind the startup server to /servers/startup

2014-10-15 Thread Justus Winter
Hi Roland :)

Quoting David Michael (2014-09-18 23:14:17)
> On Wed, Sep 3, 2014 at 8:33 AM, Justus Winter
> <4win...@informatik.uni-hamburg.de> wrote:
> > Bind the startup server to /servers/startup instead.  Use this to
> > contact the startup server.
> 
> I'm trying to test this patch, and glibc appears to need an update as
> well.  Does this look okay?
> 
> Thanks.
> 
> David
> 
> 
> diff --git a/sysdeps/mach/hurd/reboot.c b/sysdeps/mach/hurd/reboot.c
> index 60d96ea..51c3d73 100644
> --- a/sysdeps/mach/hurd/reboot.c
> +++ b/sysdeps/mach/hurd/reboot.c
> @@ -18,6 +18,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
> 
> @@ -33,8 +34,8 @@ reboot (int howto)
>if (err)
>  return __hurd_fail (EPERM);
> 
> -  err = __USEPORT (PROC, __proc_getmsgport (port, 1, &init));
> -  if (!err)
> +  init = __file_name_lookup (_SERVERS_STARTUP, 0, 0);
> +  if (init != MACH_PORT_NULL)
>  {
>err = __startup_reboot (init, hostpriv, howto);
>__mach_port_deallocate (__mach_task_self (), init);

I'd love to draw your attention to this thread.  The short story is
that we want to change how we interact with the init server.
Previously, the message port was looked up using proc_getmsgport, but
this turned out to be not ideal since a/ we changed the pid of the
init server to free pid 1 for sysvinit, and b/ this lookup mechanism
is not easily interposable.

Thanks,
Justus