Re: [PATCH v2][xdm] Implement ConsoleKit support.

2011-07-23 Thread Takashi Iwai
At Fri, 22 Jul 2011 21:49:08 +0200,
Julien Cristau wrote:
 
  +   display_device = devtmp;
  +}
  +
  +connector = ck_connector_new();
  +if (!connector) {
  +   LogOutOfMem(ck_connector);
  +   return 0;
  +}
  +
  +dbus_error_init(error);
  +ret = ck_connector_open_session_with_parameters(
  +   connector, error,
  +   unix-user, verify-uid,
  +   x11-display, display_name,
  +   x11-display-device, display_device,
  +   remote-host-name, remote_host_name,
  +   is-local, is_local,
  +   NULL);
  +if (!ret) {
  +   if (dbus_error_is_set(error)) {
  +   LogError(Dbus error: %s\n, error.message);
  +   dbus_error_free(error);
  +   } else {
  +   LogError(ConsoleKit error\n);
  +   }
  +   LogError(console-kit-daemon not running?\n);
  +   ck_connector_unref(connector);
  +   connector = NULL;
  +   return 0;
 
 Is ck-daemon isn't running a good reason to prevent the user from
 logging in?  (Why?)

It's reached only when you set use_consolekit is set.  You say you
are using CK but it's not detected, thus it's an error.


thanks,

Takashi
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH v2][xdm] Implement ConsoleKit support.

2011-07-23 Thread Julien Cristau
On Sat, Jul 23, 2011 at 09:31:41 +0200, Takashi Iwai wrote:

 At Fri, 22 Jul 2011 21:49:08 +0200,
 Julien Cristau wrote:
  
  Is ck-daemon isn't running a good reason to prevent the user from
  logging in?  (Why?)
 
 It's reached only when you set use_consolekit is set.  You say you
 are using CK but it's not detected, thus it's an error.
 
use_consolekit defaults to yes, so that's not really true.  It doesn't
seem to be flagged as an error by pam-ck-connector, in any case (it
returns PAM_IGNORE), I'm not sure why it's fatal for xdm.

Cheers,
Julien
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH v2][xdm] Implement ConsoleKit support.

2011-07-23 Thread Takashi Iwai
At Sat, 23 Jul 2011 10:49:53 +0200,
Julien Cristau wrote:
 
 On Sat, Jul 23, 2011 at 09:31:41 +0200, Takashi Iwai wrote:
 
  At Fri, 22 Jul 2011 21:49:08 +0200,
  Julien Cristau wrote:
   
   Is ck-daemon isn't running a good reason to prevent the user from
   logging in?  (Why?)
  
  It's reached only when you set use_consolekit is set.  You say you
  are using CK but it's not detected, thus it's an error.
  
 use_consolekit defaults to yes, so that's not really true.

Feel free to set it off as default.

But note that this is set true only when the consolekit support is
found and enabled via configure script, and most modern distros will
make it enabled as default anyway.

  It doesn't
 seem to be flagged as an error by pam-ck-connector, in any case (it
 returns PAM_IGNORE), I'm not sure why it's fatal for xdm.

It could be handled as non-fatal when no error code is set, too.

But usually you'll face problems sooner or later in such a situation
because it means that you logged in without proper device
permissions.


Takashi
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH v2][xdm] Implement ConsoleKit support.

2011-07-23 Thread Julien Cristau
On Sat, Jul 23, 2011 at 12:35:11 +0200, Takashi Iwai wrote:

 At Sat, 23 Jul 2011 10:49:53 +0200,
 Julien Cristau wrote:
  
  On Sat, Jul 23, 2011 at 09:31:41 +0200, Takashi Iwai wrote:
  
   At Fri, 22 Jul 2011 21:49:08 +0200,
   Julien Cristau wrote:

Is ck-daemon isn't running a good reason to prevent the user from
logging in?  (Why?)
   
   It's reached only when you set use_consolekit is set.  You say you
   are using CK but it's not detected, thus it's an error.
   
  use_consolekit defaults to yes, so that's not really true.
 
 Feel free to set it off as default.
 
 But note that this is set true only when the consolekit support is
 found and enabled via configure script, and most modern distros will
 make it enabled as default anyway.
 
There's a difference between a build-time setting and a run-time
setting.

   It doesn't
  seem to be flagged as an error by pam-ck-connector, in any case (it
  returns PAM_IGNORE), I'm not sure why it's fatal for xdm.
 
 It could be handled as non-fatal when no error code is set, too.
 
 But usually you'll face problems sooner or later in such a situation
 because it means that you logged in without proper device
 permissions.
 
