Bug 42441

Created attachment 73479 [details] [review]
hide not supported dbus-daemon commandline options

A related topic is to fix the related docs

Created attachment 73483 [details] [review]
updated dbus-daemon doc source

Comment on attachment 73479 [details] [review]
hide not supported dbus-daemon commandline options

Review of attachment 73479 [details] [review]:
-----------------------------------------------------------------

I don't really see the problem with having options that exist, but are not usable, on Windows; but if you feel strongly about this, the #ifdefs don't really hurt, I suppose...

How does this interact with Cygwin, which is both Unix and Windows? If I understand correctly, a Cygwin dbus-daemon.exe can still fork, and it might even be useful for it to do so - so the check should probably be for defined(DBUS_UNIX) rather than for !defined(DBUS_WIN).

::: bus/main.c
@@ +123,5 @@
>  #endif /* DBUS_UNIX */
>  
> +#ifdef DBUS_WIN
> +#define DBUS_USAGE_OPTIONS1 ""
> +#define DBUS_USAGE_OPTIONS2 ""

I'd prefer a single macro, DBUS_OS_DEPENDENT_OPTIONS or something. It's OK to put the options in a different order on Unix to facilitate that.

Even better, I think, would be something like this:

fprintf (stderr,
      DBUS_DAEMON_NAME
      " [--version]"
      " [--session]"
      /* ... more options here ... */
      " [--nopidfile]"
#ifdef DBUS_UNIX
      " [--fork|--nofork]"
      " [--systemd-activation]"
#endif
      "\n");

with no extra macros at all.