I don't have access to the audio device is a better failure mode than
I can't even log in, in my book.

Cheers,
Julien
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH v2][xdm] Implement ConsoleKit support.

2011-07-23 Thread Takashi Iwai
At Sat, 23 Jul 2011 12:49:29 +0200,
Julien Cristau wrote:
 
 On Sat, Jul 23, 2011 at 12:35:11 +0200, Takashi Iwai wrote:
 
  At Sat, 23 Jul 2011 10:49:53 +0200,
  Julien Cristau wrote:
   
   On Sat, Jul 23, 2011 at 09:31:41 +0200, Takashi Iwai wrote:
   
At Fri, 22 Jul 2011 21:49:08 +0200,
Julien Cristau wrote:
 
 Is ck-daemon isn't running a good reason to prevent the user from
 logging in?  (Why?)

It's reached only when you set use_consolekit is set.  You say you
are using CK but it's not detected, thus it's an error.

   use_consolekit defaults to yes, so that's not really true.
  
  Feel free to set it off as default.
  
  But note that this is set true only when the consolekit support is
  found and enabled via configure script, and most modern distros will
  make it enabled as default anyway.
  
 There's a difference between a build-time setting and a run-time
 setting.

Yes, but there is no difference: most distros will do enable it as
default for runtime setting, too.

It doesn't
   seem to be flagged as an error by pam-ck-connector, in any case (it
   returns PAM_IGNORE), I'm not sure why it's fatal for xdm.
  
  It could be handled as non-fatal when no error code is set, too.
  
  But usually you'll face problems sooner or later in such a situation
  because it means that you logged in without proper device
  permissions.
  
 I don't have access to the audio device is a better failure mode than
 I can't even log in, in my book.

Its your book, and others may think differently like me.  I'd like to
see the obvious error beforehand than too late.

That said, this is a kind of problem for which no 100% correct answer
exists.  And I personally don't care at all how the default behavior
would be, as this is about a handling of exceptional error case.  So
feel free to cook the code.


thanks,

Takashi
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH v2][xdm] Implement ConsoleKit support.

2011-07-23 Thread Fernando Lemos
On Fri, Jul 22, 2011 at 4:49 PM, Julien Cristau jcris...@debian.org wrote:
 On Mon, Jun 27, 2011 at 21:04:25 +0200, Cyril Brulebois wrote:

 +static int openCKSession(struct verify_info *verify, struct display *d)
 +{
 +    int ret;
 +    DBusError error;
 +    char *remote_host_name = ;
 +    dbus_bool_t is_local;
 +    char *display_name = ;
 +    char *display_device = ;
 +    char devtmp[16];
 +
 +    if (!use_consolekit)
 +     return 1;
 +
 +    is_local = d-displayType.location == Local;
 +    if (d-peerlen  0  d-peer)
 +     remote_host_name = d-peer;
 +    if (d-name)
 +     display_name = d-name;
 +    /* how can we get the corresponding tty at best...? */
 +    if (d-windowPath) {
 +     display_device = strchr(d-windowPath, ':');
 +     if (display_device  display_device[1])
 +         display_device++;
 +     else
 +         display_device = d-windowPath;
 +     snprintf(devtmp, sizeof(devtmp), /dev/tty%s, display_device);

 Is ck relevant to anything !linux?  If so, that path is probably wrong.
 I'm not quite sure what ck users would do with that information, though,
 and a quick codesearch doesn't really help.  So maybe we should just not
 set that…

Good point. It's ported to FreeBSD, and FreeBSD supports /dev/tty* too
(actually all BSDs do). I have no idea about other platforms, and I'm
not sure there's a portable way to do it. On the other hand, I would
think most (all?) platforms where an user would want to enable CK
support are covered, but that's more of a guess.


 +     display_device = devtmp;
 +    }
 +
 +    connector = ck_connector_new();
 +    if (!connector) {
 +     LogOutOfMem(ck_connector);
 +     return 0;
 +    }
 +
 +    dbus_error_init(error);
 +    ret = ck_connector_open_session_with_parameters(
 +             connector, error,
 +             unix-user, verify-uid,
 +             x11-display, display_name,
 +             x11-display-device, display_device,
 +             remote-host-name, remote_host_name,
 +             is-local, is_local,
 +             NULL);
 +    if (!ret) {
 +     if (dbus_error_is_set(error)) {
 +         LogError(Dbus error: %s\n, error.message);
 +         dbus_error_free(error);
 +     } else {
 +         LogError(ConsoleKit error\n);
 +     }
 +     LogError(console-kit-daemon not running?\n);
 +     ck_connector_unref(connector);
 +     connector = NULL;
 +     return 0;

 Is ck-daemon isn't running a good reason to prevent the user from
 logging in?  (Why?)

I agree it's better to let the user log in. Without a ck session, the
user might be unable to do some pretty important stuff (configure NM,
shutdown, etc.), but that's still better than not being able to log
in.
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH v2][xdm] Implement ConsoleKit support.

2011-07-22 Thread Julien Cristau
On Mon, Jun 27, 2011 at 21:04:25 +0200, Cyril Brulebois wrote:

 +static int openCKSession(struct verify_info *verify, struct display *d)
 +{
 +int ret;
 +DBusError error;
 +char *remote_host_name = ;
 +dbus_bool_t is_local;
 +char *display_name = ;
 +char *display_device = ;
 +char devtmp[16];
 +
 +if (!use_consolekit)
 + return 1;
 +
 +is_local = d-displayType.location == Local;
 +if (d-peerlen  0  d-peer)
 + remote_host_name = d-peer;
 +if (d-name)
 + display_name = d-name;
 +/* how can we get the corresponding tty at best...? */
 +if (d-windowPath) {
 + display_device = strchr(d-windowPath, ':');
 + if (display_device  display_device[1])
 + display_device++;
 + else
 + display_device = d-windowPath;
 + snprintf(devtmp, sizeof(devtmp), /dev/tty%s, display_device);

Is ck relevant to anything !linux?  If so, that path is probably wrong.
I'm not quite sure what ck users would do with that information, though,
and a quick codesearch doesn't really help.  So maybe we should just not
set that…

 + display_device = devtmp;
 +}
 +
 +connector = ck_connector_new();
 +if (!connector) {
 + LogOutOfMem(ck_connector);
 + return 0;
 +}
 +
 +dbus_error_init(error);
 +ret = ck_connector_open_session_with_parameters(
 + connector, error,
 + unix-user, verify-uid,
 + x11-display, display_name,
 + x11-display-device, display_device,
 + remote-host-name, remote_host_name,
 + is-local, is_local,
 + NULL);
 +if (!ret) {
 + if (dbus_error_is_set(error)) {
 + LogError(Dbus error: %s\n, error.message);
 + dbus_error_free(error);
 + } else {
 + LogError(ConsoleKit error\n);
 + }
 + LogError(console-kit-daemon not running?\n);
 + ck_connector_unref(connector);
 + connector = NULL;
 + return 0;

Is ck-daemon isn't running a good reason to prevent the user from
logging in?  (Why?)

 +}
 +
 +verify-userEnviron = setEnv(verify-userEnviron,
 + XDG_SESSION_COOKIE, (char 
 *)ck_connector_get_cookie(connector));
 +return 1;
 +}
 +

Cheers,
Julien
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH v2][xdm] Implement ConsoleKit support.

2011-07-22 Thread Alan Coopersmith
On 07/22/11 12:49 PM, Julien Cristau wrote:
 Is ck relevant to anything !linux?

We have it on Solaris, I think because gdm made us.

-- 
-Alan Coopersmith-alan.coopersm...@oracle.com
 Oracle Solaris Platform Engineering: X Window System

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


[PATCH v2][xdm] Implement ConsoleKit support.

2011-06-27 Thread Cyril Brulebois
Since ConsoleKit 0.4.2, the sessions created by ck-launch-session are no
longer marked as local and active. This means that distributions can't simply
call ck-launch-session in Xsession.d anymore to get an active local ConsoleKit
session. Instead, XDM must create the session directly, providing info such as
the UID of the user and the tty used to log in (like GDM does).

It's important that locally logged in users get active local ConsoleKit
sessions because that's used by PolicyKit to determine whether certain actions
are allowed or not (according to a set of policies, of course). As an example
of how this can be used, UDisks can automount devices for locally logged in
users without a password prompt with their default PolicyKit policy.

This patch allows XDM to create a ConsoleKit session. This is disabled at
build time by default, you need to enable it with --with-consolekit. It relies
on a very lightweight library called libck-connector (which is distributed
with ConsoleKit) to do the DBus magic required for the creation of the active
local session.

openSUSE bug #528829 http://bugzilla.novell.com/show_bug.cgi?id=528829
X.Org bug #17325 http://bugs.freedesktop.org/show_bug.cgi?id=17325