(In reply to comment #3)
> Comment on attachment 73479 [details] [review] [review]
> hide not supported dbus-daemon commandline options
> 
> Review of attachment 73479 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> I don't really see the problem with having options that exist, but are not
> usable, on Windows; but if you feel strongly about this, 

users will try this option, will get no result and creates bug reports, which generates useless work

> the #ifdefs don't
> really hurt, I suppose...
> 
> How does this interact with Cygwin, which is both Unix and Windows? 

cygwin is unix, so no problem here

> I'd prefer a single macro, DBUS_OS_DEPENDENT_OPTIONS or something. It's OK
> to put the options in a different order on Unix to facilitate that.
> 
> Even better, I think, would be something like this:
> 
> fprintf (stderr,
>       DBUS_DAEMON_NAME
>       " [--version]"
>       " [--session]"
>       /* ... more options here ... */
>       " [--nopidfile]"
> #ifdef DBUS_UNIX
>       " [--fork|--nofork]"
>       " [--systemd-activation]"
> #endif
>       "\n");
> 
yes, very much better, i will update the patch

Comment on attachment 73483 [details] [review]
updated dbus-daemon doc source

Review of attachment 73483 [details] [review]:
-----------------------------------------------------------------

::: cmake/bus/dbus-daemon.xml
@@ +4,1 @@
>  <!-- lifted from troff+man by doclifter -->

I don't really like having generated files checked-in to git. If XML is a better source format, I feel as though we should consider passing dbus-daemon.1.in through doclifter *once*, resulting in a doc/dbus-daemon.xml.in which is the new canonical source format, and having the Autotools build pass dbus-daemon.xml through xmltoman or docbook2man or docbook2x-man or something.

(Anyone have a favourite Docbook-to-man-page converter?)

For now, regenerating dbus-daemon.xml seems fine, though.

@@ +89,5 @@
>  In most contexts the configuration file already gets this
> +right, though.
> +<option>--nofork</option>
> +Force the message bus not to fork and become a daemon, even if
> +the configuration file specifies that it should.</para>

I assume you also want to add "This option is not available on Windows." to the source man page, and pass it back through doclifter?

(In reply to comment #4)
> > How does this interact with Cygwin, which is both Unix and Windows? 
> 
> cygwin is unix, so no problem here

Looks like I was mixing up libdbus with GLib. In GLib, Cygwin defines G_OS_UNIX, G_PLATFORM_WIN32 and G_WITH_CYGWIN (but not G_OS_WIN32); but in D-Bus there's no equivalent of G_PLATFORM_WIN32, so this doesn't apply.

Created attachment 73521 [details] [review]
hide not supported dbus-daemon commandline options

(In reply to comment #5)
> Comment on attachment 73483 [details] [review] [review]
> updated dbus-daemon doc source
> 
> Review of attachment 73483 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> ::: cmake/bus/dbus-daemon.xml
> @@ +4,1 @@
> >  <!-- lifted from troff+man by doclifter -->
> 
> I don't really like having generated files checked-in to git. If XML is a
> better source format, 

yes, all dbus man page docs could be generated from on source format (docbook)
which reduces dbus build dependencies. cmake already use xmlto to generate dbus-daemon.html and similar man pages from cmake/doc/dbus-daemon.xml. 

> I feel as though we should consider passing
> dbus-daemon.1.in through doclifter *once*, resulting in a
> doc/dbus-daemon.xml.in which is the new canonical source format, 

full agree

> and having the Autotools build pass dbus-daemon.xml through xmltoman or docbook2man or
> docbook2x-man or something.

xmlto has also a "man" format, see xmlto help page

> 
> (Anyone have a favourite Docbook-to-man-page converter?)
> 
> For now, regenerating dbus-daemon.xml seems fine, though.

> @@ +89,5 @@
> >  In most contexts the configuration file already gets this
> > +right, though.
> > +<option>--nofork</option>
> > +Force the message bus not to fork and become a daemon, even if
> > +the configuration file specifies that it should.</para>
> 
> I assume you also want to add "This option is not available on Windows." to
> the source man page, and pass it back through doclifter?

This is one possibility, I am thinking about to use conditional text in the docbook source (see http://www.sagehill.net/docbookxsl/Profiling.html for further details)

for example 

<sect1 os="win">
  <title>Installation on Windows</title>
  ...
<para os="win">blah, blah ... </para>
<para os="linux"> linux only stuff ... </para>

This let us keep all platform related docs together and give us a fine granulated control over what is included for which platform. 

With xsltproc I already produced os dependent html output
see http://www.sagehill.net/docbookxsl/AlternateText.html for the basics. 

Currently I have problems to call xmlto with these options. 

In case we like to start this migration yet, should be track the process in this bug or in a newly opened bug ?

(In reply to comment #8)
> xmlto has also a "man" format, see xmlto help page

Ah, ideal - we already use xmlto in Automake-land anyway, to get things like the D-Bus Specification, and we could drop the use of man2html too.

I wonder whether to get rid of the substitution of @EXPANDED_SYSCONFDIR@ as well - then it wouldn't have to be a .in, as far as I can see. It doesn't add much value, and assuming ${sysconfdir} is /etc is valid for 99% of installations on Unix, because D-Bus is the sort of thing you really really want to get from your distribution.

> > I assume you also want to add "This option is not available on Windows." to
> > the source man page, and pass it back through doclifter?
> 
> This is one possibility, I am thinking about to use conditional text in the
> docbook source

I'd rather avoid that. The man page documents everything dbus-daemon does, not just the bits that are relevant/useful on a particular platform - I think it's fine to have sections that are platform-specific.

> <sect1 os="win">
>   <title>Installation on Windows</title>

Man pages are the wrong place for installation instructions: by the time you get the man pages, you already have D-Bus installed :-)

> In case we like to start this migration yet, should be track the process in
> this bug or in a newly opened bug ?

A new bug, I think. Let's use this one for --help.

Comment on attachment 73521 [details] [review]
hide not supported dbus-daemon commandline options

Review of attachment 73521 [details] [review]:
-----------------------------------------------------------------

> Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>

Please don't add this until I have actually reviewed it. At the moment it isn't true :-)

(What I do is something like: fix bug; attach patch to bug; wait for someone to say "yes"; git commit --amend, add Reviewed-by; git push.)

::: bus/main.c
@@ +140,5 @@
> +      " [--introspect]"
> +      " [--address=ADDRESS]"
> +#ifdef DBUS_UNIX
> +      " [--systemd-activation]"
> +#endif

Move this up, or --fork and --nofork down, so we only need one #ifdef DBUS_UNIX block here?

Other than that, this looks good.

(In reply to comment #9)
> (In reply to comment #8)
> > xmlto has also a "man" format, see xmlto help page
> 
> Ah, ideal - we already use xmlto in Automake-land anyway, to get things like
> the D-Bus Specification, and we could drop the use of man2html too.
> 
> I wonder whether to get rid of the substitution of @EXPANDED_SYSCONFDIR@ as
> well - then it wouldn't have to be a .in, as far as I can see. It doesn't
> add much value, and assuming ${sysconfdir} is /etc is valid for 99% of
> installations on Unix, because D-Bus is the sort of thing you really really
> want to get from your distribution.
> 
> > > I assume you also want to add "This option is not available on Windows." to
> > > the source man page, and pass it back through doclifter?
> > 
> > This is one possibility, I am thinking about to use conditional text in the
> > docbook source
> 
> I'd rather avoid that. The man page documents everything dbus-daemon does,
> not just the bits that are relevant/useful on a particular platform - I
> think it's fine to have sections that are platform-specific.
> 
> > <sect1 os="win">
> >   <title>Installation on Windows</title>
> 
> Man pages are the wrong place for installation instructions: 

from http://dbus.freedesktop.org/doc/dbus-daemon.1.html
... The --session option is equivalent to "--config-file=/usr/local/etc/dbus-1/session.conf" and the --system option is equivalent to "--config-file=/usr/local/etc/dbus-1/system.conf". ... 

Aren't these installation related instructions ? They are platform specific too and they are using the wrong path 

> by the time you
> get the man pages, you already have D-Bus installed :-)
> 
> > In case we like to start this migration yet, should be track the process in
> > this bug or in a newly opened bug ?
> 
> A new bug, I think. Let's use this one for --help.

Will do so.

Created attachment 73522 [details] [review]
hide not supported dbus-daemon commandline options (update 2)

(In reply to comment #9)
> (In reply to comment #8)
> > xmlto has also a "man" format, see xmlto help page
> 
> Ah, ideal - we already use xmlto in Automake-land anyway, to get things like
> the D-Bus Specification, and we could drop the use of man2html too.
> 
> I wonder whether to get rid of the substitution of @EXPANDED_SYSCONFDIR@ as
> well - then it wouldn't have to be a .in, as far as I can see. It doesn't
> add much value, and assuming ${sysconfdir} is /etc is valid for 99% of
> installations on Unix, because D-Bus is the sort of thing you really really
> want to get from your distribution.

not for windows, there it should be <dbus-install-root>

(In reply to comment #11)
> from http://dbus.freedesktop.org/doc/dbus-daemon.1.html
> ... The --session option is equivalent to
> "--config-file=/usr/local/etc/dbus-1/session.conf" and the --system option
> is equivalent to "--config-file=/usr/local/etc/dbus-1/system.conf". ... 
> 
> Aren't these installation related instructions ? They are platform specific
> too and they are using the wrong path 

They're platform-specific, and indeed specific to a particular installation, but they don't describe how to install D-Bus (unlike, say, INSTALL).

And, yeah, /usr/local/etc is the default if you compile with no extra options (which distributions basically never do).

How about this for some installation-neutral wording which could go in the man page, avoiding having to do the dbus-daemon.1.in -> dbus-daemon.1 dance?

"""
The --session option is equivalent to using --config-file to select the standard configuration file session.conf provided with D-Bus. Similarly, the --system option is equivalent to using --config-file to select system.conf. These files are typically found in /etc/dbus-1 on Unix systems, or in the etc/dbus-1 subdirectory of the installation root on Windows systems.
"""

Created attachment 73574 [details] [review]
Added 'This options is not available on windows' note

Comment on attachment 73574 [details] [review]
Added 'This options is not available on windows' note

Review of attachment 73574 [details] [review]:
-----------------------------------------------------------------

This change will be OK once there is a single source file for the man page, but is not OK yet: any given file should either be generated or edited, not some mixture of the two.

(In reply to comment #16)
> Comment on attachment 73574 [details] [review] [review]
> Added 'This options is not available on windows' note
> 
> Review of attachment 73574 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> This change will be OK once there is a single source file for the man page,
> but is not OK yet: any given file should either be generated or edited, not
> some mixture of the two.

As I haven't edited any troff source, then we should get rid of troff sources immediatly :-) 

see Bug 59805

Comment on attachment 73522 [details] [review]
hide not supported dbus-daemon commandline options (update 2)

applied for 1.7.6, thanks

Comment on attachment 73483 [details] [review]
updated dbus-daemon doc source

rejected, fixed differently on another bug

Created attachment 81109 [details] [review]
dbus-daemon docs: note options that are unavailable on  Windows

Also reformat those options so --nofork is presented correctly
as a separate option.

---

Using footnotes for this was a nice idea, but it doesn't look good when viewed as a man page, and is potentially confusing: it could be interpreted as saying you should run "dbus-daemon --fork[1] --system" or something.

How about this?

(In reply to comment #20)
> Created attachment 81109 [details] [review] [review]
> dbus-daemon docs: note options that are unavailable on  Windows
> 
> Also reformat those options so --nofork is presented correctly
> as a separate option.
> 
> ---
> 
> Using footnotes for this was a nice idea, but it doesn't look good when
> viewed as a man page, and is potentially confusing: it could be interpreted
> as saying you should run "dbus-daemon --fork[1] --system" or something.
> 
> How about this?

You need to mark  more parts like this for example 

1. --------------------------------
... 
<fork>

If present, the bus daemon becomes a real daemon (forks into the background, etc.). This is generally used rather than the --fork command line option.

2. --------------------------------
<keep_umask>

If present, the bus daemon keeps its original umask when forking. This may be useful to avoid affecting the behavior of child processes.


3. --------------------------------

--system
Use the standard configuration file for the systemwide message bus. 

... 


4. --------------------------------

<servicehelper/>

<servicehelper/> specifies the setuid helper that is used to launch system daemons with an alternate user. Typically this should be the dbus-daemon-launch-helper executable in located in libexec.


<selinux>

The <selinux> element contains settings related to Security Enhanced Linux. More details below.

<associate>

An <associate> element appears below an <selinux> element and creates a mapping. Right now only one kind of association is possible:


   <associate own="org.freedesktop.Foobar" context="foo_t"/>
This means that if a connection asks to own the name "org.freedesktop.Foobar" then the source context will be the context of the connection and the target context will be "foo_t" - see the short discussion of SELinux below.

Note, the context here is the target context when requesting a name, NOT the context of the connection owning the name.

There's currently no way to set a default for owning any name, if we add this syntax it will look like:


   <associate own="*" context="foo_t"/>
If you find a reason this is useful, let the developers know. Right now the default will be the security context of the bus itself.

If two <associate> elements specify the same name, the element appearing later in the configuration file will be used.



SELinux

See http://www.nsa.gov/selinux/ for full details on SELinux. Some useful excerpts:

Every subject (process) and object (e.g. file, socket, IPC object, etc) in the system is assigned a collection of security attributes, known as a security context. A security context contains all of the security attributes associated with a particular subject or object that are relevant to the security policy.

In order to better encapsulate security contexts and to provide greater efficiency, the policy enforcement code of SELinux typically handles security identifiers (SIDs) rather than security contexts. A SID is an integer that is mapped by the security server to a security context at runtime.

When a security decision is required, the policy enforcement code passes a pair of SIDs (typically the SID of a subject and the SID of an object, but sometimes a pair of subject SIDs or a pair of object SIDs), and an object security class to the security server. The object security class indicates the kind of object, e.g. a process, a regular file, a directory, a TCP socket, etc.

Access decisions specify whether or not a permission is granted for a given pair of SIDs and class. Each object class has a set of associated permissions defined to control operations on objects with that class.

D-Bus performs SELinux security checks in two places.

First, any time a message is routed from one connection to another connection, the bus daemon will check permissions with the security context of the first connection as source, security context of the second connection as target, object class "dbus" and requested permission "send_msg".

If a security context is not available for a connection (impossible when using UNIX domain sockets), then the target context used is the context of the bus daemon itself. There is currently no way to change this default, because we're assuming that only UNIX domain sockets will be used to connect to the systemwide bus. If this changes, we'll probably add a way to set the default connection context.

Second, any time a connection asks to own a name, the bus daemon will check permissions with the security context of the connection as source, the security context specified for the name in the config file as target, object class "dbus" and requested permission "acquire_svc".

The security context for a bus name is specified with the <associate> element described earlier in this document. If a name has no security context associated in the configuration file, the security context of the bus daemon itself will be used.

------------------------------

Note that the refered SID's do not relates to the windows SID's 


Additional there are different wordings required for example: 


1. ---------------------------------
<unix>

<user>

The user account the daemon should run as, as either a username or a UID. If the daemon cannot change to this UID on startup, it will exit. If this element is not present, the daemon will not change or care about its UID.

The last <user> entry in the file "wins", the others are ignored.

</unix>

<win>
The user account the daemon should run as, as either a username or a security descriptor (SID). If the daemon cannot change to this SID on startup, it will exit. If this element is not present, the daemon will not change or care about its SID.

<!win>

2. ---------------------------------


<unix>
To run a separate test copy of the daemon, for example you might open a terminal and type:

  DBUS_VERBOSE=1 dbus-daemon --session --print-address
</unix>

<win>
To run a separate test copy of the daemon, for example you might open a command shell and type:

  set DBUS_VERBOSE=1 
  dbus-daemon --session --print-address
</win>

BTW: I still believe that supporting platform related parts makes sense and do not have any drawbacks.

(In reply to comment #21)
> You need to mark  more parts like this

I'd accept additional patches, but I can't spend much more time on this myself.

> <selinux>
> 
> The <selinux> element contains settings related to Security Enhanced Linux.

I would hope that it's reasonably obvious that Security Enhanced Linux features can only work on Linux :-)

> Note that the refered SID's do not relates to the windows SID's 

Sure. I would hope that this is obvious from context.

> BTW: I still believe that supporting platform related parts makes sense and
> do not have any drawbacks.

The drawback is "there are now (at least) two sets of documentation, I'll upload the Unix version when I do a release, and the Windows version will basically never be uploaded".

For dbus-launch, since its source code, usage and command-line syntax are completely different on Windows, I wouldn't object to a separate dbus-launch-win.{xml.in,html} if you want to write one?

-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/dbus/dbus/issues/58.