Signed-off-by: Takashi Iwai ti...@suse.de
Tested-by: Cyril Brulebois k...@debian.org
Signed-off-by: Cyril Brulebois k...@debian.org
---
 configure.ac   |   12 ++
 include/dm.h   |3 ++
 man/xdm.man|6 +++
 xdm/resource.c |   13 ++-
 xdm/session.c  |  102 
 5 files changed, 135 insertions(+), 1 deletions(-)


To recap, Takashi Iwai seems to be the author, and also agreed to have the patch
pushed upstream (https://bugzilla.novell.com/show_bug.cgi?id=528829#c3);
Fernando and Stefan “only” passed it on (but feel free to correct that, I can
add your S-o-b). Thanks for passing on the patch anyway! And finally I
made some modifications and tested it, hence the tags.

From v1, mostly:
 - configure.ac: redone to follow Dan's excellent comments.

Tests I performed:
 - even with ck-connector's devel files available, ./configure defaults to no ck
   support; same result with an explicit --without-consolekit
 - without ck-connector's devel files, ./configure --with-consolekit bails out
   since it doesn't find ck through pkg-config.
 - with ck-connector's devel files, ./configure --with-consolekit builds xdm
   with ck support, and the xdm binary gets an extra NEEDED entry:
   libck-connector.so.0

Consequently:
 - distros enabling ck support will have an extra dependency on the
   libck-connector library;
 - they also might want to add a dependency on the consolekit daemon; if it
   can't be contacted while starting the session, that session dies
   right away and one gets back the xdm prompt. It's still possible to
   start xdm with -noconsolekit though, in which case sessions can be
   started without consolekit.

[ At least that last point should be in the commit message, I think. ]

Run-time tests performed with xfce4: Without active, local ck support,
one can't shut down, restart, suspend or hibernate. With ck support, all
that becomes possible (again…).


diff --git a/configure.ac b/configure.ac
index 0c7..4fc0531 100644
--- a/configure.ac
+++ b/configure.ac
@@ -358,6 +358,18 @@ PKG_CHECK_MODULES(DMCP, xdmcp)
 PKG_CHECK_MODULES(XLIB, x11)
 PKG_CHECK_MODULES(AUTH, xau)
 
+# ConsoleKit support
+AC_ARG_WITH(consolekit,
+   AS_HELP_STRING([--with-consolekit], [Use ConsoleKit]),
+   [USE_CONSOLEKIT=$withval],
+   [USE_CONSOLEKIT=no])
+if test x$USE_CONSOLEKIT != xno; then
+   PKG_CHECK_MODULES(CK_CONNECTOR, ck-connector)
+   AC_DEFINE([USE_CONSOLEKIT], 1, [Define to 1 to use ConsoleKit])
+   XDM_CFLAGS=$XDM_CFLAGS $CK_CONNECTOR_CFLAGS
+   XDM_LIBS=$XDM_LIBS $CK_CONNECTOR_LIBS
+fi
+
 #
 # Greeter
 #
diff --git a/include/dm.h b/include/dm.h
index 316dd46..b81d735 100644
--- a/include/dm.h
+++ b/include/dm.h
@@ -325,6 +325,9 @@ extern char *randomFile;
 extern char*prngdSocket;
 extern int prngdPort;
 # endif
+# ifdef USE_CONSOLEKIT
+extern int use_consolekit;
+# endif
 
 extern char*greeterLib;
 extern char*willing;
diff --git a/man/xdm.man b/man/xdm.man
index 9590c1a..fb50349 100644
--- a/man/xdm.man
+++ b/man/xdm.man
@@ -48,6 +48,8 @@ xdm \- X Display Manager with support for XDMCP, host chooser
 ] [
 .B \-session
 .I session_program
+] [
+.B \-noconsolekit
 ]
 .SH DESCRIPTION
 .I Xdm
@@ -215,6 +217,10 @@ indicates the program to run as the session after the user 
has logged in.
 .IP \fB\-xrm\fP \fIresource_specification\fP
 Allows an arbitrary resource to be specified, as in most
 X Toolkit applications.
+.IP \fB\-noconsolekit\fP
+Specifies ``false'' as the value for the \fBDisplayManager.consoleKit\fP
+resource.
+This suppresses the session management using ConsoleKit.
 .SH RESOURCES
 At many stages the actions of
 .I xdm
diff --git a/xdm/resource.c b/xdm/resource.c
index ece4de3..ebcbeaa 100644
---