bug#71969: [PATCH] Support interactive D-Bus authentication

bug#71969: [PATCH] Support interactive D-Bus authentication

[Steven Allen via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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


When invoking D-Bus methods, let the user enable interactive
authorization by passing an :authenticate t parameter.  This makes it
possible to D-Bus methods that require polkit authorization.

Alternatively, we could allow interactive authorization unconditionally,
but I'd prefer to leave it up to the caller.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Support-interactive-D-Bus-authentication.patch --]
[-- Type: text/x-patch, Size: 6529 bytes --]

From fa996a3363e9bcefb547c2a587d55b279d44c5dd Mon Sep 17 00:00:00 2001
From: Steven Allen <steven@stebalien.com>
Date: Thu, 4 Jul 2024 20:45:07 +0200
Subject: [PATCH] Support interactive D-Bus authentication

When invoking D-Bus methods, let the user enable interactive
authorization by passing an :authenticate t parameter.  This makes it
possible to D-Bus methods that require polkit authorization.

* src/dbusbind.c (dbus-message-internal): Allow interactive
authorization by passing :authenticate t.
* lisp/net/dbus.el (dbus-call-method-asynchronously): Document the new
parameter.
* doc/misc/dbus.texi (Synchronous Methods, Asynchronous Methods):
Document the new parameter.
---
 doc/misc/dbus.texi | 12 ++++++++++--
 etc/NEWS           |  6 ++++++
 lisp/net/dbus.el   |  8 ++++++++
 src/dbusbind.c     | 26 ++++++++++++++++++++------
 4 files changed, 44 insertions(+), 8 deletions(-)

diff --git a/doc/misc/dbus.texi b/doc/misc/dbus.texi
index e5d867acd40..9bde8cbc76b 100644
--- a/doc/misc/dbus.texi
+++ b/doc/misc/dbus.texi
@@ -1208,7 +1208,7 @@ Synchronous Methods
 be called, and a reply message returning the resulting output
 parameters from the object.
 
-@defun dbus-call-method bus service path interface method &optional :timeout timeout &rest args
+@defun dbus-call-method bus service path interface method &optional :timeout timeout :authenticate auth &rest args
 @anchor{dbus-call-method}
 This function calls @var{method} on the D-Bus @var{bus}.  @var{bus} is
 either the keyword @code{:system} or the keyword @code{:session}.
@@ -1223,6 +1223,10 @@ Synchronous Methods
 call doesn't return in time, a D-Bus error is raised (@pxref{Errors
 and Events}).
 
+If the parameter @code{:authenticate} is given and the following
+@var{auth} is non-nil, the invoked method may interactively prompt the
+user for authorization.  The default is @code{nil}.
+
 The remaining arguments @var{args} are passed to @var{method} as
 arguments.  They are converted into D-Bus types as described in
 @ref{Type Conversion}.
@@ -1302,7 +1306,7 @@ Asynchronous Methods
 @cindex method calls, asynchronous
 @cindex asynchronous method calls
 
-@defun dbus-call-method-asynchronously bus service path interface method handler &optional :timeout timeout &rest args
+@defun dbus-call-method-asynchronously bus service path interface method handler &optional :timeout timeout :authenticate auth &rest args
 This function calls @var{method} on the D-Bus @var{bus}
 asynchronously.  @var{bus} is either the keyword @code{:system} or the
 keyword @code{:session}.
@@ -1321,6 +1325,10 @@ Asynchronous Methods
 no reply message in time, a D-Bus error is raised (@pxref{Errors and
 Events}).
 
+If the parameter @code{:authenticate} is given and the following
+@var{auth} is non-nil, the invoked method may interactively prompt the
+user for authorization.  The default is @code{nil}.
+
 The remaining arguments @var{args} are passed to @var{method} as
 arguments.  They are converted into D-Bus types as described in
 @ref{Type Conversion}.
diff --git a/etc/NEWS b/etc/NEWS
index 3d2b86cfb6a..fd6e3737eb8 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -79,6 +79,12 @@ levels that SHR cycles through when calling 'shr-zoom-image'.
 \f
 * Lisp Changes in Emacs 31.1
 
++++
+*** Support interactive D-Bus authentication
+A new ':authenticate t' parameter has been added to 'dbus-call-method'
+and 'dbus-call-method-asynchronously' to allow the invoked D-Bus method
+to interactively authenticate the user (e.g., via polkit).
+
 \f
 * Changes in Emacs 31.1 on Non-Free Operating Systems
 
diff --git a/lisp/net/dbus.el b/lisp/net/dbus.el
index dd5f0e88859..749a12b5368 100644
--- a/lisp/net/dbus.el
+++ b/lisp/net/dbus.el
@@ -297,6 +297,10 @@ dbus-call-method
 method call must return.  The default value is 25,000.  If the
 method call doesn't return in time, a D-Bus error is raised.
 
+If the parameter `:authenticate' is given and the following AUTH
+is non-nil, the invoked method may interactively prompt the user
+for authorization.  The default is nil.
+
 All other arguments ARGS are passed to METHOD as arguments.  They are
 converted into D-Bus types via the following rules:
 
@@ -427,6 +431,10 @@ dbus-call-method-asynchronously
 method call must return.  The default value is 25,000.  If the
 method call doesn't return in time, a D-Bus error is raised.
 
+If the parameter `:authenticate' is given and the following AUTH
+is non-nil, the invoked method may interactively prompt the user
+for authorization.  The default is nil.
+
 All other arguments ARGS are passed to METHOD as arguments.  They are
 converted into D-Bus types via the following rules:
 
diff --git a/src/dbusbind.c b/src/dbusbind.c
index 35ce03c7911..6037112cfe7 100644
--- a/src/dbusbind.c
+++ b/src/dbusbind.c
@@ -1314,7 +1314,7 @@ DEFUN ("dbus-message-internal", Fdbus_message_internal, Sdbus_message_internal,
 `dbus-call-method', `dbus-call-method-asynchronously':
   (dbus-message-internal
     dbus-message-type-method-call BUS SERVICE PATH INTERFACE METHOD HANDLER
-    &optional :timeout TIMEOUT &rest ARGS)
+    &optional :timeout TIMEOUT :authenticate AUTH &rest ARGS)
 
 `dbus-send-signal':
   (dbus-message-internal
@@ -1512,12 +1512,23 @@ DEFUN ("dbus-message-internal", Fdbus_message_internal, Sdbus_message_internal,
 	XD_SIGNAL1 (build_string ("Unable to create an error message"));
     }
 
-  /* Check for timeout parameter.  */
-  if ((count + 2 <= nargs) && EQ (args[count], QCtimeout))
+  while ((count + 2 <= nargs))
     {
-      CHECK_FIXNAT (args[count+1]);
-      timeout = min (XFIXNAT (args[count+1]), INT_MAX);
-      count = count+2;
+      /* Check for timeout parameter.  */
+      if (EQ (args[count], QCtimeout))
+        {
+          CHECK_FIXNAT (args[count+1]);
+          timeout = min (XFIXNAT (args[count+1]), INT_MAX);
+          count = count+2;
+	}
+      /* Check for authenticate parameter.  */
+      else if (EQ (args[count], QCauthenticate))
+        {
+	  dbus_message_set_allow_interactive_authorization
+	  (dmessage, NILP (args[count+1]) ? FALSE : TRUE);
+          count = count+2;
+	}
+      else break;
     }
 
   /* Initialize parameter list of message.  */
@@ -1895,6 +1906,9 @@ syms_of_dbusbind (void)
   /* Lisp symbol for method call timeout.  */
   DEFSYM (QCtimeout, ":timeout");
 
+  /* Lisp symbol for method interactive authentication.  */
+  DEFSYM (QCauthenticate, ":authenticate");
+
   /* Lisp symbols of D-Bus types.  */
   DEFSYM (QCbyte, ":byte");
   DEFSYM (QCboolean, ":boolean");
-- 
2.45.2

bug#71969: [PATCH] Support interactive D-Bus authentication

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Steven Allen via "Bug reports for GNU Emacs, the Swiss army knife of
text editors" <bug-gnu-emacs@gnu.org> writes:

Hi Steven,

> When invoking D-Bus methods, let the user enable interactive
> authorization by passing an :authenticate t parameter.  This makes it
> possible to D-Bus methods that require polkit authorization.

Thanks for the patch, it looks interesting. Since I've never used D-Bus
interactive authentication, could you pls provide an example that I
could see it in action? I'm running Fedora 40, if that matters.

Bonus point, if you could add a test to dbus-tests.el.

> Alternatively, we could allow interactive authorization unconditionally,
> but I'd prefer to leave it up to the caller.

Agreed.

> From fa996a3363e9bcefb547c2a587d55b279d44c5dd Mon Sep 17 00:00:00 2001
> From: Steven Allen <steven@stebalien.com>
> Date: Thu, 4 Jul 2024 20:45:07 +0200
> Subject: [PATCH] Support interactive D-Bus authentication
>
> When invoking D-Bus methods, let the user enable interactive
> authorization by passing an :authenticate t parameter.  This makes it
> possible to D-Bus methods that require polkit authorization.

Do you mean authorization or authentication? I guess authorization. Pls
change all places consistently.

> +++ b/src/dbusbind.c
> @@ -1512,12 +1512,23 @@ DEFUN ("dbus-message-internal", Fdbus_message_internal, Sdbus_message_internal,
>  	XD_SIGNAL1 (build_string ("Unable to create an error message"));
>      }
>
> -  /* Check for timeout parameter.  */
> -  if ((count + 2 <= nargs) && EQ (args[count], QCtimeout))
> +  while ((count + 2 <= nargs))
>      {
> -      CHECK_FIXNAT (args[count+1]);
> -      timeout = min (XFIXNAT (args[count+1]), INT_MAX);
> -      count = count+2;
> +      /* Check for timeout parameter.  */
> +      if (EQ (args[count], QCtimeout))
> +        {
> +          CHECK_FIXNAT (args[count+1]);
> +          timeout = min (XFIXNAT (args[count+1]), INT_MAX);
> +          count = count+2;
> +	}
> +      /* Check for authenticate parameter.  */
> +      else if (EQ (args[count], QCauthenticate))
> +        {
> +	  dbus_message_set_allow_interactive_authorization
> +	  (dmessage, NILP (args[count+1]) ? FALSE : TRUE);
> +          count = count+2;
> +	}
> +      else break;

I guess, we shall check only in case the message type is
DBUS_MESSAGE_TYPE_METHOD_CALL. Otherwise, if there is a :timeout or
:authorize keyword, we shall raise an error.

Furthermore, does exist dbus_message_set_allow_interactive_authorization
"since ever"? Or shall we check for the existence in configure.ac, like
we do it with other dbus_* functions?

Best regards, Michael.

bug#71969: [PATCH] Support interactive D-Bus authentication

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Michael Albinus <michael.albinus@gmx.de> writes:

Hi Steven,

> Furthermore, does exist dbus_message_set_allow_interactive_authorization
> "since ever"? Or shall we check for the existence in configure.ac, like
> we do it with other dbus_* functions?

I've just checked the dbus git repo.
dbus_message_set_allow_interactive_authorization was added in
D-Bus 1.8.10, so we need a check in configure.ac.

Best regards, Michael.

bug#71969: [PATCH] Support interactive D-Bus authentication

[Eli Zaretskii]

> Cc: steven@stebalien.com
> Date: Sat, 06 Jul 2024 16:53:50 +0200
> From:  Michael Albinus via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> > Furthermore, does exist dbus_message_set_allow_interactive_authorization
> > "since ever"? Or shall we check for the existence in configure.ac, like
> > we do it with other dbus_* functions?
> 
> I've just checked the dbus git repo.
> dbus_message_set_allow_interactive_authorization was added in
> D-Bus 1.8.10, so we need a check in configure.ac.

Is this really a build-time condition, not a run-time condition?  Can
we know in advance which version of D-Bus will be used at run time?

bug#71969: [PATCH] Support interactive D-Bus authentication

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

Hi Eli,

>> > Furthermore, does exist dbus_message_set_allow_interactive_authorization
>> > "since ever"? Or shall we check for the existence in configure.ac, like
>> > we do it with other dbus_* functions?
>>
>> I've just checked the dbus git repo.
>> dbus_message_set_allow_interactive_authorization was added in
>> D-Bus 1.8.10, so we need a check in configure.ac.
>
> Is this really a build-time condition, not a run-time condition?  Can
> we know in advance which version of D-Bus will be used at run time?

Well, we must call this function, or not, in dbusbind.c. Of course it
depends on the libdbus-1.so version whether it is available at
runtime. But we must know already at build time, whether the function is
offered via /usr/include/dbus-1.0/dbus/dbus-message.h. Otherwise, the
function call would result in a compiler error.

So it is a build time dependency. We do a similar check for other D-Bus
functions in configure.ac, see this snippet:

--8<---------------cut here---------------start------------->8---
     AC_CHECK_FUNCS([dbus_watch_get_unix_fd \
		    dbus_type_is_valid \
		    dbus_validate_bus_name \
                    dbus_validate_path \
		    dbus_validate_interface \
		    dbus_validate_member])
--8<---------------cut here---------------end--------------->8---

And in dbusbind.c, we have then code like

--8<---------------cut here---------------start------------->8---
#if HAVE_DBUS_WATCH_GET_UNIX_FD
  /* TODO: Reverse these on w32, which prefers the opposite.  */
  int fd = dbus_watch_get_unix_fd (watch);
  if (fd == -1)
    fd = dbus_watch_get_socket (watch);
#else
  int fd = dbus_watch_get_fd (watch);
#endif
--8<---------------cut here---------------end--------------->8---

Of course there is the risk, that Emacs is compiled with a function call
(based on an include file), which does not exist at runtime, because the
Emacs program runs somewhere else. OTOH, D-Bus 1.8.10 was tagged in the
D-Bus git on "Thu Nov 6 15:39:51 2014 +0000". The likelihood, that we run
into problems, is small, and the check in configure.ac is just "to be
sure".

Best regards, Michael.

bug#71969: [PATCH] Support interactive D-Bus authentication

[Steven Allen via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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


I've attached a patch that addresses the feedback so far:

1. Defines HAVE_DBUS_MESSAGE_SET_ALLOW_INTERACTIVE_AUTHORIZATION and
   uses it.
2. Renames :authenticate to :authorize for consistency.
3. Signals an error when either :timeout or :authorize are passed when
   not invoking a method.

Remaining questions:

1. I'm not sure if :authorize is quite correct either. Really, the key
part is that it allows /interactive/ authorization. I wonder if
:interactive-authorization or :interactive might be better (although
they're kind of long).
2. Am I correctly signaling the error? I just copied that code from
other parts of debusbind.c.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Support-interactive-D-Bus-authorization.patch --]
[-- Type: text/x-patch, Size: 7921 bytes --]

From 14a6c82848ebc02e98ca76aa8f7465209960c227 Mon Sep 17 00:00:00 2001
From: Steven Allen <steven@stebalien.com>
Date: Thu, 4 Jul 2024 20:45:07 +0200
Subject: [PATCH] Support interactive D-Bus authorization

When invoking D-Bus methods, let the user enable interactive
authorization by passing an :authorize t parameter.  This makes it
possible to D-Bus methods that require polkit authorization.

* src/dbusbind.c (dbus-message-internal): Allow interactive
authorization by passing :authorize t.
* lisp/net/dbus.el (dbus-call-method-asynchronously): Document the new
parameter.
* doc/misc/dbus.texi (Synchronous Methods, Asynchronous Methods):
Document the new parameter.
* configure.ac (HAVE_DBUS_MESSAGE_SET_ALLOW_INTERACTIVE_AUTHORIZATION):
Set a new variable if
`dbus_message_set_allow_interactive_authorization' is available.
---
 configure.ac       |  5 ++++-
 doc/misc/dbus.texi | 12 ++++++++++--
 etc/NEWS           |  6 ++++++
 lisp/net/dbus.el   |  8 ++++++++
 src/dbusbind.c     | 37 +++++++++++++++++++++++++++++++------
 5 files changed, 59 insertions(+), 9 deletions(-)

diff --git a/configure.ac b/configure.ac
index 909f5786c9a..ee2ef1c60fb 100644
--- a/configure.ac
+++ b/configure.ac
@@ -3943,6 +3943,8 @@ AC_DEFUN
      dnl dbus_watch_get_unix_fd has been introduced in D-Bus 1.1.1.
      dnl dbus_type_is_valid and dbus_validate_* have been introduced in
      dnl D-Bus 1.5.12.
+     dnl dbus_message_set_allow_interactive_authorization was introduced
+     dnl in D-Bus 1.8.10.
      OLD_LIBS=$LIBS
      LIBS="$LIBS $DBUS_LIBS"
      AC_CHECK_FUNCS([dbus_watch_get_unix_fd \
@@ -3950,7 +3952,8 @@ AC_DEFUN
 		    dbus_validate_bus_name \
                     dbus_validate_path \
 		    dbus_validate_interface \
-		    dbus_validate_member])
+		    dbus_validate_member \
+                    dbus_message_set_allow_interactive_authorization])
      LIBS=$OLD_LIBS
      DBUS_OBJ=dbusbind.o
    fi
diff --git a/doc/misc/dbus.texi b/doc/misc/dbus.texi
index e5d867acd40..6c02546508a 100644
--- a/doc/misc/dbus.texi
+++ b/doc/misc/dbus.texi
@@ -1208,7 +1208,7 @@ Synchronous Methods
 be called, and a reply message returning the resulting output
 parameters from the object.
 
-@defun dbus-call-method bus service path interface method &optional :timeout timeout &rest args
+@defun dbus-call-method bus service path interface method &optional :timeout timeout :authorize auth &rest args
 @anchor{dbus-call-method}
 This function calls @var{method} on the D-Bus @var{bus}.  @var{bus} is
 either the keyword @code{:system} or the keyword @code{:session}.
@@ -1223,6 +1223,10 @@ Synchronous Methods
 call doesn't return in time, a D-Bus error is raised (@pxref{Errors
 and Events}).
 
+If the parameter @code{:authorize} is given and the following
+@var{auth} is non-nil, the invoked method may interactively prompt the
+user for authorization.  The default is @code{nil}.
+
 The remaining arguments @var{args} are passed to @var{method} as
 arguments.  They are converted into D-Bus types as described in
 @ref{Type Conversion}.
@@ -1302,7 +1306,7 @@ Asynchronous Methods
 @cindex method calls, asynchronous
 @cindex asynchronous method calls
 
-@defun dbus-call-method-asynchronously bus service path interface method handler &optional :timeout timeout &rest args
+@defun dbus-call-method-asynchronously bus service path interface method handler &optional :timeout timeout :authorize auth &rest args
 This function calls @var{method} on the D-Bus @var{bus}
 asynchronously.  @var{bus} is either the keyword @code{:system} or the
 keyword @code{:session}.
@@ -1321,6 +1325,10 @@ Asynchronous Methods
 no reply message in time, a D-Bus error is raised (@pxref{Errors and
 Events}).
 
+If the parameter @code{:authorize} is given and the following
+@var{auth} is non-nil, the invoked method may interactively prompt the
+user for authorization.  The default is @code{nil}.
+
 The remaining arguments @var{args} are passed to @var{method} as
 arguments.  They are converted into D-Bus types as described in
 @ref{Type Conversion}.
diff --git a/etc/NEWS b/etc/NEWS
index 3d2b86cfb6a..d6128cc078e 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -79,6 +79,12 @@ levels that SHR cycles through when calling 'shr-zoom-image'.
 \f
 * Lisp Changes in Emacs 31.1
 
++++
+*** Support interactive D-Bus authorization
+A new ':authorization t' parameter has been added to 'dbus-call-method'
+and 'dbus-call-method-asynchronously' to allow the user to interactively
+authorize the invoked D-Bus method (e.g., via polkit).
+
 \f
 * Changes in Emacs 31.1 on Non-Free Operating Systems
 
diff --git a/lisp/net/dbus.el b/lisp/net/dbus.el
index dd5f0e88859..d526423e089 100644
--- a/lisp/net/dbus.el
+++ b/lisp/net/dbus.el
@@ -297,6 +297,10 @@ dbus-call-method
 method call must return.  The default value is 25,000.  If the
 method call doesn't return in time, a D-Bus error is raised.
 
+If the parameter `:authorize' is given and the following AUTH
+is non-nil, the invoked method may interactively prompt the user
+for authorization.  The default is nil.
+
 All other arguments ARGS are passed to METHOD as arguments.  They are
 converted into D-Bus types via the following rules:
 
@@ -427,6 +431,10 @@ dbus-call-method-asynchronously
 method call must return.  The default value is 25,000.  If the
 method call doesn't return in time, a D-Bus error is raised.
 
+If the parameter `:authorize' is given and the following AUTH
+is non-nil, the invoked method may interactively prompt the user
+for authorization.  The default is nil.
+
 All other arguments ARGS are passed to METHOD as arguments.  They are
 converted into D-Bus types via the following rules:
 
diff --git a/src/dbusbind.c b/src/dbusbind.c
index 35ce03c7911..827411644ec 100644
--- a/src/dbusbind.c
+++ b/src/dbusbind.c
@@ -1314,7 +1314,7 @@ DEFUN ("dbus-message-internal", Fdbus_message_internal, Sdbus_message_internal,
 `dbus-call-method', `dbus-call-method-asynchronously':
   (dbus-message-internal
     dbus-message-type-method-call BUS SERVICE PATH INTERFACE METHOD HANDLER
-    &optional :timeout TIMEOUT &rest ARGS)
+    &optional :timeout TIMEOUT :authorize AUTH &rest ARGS)
 
 `dbus-send-signal':
   (dbus-message-internal
@@ -1512,12 +1512,34 @@ DEFUN ("dbus-message-internal", Fdbus_message_internal, Sdbus_message_internal,
 	XD_SIGNAL1 (build_string ("Unable to create an error message"));
     }
 
-  /* Check for timeout parameter.  */
-  if ((count + 2 <= nargs) && EQ (args[count], QCtimeout))
+  while ((count + 2 <= nargs))
     {
-      CHECK_FIXNAT (args[count+1]);
-      timeout = min (XFIXNAT (args[count+1]), INT_MAX);
-      count = count+2;
+      /* Check for timeout parameter.  */
+      if (EQ (args[count], QCtimeout))
+        {
+	  if (mtype != DBUS_MESSAGE_TYPE_METHOD_CALL)
+	    XD_SIGNAL2 (build_string (":timeout is only supported on method calls"), bus);
+
+          CHECK_FIXNAT (args[count+1]);
+          timeout = min (XFIXNAT (args[count+1]), INT_MAX);
+          count = count+2;
+	}
+      /* Check for authorize parameter.  */
+      else if (EQ (args[count], QCauthorize))
+        {
+	  if (mtype != DBUS_MESSAGE_TYPE_METHOD_CALL)
+	    XD_SIGNAL2 (build_string (":authorize is only supported on method calls"), bus);
+
+	  /* Ignore this keyword if unsupported. */
+	  #ifdef HAVE_DBUS_MESSAGE_SET_ALLOW_INTERACTIVE_AUTHORIZATION
+	  dbus_message_set_allow_interactive_authorization
+	  (dmessage, NILP (args[count+1]) ? FALSE : TRUE);
+	  #endif
+
+          count = count+2;
+	}
+      else break;
+
     }
 
   /* Initialize parameter list of message.  */
@@ -1895,6 +1917,9 @@ syms_of_dbusbind (void)
   /* Lisp symbol for method call timeout.  */
   DEFSYM (QCtimeout, ":timeout");
 
+  /* Lisp symbol for method interactive authorization.  */
+  DEFSYM (QCauthorize, ":authorize");
+
   /* Lisp symbols of D-Bus types.  */
   DEFSYM (QCbyte, ":byte");
   DEFSYM (QCboolean, ":boolean");
-- 
2.45.2

bug#71969: [PATCH] Support interactive D-Bus authentication

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Steven Allen <steven@stebalien.com> writes:

Hi Steven,

> I've attached a patch that addresses the feedback so far:
>
> 1. Defines HAVE_DBUS_MESSAGE_SET_ALLOW_INTERACTIVE_AUTHORIZATION and
>    uses it.
> 2. Renames :authenticate to :authorize for consistency.
> 3. Signals an error when either :timeout or :authorize are passed when
>    not invoking a method.

Thanks!

> Remaining questions:
>
> 1. I'm not sure if :authorize is quite correct either. Really, the key
> part is that it allows /interactive/ authorization. I wonder if
> :interactive-authorization or :interactive might be better (although
> they're kind of long).

I believe :authorize is OK. In the docstrings as well as in the D-Bus
manual, interactive authorization is mentioned, so a user shall know
what's about.

> 2. Am I correctly signaling the error? I just copied that code from
> other parts of debusbind.c.

I guess the better call would be

--8<---------------cut here---------------start------------->8---
	    XD_SIGNAL1 (build_string (":timeout is only supported on method calls"));
--8<---------------cut here---------------end--------------->8---

The bus argument isn't needed.

Furthermore, you haven't given an example. I really would like to see
how it works in practice.

Some remarks:

> --- a/doc/misc/dbus.texi
> +++ b/doc/misc/dbus.texi
> @@ -1223,6 +1223,10 @@ Synchronous Methods
>  call doesn't return in time, a D-Bus error is raised (@pxref{Errors
>  and Events}).
>
> +If the parameter @code{:authorize} is given and the following
> +@var{auth} is non-nil, the invoked method may interactively prompt the

non-@code{nil}

> @@ -1321,6 +1325,10 @@ Asynchronous Methods
>  no reply message in time, a D-Bus error is raised (@pxref{Errors and
>  Events}).
>
> +If the parameter @code{:authorize} is given and the following
> +@var{auth} is non-nil, the invoked method may interactively prompt the

ditto

> --- a/etc/NEWS
> +++ b/etc/NEWS
> @@ -79,6 +79,12 @@ levels that SHR cycles through when calling 'shr-zoom-image'.
>  \f
>  * Lisp Changes in Emacs 31.1
>
> ++++
> +*** Support interactive D-Bus authorization

Please add a trailing period.

> +A new ':authorization t' parameter has been added to 'dbus-call-method'

':authorize t'

> --- a/src/dbusbind.c
> +++ b/src/dbusbind.c
> @@ -1512,12 +1512,34 @@ DEFUN ("dbus-message-internal", Fdbus_message_internal, Sdbus_message_internal,
> +	    XD_SIGNAL2 (build_string (":timeout is only supported on method calls"), bus);

XD_SIGNAL1

> +	    XD_SIGNAL2 (build_string (":authorize is only supported on method calls"), bus);

XD_SIGNAL1

> +	  /* Ignore this keyword if unsupported. */
> +	  #ifdef HAVE_DBUS_MESSAGE_SET_ALLOW_INTERACTIVE_AUTHORIZATION
> +	  dbus_message_set_allow_interactive_authorization
> +	  (dmessage, NILP (args[count+1]) ? FALSE : TRUE);
> +	  #endif

#ifdef end #endif shall start in column 1. Futhermore, we need an #else
clause. There shall be an error or a warning, that :authorize is not supported.

Best regards, Michael.

bug#71969: [PATCH] Support interactive D-Bus authentication

[Steven Allen via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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


Michael Albinus <michael.albinus@gmx.de> writes:
> Steven Allen <steven@stebalien.com> writes:
>
> Hi Steven,
>
>> I've attached a patch that addresses the feedback so far:
>>
>> 1. Defines HAVE_DBUS_MESSAGE_SET_ALLOW_INTERACTIVE_AUTHORIZATION and
>>    uses it.
>> 2. Renames :authenticate to :authorize for consistency.
>> 3. Signals an error when either :timeout or :authorize are passed when
>>    not invoking a method.
>
> Thanks!
>
>> Remaining questions:
>>
>> 1. I'm not sure if :authorize is quite correct either. Really, the key
>> part is that it allows /interactive/ authorization. I wonder if
>> :interactive-authorization or :interactive might be better (although
>> they're kind of long).
>
> I believe :authorize is OK. In the docstrings as well as in the D-Bus
> manual, interactive authorization is mentioned, so a user shall know
> what's about.

Hm, it's still bugging me. We're _not_ authorizing the request, we're
telling D-Bus that it's ok to ask the user if they want to authorize it.
I'm hoping the example below will make this clearer.

>> 2. Am I correctly signaling the error? I just copied that code from
>> other parts of debusbind.c.
>
> I guess the better call would be
>
> --8<---------------cut here---------------start------------->8---
> 	    XD_SIGNAL1 (build_string (":timeout is only supported on method calls"));
> --8<---------------cut here---------------end--------------->8---
>
> The bus argument isn't needed.
>
> Furthermore, you haven't given an example. I really would like to see
> how it works in practice.

Sorry about that. To restart the bluetooth service, execute:

    (dbus-call-method
     :system
     "org.freedesktop.systemd1" "/org/freedesktop/systemd1"
     "org.freedesktop.systemd1.Manager" "RestartUnit"
     :authorize t
     "bluetooth.service" "replace")

Assuming you have a polkit agent running (most DEs will run one by
default, but agents like mate-polkit work pretty well standalone),
you'll be prompted to authorize the operation and the bluetooth service
will be restarted.

> Some remarks:
>
>> --- a/doc/misc/dbus.texi
>> +++ b/doc/misc/dbus.texi
>> @@ -1223,6 +1223,10 @@ Synchronous Methods
>>  call doesn't return in time, a D-Bus error is raised (@pxref{Errors
>>  and Events}).
>>
>> +If the parameter @code{:authorize} is given and the following
>> +@var{auth} is non-nil, the invoked method may interactively prompt the
>
> non-@code{nil}
>
>> @@ -1321,6 +1325,10 @@ Asynchronous Methods
>>  no reply message in time, a D-Bus error is raised (@pxref{Errors and
>>  Events}).
>>
>> +If the parameter @code{:authorize} is given and the following
>> +@var{auth} is non-nil, the invoked method may interactively prompt the
>
> ditto
>

Done and done (the info manuals are pretty inconsistent in this regard...).

>> --- a/etc/NEWS
>> +++ b/etc/NEWS
>> @@ -79,6 +79,12 @@ levels that SHR cycles through when calling 'shr-zoom-image'.
>>  \f
>>  * Lisp Changes in Emacs 31.1
>>
>> ++++
>> +*** Support interactive D-Bus authorization
>
> Please add a trailing period.
>
>> +A new ':authorization t' parameter has been added to 'dbus-call-method'
>
> ':authorize t'
>

done and done.

>> --- a/src/dbusbind.c
>> +++ b/src/dbusbind.c
>> @@ -1512,12 +1512,34 @@ DEFUN ("dbus-message-internal", Fdbus_message_internal, Sdbus_message_internal,
>> +	    XD_SIGNAL2 (build_string (":timeout is only supported on method calls"), bus);
>
> XD_SIGNAL1

Ah... I was wondering about the difference between the different
signals. I didn't even notice the bus argument... Thanks!

>> +	    XD_SIGNAL2 (build_string (":authorize is only supported on method calls"), bus);
>
> XD_SIGNAL1
>
>> +	  /* Ignore this keyword if unsupported. */
>> +	  #ifdef HAVE_DBUS_MESSAGE_SET_ALLOW_INTERACTIVE_AUTHORIZATION
>> +	  dbus_message_set_allow_interactive_authorization
>> +	  (dmessage, NILP (args[count+1]) ? FALSE : TRUE);
>> +	  #endif
>
> #ifdef end #endif shall start in column 1. Futhermore, we need an #else
> clause. There shall be an error or a warning, that :authorize is not supported.

I'm going to disagree on this last point. The flag is specifying whether
or not the D-Bus is _allowed_ to ask the user to ask the user to
authorize requests which can fail for multiple reasons anyways (e.g., if
no polkit agent is running, the user rejects the interactive
authorization, etc.).

If authorization is required and wasn't possible for some reason,
D-Bus will return an error to the user anyways. So the user will get
their warning either way _if_ something actually goes wrong.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Support-interactive-D-Bus-authorization.patch --]
[-- Type: text/x-patch, Size: 7916 bytes --]

From 62ea28d5d3b34ddd83bdcf5357ebfa0b24d8688e Mon Sep 17 00:00:00 2001
From: Steven Allen <steven@stebalien.com>
Date: Thu, 4 Jul 2024 20:45:07 +0200
Subject: [PATCH] Support interactive D-Bus authorization

When invoking D-Bus methods, let the user enable interactive
authorization by passing an :authorize t parameter.  This makes it
possible to D-Bus methods that require polkit authorization.

* src/dbusbind.c (dbus-message-internal): Allow interactive
authorization by passing :authorize t.
* lisp/net/dbus.el (dbus-call-method-asynchronously): Document the new
parameter.
* doc/misc/dbus.texi (Synchronous Methods, Asynchronous Methods):
Document the new parameter.
* configure.ac (HAVE_DBUS_MESSAGE_SET_ALLOW_INTERACTIVE_AUTHORIZATION):
Set a new variable if
`dbus_message_set_allow_interactive_authorization' is available.
---
 configure.ac       |  5 ++++-
 doc/misc/dbus.texi | 12 ++++++++++--
 etc/NEWS           |  6 ++++++
 lisp/net/dbus.el   |  8 ++++++++
 src/dbusbind.c     | 37 +++++++++++++++++++++++++++++++------
 5 files changed, 59 insertions(+), 9 deletions(-)

diff --git a/configure.ac b/configure.ac
index 909f5786c9a..ee2ef1c60fb 100644
--- a/configure.ac
+++ b/configure.ac
@@ -3943,6 +3943,8 @@ AC_DEFUN
      dnl dbus_watch_get_unix_fd has been introduced in D-Bus 1.1.1.
      dnl dbus_type_is_valid and dbus_validate_* have been introduced in
      dnl D-Bus 1.5.12.
+     dnl dbus_message_set_allow_interactive_authorization was introduced
+     dnl in D-Bus 1.8.10.
      OLD_LIBS=$LIBS
      LIBS="$LIBS $DBUS_LIBS"
      AC_CHECK_FUNCS([dbus_watch_get_unix_fd \
@@ -3950,7 +3952,8 @@ AC_DEFUN
 		    dbus_validate_bus_name \
                     dbus_validate_path \
 		    dbus_validate_interface \
-		    dbus_validate_member])
+		    dbus_validate_member \
+                    dbus_message_set_allow_interactive_authorization])
      LIBS=$OLD_LIBS
      DBUS_OBJ=dbusbind.o
    fi
diff --git a/doc/misc/dbus.texi b/doc/misc/dbus.texi
index e5d867acd40..46a666084bb 100644
--- a/doc/misc/dbus.texi
+++ b/doc/misc/dbus.texi
@@ -1208,7 +1208,7 @@ Synchronous Methods
 be called, and a reply message returning the resulting output
 parameters from the object.
 
-@defun dbus-call-method bus service path interface method &optional :timeout timeout &rest args
+@defun dbus-call-method bus service path interface method &optional :timeout timeout :authorize auth &rest args
 @anchor{dbus-call-method}
 This function calls @var{method} on the D-Bus @var{bus}.  @var{bus} is
 either the keyword @code{:system} or the keyword @code{:session}.
@@ -1223,6 +1223,10 @@ Synchronous Methods
 call doesn't return in time, a D-Bus error is raised (@pxref{Errors
 and Events}).
 
+If the parameter @code{:authorize} is given and the following
+@var{auth} is non-@code{nil}, the invoked method may interactively
+prompt the user for authorization.  The default is @code{nil}.
+
 The remaining arguments @var{args} are passed to @var{method} as
 arguments.  They are converted into D-Bus types as described in
 @ref{Type Conversion}.
@@ -1302,7 +1306,7 @@ Asynchronous Methods
 @cindex method calls, asynchronous
 @cindex asynchronous method calls
 
-@defun dbus-call-method-asynchronously bus service path interface method handler &optional :timeout timeout &rest args
+@defun dbus-call-method-asynchronously bus service path interface method handler &optional :timeout timeout :authorize auth &rest args
 This function calls @var{method} on the D-Bus @var{bus}
 asynchronously.  @var{bus} is either the keyword @code{:system} or the
 keyword @code{:session}.
@@ -1321,6 +1325,10 @@ Asynchronous Methods
 no reply message in time, a D-Bus error is raised (@pxref{Errors and
 Events}).
 
+If the parameter @code{:authorize} is given and the following
+@var{auth} is non-@code{nil}, the invoked method may interactively
+prompt the user for authorization.  The default is @code{nil}.
+
 The remaining arguments @var{args} are passed to @var{method} as
 arguments.  They are converted into D-Bus types as described in
 @ref{Type Conversion}.
diff --git a/etc/NEWS b/etc/NEWS
index 3d2b86cfb6a..4705b28238c 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -79,6 +79,12 @@ levels that SHR cycles through when calling 'shr-zoom-image'.
 \f
 * Lisp Changes in Emacs 31.1
 
++++
+*** Support interactive D-Bus authorization.
+A new ':authorize t' parameter has been added to 'dbus-call-method'
+and 'dbus-call-method-asynchronously' to allow the user to interactively
+authorize the invoked D-Bus method (e.g., via polkit).
+
 \f
 * Changes in Emacs 31.1 on Non-Free Operating Systems
 
diff --git a/lisp/net/dbus.el b/lisp/net/dbus.el
index dd5f0e88859..d526423e089 100644
--- a/lisp/net/dbus.el
+++ b/lisp/net/dbus.el
@@ -297,6 +297,10 @@ dbus-call-method
 method call must return.  The default value is 25,000.  If the
 method call doesn't return in time, a D-Bus error is raised.
 
+If the parameter `:authorize' is given and the following AUTH
+is non-nil, the invoked method may interactively prompt the user
+for authorization.  The default is nil.
+
 All other arguments ARGS are passed to METHOD as arguments.  They are
 converted into D-Bus types via the following rules:
 
@@ -427,6 +431,10 @@ dbus-call-method-asynchronously
 method call must return.  The default value is 25,000.  If the
 method call doesn't return in time, a D-Bus error is raised.
 
+If the parameter `:authorize' is given and the following AUTH
+is non-nil, the invoked method may interactively prompt the user
+for authorization.  The default is nil.
+
 All other arguments ARGS are passed to METHOD as arguments.  They are
 converted into D-Bus types via the following rules:
 
diff --git a/src/dbusbind.c b/src/dbusbind.c
index 35ce03c7911..67a1a30dc55 100644
--- a/src/dbusbind.c
+++ b/src/dbusbind.c
@@ -1314,7 +1314,7 @@ DEFUN ("dbus-message-internal", Fdbus_message_internal, Sdbus_message_internal,
 `dbus-call-method', `dbus-call-method-asynchronously':
   (dbus-message-internal
     dbus-message-type-method-call BUS SERVICE PATH INTERFACE METHOD HANDLER
-    &optional :timeout TIMEOUT &rest ARGS)
+    &optional :timeout TIMEOUT :authorize AUTH &rest ARGS)
 
 `dbus-send-signal':
   (dbus-message-internal
@@ -1512,12 +1512,34 @@ DEFUN ("dbus-message-internal", Fdbus_message_internal, Sdbus_message_internal,
 	XD_SIGNAL1 (build_string ("Unable to create an error message"));
     }
 
-  /* Check for timeout parameter.  */
-  if ((count + 2 <= nargs) && EQ (args[count], QCtimeout))
+  while ((count + 2 <= nargs))
     {
-      CHECK_FIXNAT (args[count+1]);
-      timeout = min (XFIXNAT (args[count+1]), INT_MAX);
-      count = count+2;
+      /* Check for timeout parameter.  */
+      if (EQ (args[count], QCtimeout))
+        {
+	  if (mtype != DBUS_MESSAGE_TYPE_METHOD_CALL)
+	    XD_SIGNAL1 (build_string (":timeout is only supported on method calls"));
+
+          CHECK_FIXNAT (args[count+1]);
+          timeout = min (XFIXNAT (args[count+1]), INT_MAX);
+          count = count+2;
+	}
+      /* Check for authorize parameter.  */
+      else if (EQ (args[count], QCauthorize))
+        {
+	  if (mtype != DBUS_MESSAGE_TYPE_METHOD_CALL)
+	    XD_SIGNAL1 (build_string (":authorize is only supported on method calls"));
+
+	  /* Ignore this keyword if unsupported. */
+#ifdef HAVE_DBUS_MESSAGE_SET_ALLOW_INTERACTIVE_AUTHORIZATION
+	  dbus_message_set_allow_interactive_authorization
+	  (dmessage, NILP (args[count+1]) ? FALSE : TRUE);
+#endif
+
+          count = count+2;
+	}
+      else break;
+
     }
 
   /* Initialize parameter list of message.  */
@@ -1895,6 +1917,9 @@ syms_of_dbusbind (void)
   /* Lisp symbol for method call timeout.  */
   DEFSYM (QCtimeout, ":timeout");
 
+  /* Lisp symbol for method interactive authorization.  */
+  DEFSYM (QCauthorize, ":authorize");
+
   /* Lisp symbols of D-Bus types.  */
   DEFSYM (QCbyte, ":byte");
   DEFSYM (QCboolean, ":boolean");
-- 
2.45.2

bug#71969: [PATCH] Support interactive D-Bus authentication

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Steven Allen <steven@stebalien.com> writes:

Hi Steven,

>>> Remaining questions:
>>>
>>> 1. I'm not sure if :authorize is quite correct either. Really, the key
>>> part is that it allows /interactive/ authorization. I wonder if
>>> :interactive-authorization or :interactive might be better (although
>>> they're kind of long).
>>
>> I believe :authorize is OK. In the docstrings as well as in the D-Bus
>> manual, interactive authorization is mentioned, so a user shall know
>> what's about.
>
> Hm, it's still bugging me. We're _not_ authorizing the request, we're
> telling D-Bus that it's ok to ask the user if they want to authorize it.
> I'm hoping the example below will make this clearer.

What about :authorizable? I don't like the alternative
:interactive-authorize; it's too long to type, and it's also not obvious
w/o knowing the context.

>> Furthermore, you haven't given an example. I really would like to see
>> how it works in practice.
>
> Sorry about that. To restart the bluetooth service, execute:
>
>     (dbus-call-method
>      :system
>      "org.freedesktop.systemd1" "/org/freedesktop/systemd1"
>      "org.freedesktop.systemd1.Manager" "RestartUnit"
>      :authorize t
>      "bluetooth.service" "replace")
>
> Assuming you have a polkit agent running (most DEs will run one by
> default, but agents like mate-polkit work pretty well standalone),
> you'll be prompted to authorize the operation and the bluetooth service
> will be restarted.

Nice. I get an authorization prompt.

However, on my Fedora 40 / Gnome 46 / systemd 255 system, it doesn't
matter, whether I use ':authorize t', ':authorize nil', or none of
them. Is interactive authorization enabled by default, and we don't need
to care about?

>>> +If the parameter @code{:authorize} is given and the following
>>> +@var{auth} is non-nil, the invoked method may interactively prompt the
>>
>> non-@code{nil}

> Done and done (the info manuals are pretty inconsistent in this regard...).

If you see it somewhere else in the manuals, it is an error. The rule is
to use @code{nil}, non-@code{nil}, and @code{t}. Feel free to correct this.

>>> +	  /* Ignore this keyword if unsupported. */
>>> +	  #ifdef HAVE_DBUS_MESSAGE_SET_ALLOW_INTERACTIVE_AUTHORIZATION
>>> +	  dbus_message_set_allow_interactive_authorization
>>> +	  (dmessage, NILP (args[count+1]) ? FALSE : TRUE);
>>> +	  #endif
>>
>> #ifdef end #endif shall start in column 1. Futhermore, we need an #else
>> clause. There shall be an error or a warning, that :authorize is not supported.
>
> I'm going to disagree on this last point. The flag is specifying whether
> or not the D-Bus is _allowed_ to ask the user to ask the user to
> authorize requests which can fail for multiple reasons anyways (e.g., if
> no polkit agent is running, the user rejects the interactive
> authorization, etc.).
>
> If authorization is required and wasn't possible for some reason,
> D-Bus will return an error to the user anyways. So the user will get
> their warning either way _if_ something actually goes wrong.

Good point. However, we shall support developers if they run into this
case. What about a debug message like

--8<---------------cut here---------------start------------->8---
#ifdef HAVE_DBUS_MESSAGE_SET_ALLOW_INTERACTIVE_AUTHORIZATION
	  dbus_message_set_allow_interactive_authorization
	    (dmessage, NILP (args[count+1]) ? FALSE : TRUE);
#else
	  XD_DEBUG_MESSAGE (":authorize not supported");
#endif
--8<---------------cut here---------------end--------------->8---

Best regards, Michael.

bug#71969: [PATCH] Support interactive D-Bus authentication

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Michael Albinus <michael.albinus@gmx.de> writes:

Hi Steven,

>>     (dbus-call-method
>>      :system
>>      "org.freedesktop.systemd1" "/org/freedesktop/systemd1"
>>      "org.freedesktop.systemd1.Manager" "RestartUnit"
>>      :authorize t
>>      "bluetooth.service" "replace")
>>
>> Assuming you have a polkit agent running (most DEs will run one by
>> default, but agents like mate-polkit work pretty well standalone),
>> you'll be prompted to authorize the operation and the bluetooth service
>> will be restarted.
>
> Nice. I get an authentication prompt.
>
> However, on my Fedora 40 / Gnome 46 / systemd 255 system, it doesn't
> matter, whether I use ':authorize t', ':authorize nil', or none of
> them. Is interactive authorization enabled by default, and we don't need
> to care about?

Perhaps this is due to the example. I'm asked for "Authentication
Required". This is not interactive authorization.

Do you have another example?

Best regards, Michael.

bug#71969: [PATCH] Support interactive D-Bus authentication

[Steven Allen via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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


Michael Albinus <michael.albinus@gmx.de> writes:
> Steven Allen <steven@stebalien.com> writes:
>
> Hi Steven,
>
>>>> Remaining questions:
>>>>
>>>> 1. I'm not sure if :authorize is quite correct either. Really, the key
>>>> part is that it allows /interactive/ authorization. I wonder if
>>>> :interactive-authorization or :interactive might be better (although
>>>> they're kind of long).
>>>
>>> I believe :authorize is OK. In the docstrings as well as in the D-Bus
>>> manual, interactive authorization is mentioned, so a user shall know
>>> what's about.
>>
>> Hm, it's still bugging me. We're _not_ authorizing the request, we're
>> telling D-Bus that it's ok to ask the user if they want to authorize it.
>> I'm hoping the example below will make this clearer.
>
> What about :authorizable? I don't like the alternative
> :interactive-authorize; it's too long to type, and it's also not obvious
> w/o knowing the context.

It's a bit funky but good enough. Thanks!

>
>>> Furthermore, you haven't given an example. I really would like to see
>>> how it works in practice.
>>
>> Sorry about that. To restart the bluetooth service, execute:
>>
>>     (dbus-call-method
>>      :system
>>      "org.freedesktop.systemd1" "/org/freedesktop/systemd1"
>>      "org.freedesktop.systemd1.Manager" "RestartUnit"
>>      :authorize t
>>      "bluetooth.service" "replace")
>>
>> Assuming you have a polkit agent running (most DEs will run one by
>> default, but agents like mate-polkit work pretty well standalone),
>> you'll be prompted to authorize the operation and the bluetooth service
>> will be restarted.
>
> Nice. I get an authorization prompt.
>
> However, on my Fedora 40 / Gnome 46 / systemd 255 system, it doesn't
> matter, whether I use ':authorize t', ':authorize nil', or none of
> them. Is interactive authorization enabled by default, and we don't need
> to care about?

It worked for me as well until a recent update (likely polkit 124 or
systemd 256). I'm guessing one of these projects fixed a bug somewhere
as it sounds like this flag should always have been required.

>>>> +If the parameter @code{:authorize} is given and the following
>>>> +@var{auth} is non-nil, the invoked method may interactively prompt the
>>>
>>> non-@code{nil}
>
>> Done and done (the info manuals are pretty inconsistent in this regard...).
>
> If you see it somewhere else in the manuals, it is an error. The rule is
> to use @code{nil}, non-@code{nil}, and @code{t}. Feel free to correct this.

I'll submit a separate patch.

>>>> +	  /* Ignore this keyword if unsupported. */
>>>> +	  #ifdef HAVE_DBUS_MESSAGE_SET_ALLOW_INTERACTIVE_AUTHORIZATION
>>>> +	  dbus_message_set_allow_interactive_authorization
>>>> +	  (dmessage, NILP (args[count+1]) ? FALSE : TRUE);
>>>> +	  #endif
>>>
>>> #ifdef end #endif shall start in column 1. Futhermore, we need an #else
>>> clause. There shall be an error or a warning, that :authorize is not supported.
>>
>> I'm going to disagree on this last point. The flag is specifying whether
>> or not the D-Bus is _allowed_ to ask the user to ask the user to
>> authorize requests which can fail for multiple reasons anyways (e.g., if
>> no polkit agent is running, the user rejects the interactive
>> authorization, etc.).
>>
>> If authorization is required and wasn't possible for some reason,
>> D-Bus will return an error to the user anyways. So the user will get
>> their warning either way _if_ something actually goes wrong.
>
> Good point. However, we shall support developers if they run into this
> case. What about a debug message like
>
> --8<---------------cut here---------------start------------->8---
> #ifdef HAVE_DBUS_MESSAGE_SET_ALLOW_INTERACTIVE_AUTHORIZATION
> 	  dbus_message_set_allow_interactive_authorization
> 	    (dmessage, NILP (args[count+1]) ? FALSE : TRUE);
> #else
> 	  XD_DEBUG_MESSAGE (":authorize not supported");
> #endif
> --8<---------------cut here---------------end--------------->8---

Fair enough. I don't want to be too noisy (I want to be able to just add
a blanket ":authorize t" to all my potentially privileged D-Bus calls),
but we add the debug message and see what feedback we get.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Support-interactive-D-Bus-authorization.patch --]
[-- Type: text/x-patch, Size: 8027 bytes --]

From 56ff5d1ed55303526f5789bab7ce2abf968f9bfb Mon Sep 17 00:00:00 2001
From: Steven Allen <steven@stebalien.com>
Date: Thu, 4 Jul 2024 20:45:07 +0200
Subject: [PATCH] Support interactive D-Bus authorization

When invoking D-Bus methods, let the user enable interactive
authorization by passing an :authorizable t parameter.  This makes it
possible to D-Bus methods that require polkit authorization.

* src/dbusbind.c (dbus-message-internal): Allow interactive
authorization by passing :authorizable t.
* lisp/net/dbus.el (dbus-call-method-asynchronously): Document the new
parameter.
* doc/misc/dbus.texi (Synchronous Methods, Asynchronous Methods):
Document the new parameter.
* configure.ac (HAVE_DBUS_MESSAGE_SET_ALLOW_INTERACTIVE_AUTHORIZATION):
Set a new variable if
`dbus_message_set_allow_interactive_authorization' is available.
---
 configure.ac       |  5 ++++-
 doc/misc/dbus.texi | 12 ++++++++++--
 etc/NEWS           |  6 ++++++
 lisp/net/dbus.el   |  8 ++++++++
 src/dbusbind.c     | 39 +++++++++++++++++++++++++++++++++------
 5 files changed, 61 insertions(+), 9 deletions(-)

diff --git a/configure.ac b/configure.ac
index 909f5786c9a..ee2ef1c60fb 100644
--- a/configure.ac
+++ b/configure.ac
@@ -3943,6 +3943,8 @@ AC_DEFUN
      dnl dbus_watch_get_unix_fd has been introduced in D-Bus 1.1.1.
      dnl dbus_type_is_valid and dbus_validate_* have been introduced in
      dnl D-Bus 1.5.12.
+     dnl dbus_message_set_allow_interactive_authorization was introduced
+     dnl in D-Bus 1.8.10.
      OLD_LIBS=$LIBS
      LIBS="$LIBS $DBUS_LIBS"
      AC_CHECK_FUNCS([dbus_watch_get_unix_fd \
@@ -3950,7 +3952,8 @@ AC_DEFUN
 		    dbus_validate_bus_name \
                     dbus_validate_path \
 		    dbus_validate_interface \
-		    dbus_validate_member])
+		    dbus_validate_member \
+                    dbus_message_set_allow_interactive_authorization])
      LIBS=$OLD_LIBS
      DBUS_OBJ=dbusbind.o
    fi
diff --git a/doc/misc/dbus.texi b/doc/misc/dbus.texi
index e5d867acd40..20d26c80d38 100644
--- a/doc/misc/dbus.texi
+++ b/doc/misc/dbus.texi
@@ -1208,7 +1208,7 @@ Synchronous Methods
 be called, and a reply message returning the resulting output
 parameters from the object.
 
-@defun dbus-call-method bus service path interface method &optional :timeout timeout &rest args
+@defun dbus-call-method bus service path interface method &optional :timeout timeout :authorizable auth &rest args
 @anchor{dbus-call-method}
 This function calls @var{method} on the D-Bus @var{bus}.  @var{bus} is
 either the keyword @code{:system} or the keyword @code{:session}.
@@ -1223,6 +1223,10 @@ Synchronous Methods
 call doesn't return in time, a D-Bus error is raised (@pxref{Errors
 and Events}).
 
+If the parameter @code{:authorizable} is given and the following
+@var{auth} is non-@code{nil}, the invoked method may interactively
+prompt the user for authorization.  The default is @code{nil}.
+
 The remaining arguments @var{args} are passed to @var{method} as
 arguments.  They are converted into D-Bus types as described in
 @ref{Type Conversion}.
@@ -1302,7 +1306,7 @@ Asynchronous Methods
 @cindex method calls, asynchronous
 @cindex asynchronous method calls
 
-@defun dbus-call-method-asynchronously bus service path interface method handler &optional :timeout timeout &rest args
+@defun dbus-call-method-asynchronously bus service path interface method handler &optional :timeout timeout :authorizable auth &rest args
 This function calls @var{method} on the D-Bus @var{bus}
 asynchronously.  @var{bus} is either the keyword @code{:system} or the
 keyword @code{:session}.
@@ -1321,6 +1325,10 @@ Asynchronous Methods
 no reply message in time, a D-Bus error is raised (@pxref{Errors and
 Events}).
 
+If the parameter @code{:authorizable} is given and the following
+@var{auth} is non-@code{nil}, the invoked method may interactively
+prompt the user for authorization.  The default is @code{nil}.
+
 The remaining arguments @var{args} are passed to @var{method} as
 arguments.  They are converted into D-Bus types as described in
 @ref{Type Conversion}.
diff --git a/etc/NEWS b/etc/NEWS
index 3d2b86cfb6a..75771b0c05a 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -79,6 +79,12 @@ levels that SHR cycles through when calling 'shr-zoom-image'.
 \f
 * Lisp Changes in Emacs 31.1
 
++++
+*** Support interactive D-Bus authorization.
+A new ':authorizable t' parameter has been added to 'dbus-call-method'
+and 'dbus-call-method-asynchronously' to allow the user to interactively
+authorizable the invoked D-Bus method (e.g., via polkit).
+
 \f
 * Changes in Emacs 31.1 on Non-Free Operating Systems
 
diff --git a/lisp/net/dbus.el b/lisp/net/dbus.el
index dd5f0e88859..a50f3a93938 100644
--- a/lisp/net/dbus.el
+++ b/lisp/net/dbus.el
@@ -297,6 +297,10 @@ dbus-call-method
 method call must return.  The default value is 25,000.  If the
 method call doesn't return in time, a D-Bus error is raised.
 
+If the parameter `:authorizable' is given and the following AUTH
+is non-nil, the invoked method may interactively prompt the user
+for authorization.  The default is nil.
+
 All other arguments ARGS are passed to METHOD as arguments.  They are
 converted into D-Bus types via the following rules:
 
@@ -427,6 +431,10 @@ dbus-call-method-asynchronously
 method call must return.  The default value is 25,000.  If the
 method call doesn't return in time, a D-Bus error is raised.
 
+If the parameter `:authorizable' is given and the following AUTH
+is non-nil, the invoked method may interactively prompt the user
+for authorization.  The default is nil.
+
 All other arguments ARGS are passed to METHOD as arguments.  They are
 converted into D-Bus types via the following rules:
 
diff --git a/src/dbusbind.c b/src/dbusbind.c
index 35ce03c7911..cf292944531 100644
--- a/src/dbusbind.c
+++ b/src/dbusbind.c
@@ -1314,7 +1314,7 @@ DEFUN ("dbus-message-internal", Fdbus_message_internal, Sdbus_message_internal,
 `dbus-call-method', `dbus-call-method-asynchronously':
   (dbus-message-internal
     dbus-message-type-method-call BUS SERVICE PATH INTERFACE METHOD HANDLER
-    &optional :timeout TIMEOUT &rest ARGS)
+    &optional :timeout TIMEOUT :authorizable AUTH &rest ARGS)
 
 `dbus-send-signal':
   (dbus-message-internal
@@ -1512,12 +1512,36 @@ DEFUN ("dbus-message-internal", Fdbus_message_internal, Sdbus_message_internal,
 	XD_SIGNAL1 (build_string ("Unable to create an error message"));
     }
 
-  /* Check for timeout parameter.  */
-  if ((count + 2 <= nargs) && EQ (args[count], QCtimeout))
+  while ((count + 2 <= nargs))
     {
-      CHECK_FIXNAT (args[count+1]);
-      timeout = min (XFIXNAT (args[count+1]), INT_MAX);
-      count = count+2;
+      /* Check for timeout parameter.  */
+      if (EQ (args[count], QCtimeout))
+        {
+	  if (mtype != DBUS_MESSAGE_TYPE_METHOD_CALL)
+	    XD_SIGNAL1 (build_string (":timeout is only supported on method calls"));
+
+          CHECK_FIXNAT (args[count+1]);
+          timeout = min (XFIXNAT (args[count+1]), INT_MAX);
+          count = count+2;
+	}
+      /* Check for authorizable parameter.  */
+      else if (EQ (args[count], QCauthorizable))
+        {
+	  if (mtype != DBUS_MESSAGE_TYPE_METHOD_CALL)
+	    XD_SIGNAL1 (build_string (":authorizable is only supported on method calls"));
+
+	  /* Ignore this keyword if unsupported. */
+#ifdef HAVE_DBUS_MESSAGE_SET_ALLOW_INTERACTIVE_AUTHORIZATION
+	  dbus_message_set_allow_interactive_authorization
+	  (dmessage, NILP (args[count+1]) ? FALSE : TRUE);
+#else
+	  XD_DEBUG_MESSAGE (":authorizable not supported");
+#endif
+
+          count = count+2;
+	}
+      else break;
+
     }
 
   /* Initialize parameter list of message.  */
@@ -1895,6 +1919,9 @@ syms_of_dbusbind (void)
   /* Lisp symbol for method call timeout.  */
   DEFSYM (QCtimeout, ":timeout");
 
+  /* Lisp symbol for method interactive authorization.  */
+  DEFSYM (QCauthorizable, ":authorizable");
+
   /* Lisp symbols of D-Bus types.  */
   DEFSYM (QCbyte, ":byte");
   DEFSYM (QCboolean, ":boolean");
-- 
2.45.2

bug#71969: [PATCH] Support interactive D-Bus authentication

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Steven Allen <steven@stebalien.com> writes:

Hi Steven,

>>> Sorry about that. To restart the bluetooth service, execute:
>>>
>>>     (dbus-call-method
>>>      :system
>>>      "org.freedesktop.systemd1" "/org/freedesktop/systemd1"
>>>      "org.freedesktop.systemd1.Manager" "RestartUnit"
>>>      :authorize t
>>>      "bluetooth.service" "replace")
>>>
>>> Assuming you have a polkit agent running (most DEs will run one by
>>> default, but agents like mate-polkit work pretty well standalone),
>>> you'll be prompted to authorize the operation and the bluetooth service
>>> will be restarted.
>>
>> Nice. I get an authorization prompt.
>>
>> However, on my Fedora 40 / Gnome 46 / systemd 255 system, it doesn't
>> matter, whether I use ':authorize t', ':authorize nil', or none of
>> them. Is interactive authorization enabled by default, and we don't need
>> to care about?
>
> It worked for me as well until a recent update (likely polkit 124 or
> systemd 256). I'm guessing one of these projects fixed a bug somewhere
> as it sounds like this flag should always have been required.

Hmm, I don't understand what you want to say.

Anyway, I believe we shall add this feature to Emacs only when we have
seen an example where it makes a difference.

Best regards, Michael.

bug#71969: [PATCH] Support interactive D-Bus authentication

[Steven Allen via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Michael Albinus <michael.albinus@gmx.de> writes:
> Steven Allen <steven@stebalien.com> writes:
>
> Hi Steven,
>
>>>> Sorry about that. To restart the bluetooth service, execute:
>>>>
>>>>     (dbus-call-method
>>>>      :system
>>>>      "org.freedesktop.systemd1" "/org/freedesktop/systemd1"
>>>>      "org.freedesktop.systemd1.Manager" "RestartUnit"
>>>>      :authorize t
>>>>      "bluetooth.service" "replace")
>>>>
>>>> Assuming you have a polkit agent running (most DEs will run one by
>>>> default, but agents like mate-polkit work pretty well standalone),
>>>> you'll be prompted to authorize the operation and the bluetooth service
>>>> will be restarted.
>>>
>>> Nice. I get an authorization prompt.
>>>
>>> However, on my Fedora 40 / Gnome 46 / systemd 255 system, it doesn't
>>> matter, whether I use ':authorize t', ':authorize nil', or none of
>>> them. Is interactive authorization enabled by default, and we don't need
>>> to care about?
>>
>> It worked for me as well until a recent update (likely polkit 124 or
>> systemd 256). I'm guessing one of these projects fixed a bug somewhere
>> as it sounds like this flag should always have been required.
>
> Hmm, I don't understand what you want to say.
>
> Anyway, I believe we shall add this feature to Emacs only when we have
> seen an example where it makes a difference.
>
> Best regards, Michael.

I meant that `dbus-method-call` worked without `:authorize t` for me as
well until I updated my system. Testing both systemd and fwupd, it
appears that the issue was caused by upgrading to systemd 256 (available
on Arch Linux but not Fedora 40). That is, systemd 256 requires
`:authorize t` while fwupd does not.

From what I can tell, this change was introduced in this [1] systemd PR.
Specifically, this comment:

    This also fixes a bunch of wrong uses of the "interactive" bool. The
    bool makes no sense today as the ALLOW_INTERACTIVE_AUTHORIZATION
    field in the D-Bus message header replaces it fully.

[1]: https://github.com/systemd/systemd/pull/30565

bug#71969: [PATCH] Support interactive D-Bus authentication

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Version: 31.1

Steven Allen <steven@stebalien.com> writes:

Hi Steven,

> I meant that `dbus-method-call` worked without `:authorize t` for me as
> well until I updated my system. Testing both systemd and fwupd, it
> appears that the issue was caused by upgrading to systemd 256 (available
> on Arch Linux but not Fedora 40). That is, systemd 256 requires
> `:authorize t` while fwupd does not.
>
> From what I can tell, this change was introduced in this [1] systemd PR.
> Specifically, this comment:
>
>     This also fixes a bunch of wrong uses of the "interactive" bool. The
>     bool makes no sense today as the ALLOW_INTERACTIVE_AUTHORIZATION
>     field in the D-Bus message header replaces it fully.
>
> [1]: https://github.com/systemd/systemd/pull/30565

Thanks for the explanation. I've installed systemd 256 from Fedora
rawhide on my system, and now I see the same behavior.

I've installed your patch to the Emacs master branch. Closing the bug.

Do we need to mention somewhere in the doc, that this is needed starting
with systemd 256?

Best regards, Michael.

bug#71969: [PATCH] Support interactive D-Bus authentication

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Steven Allen <steven@stebalien.com> writes:

Hi Steven,

> (I want to be able to just add a blanket ":authorize t" to all my
> potentially privileged D-Bus calls)

That's not needed anymore. I've added a warning when we get
"org.freedesktop.DBus.Error.InteractiveAuthorizationRequired", so you
don't need to add the :authorizable argument proactively.

Best regards, Michael.

bug#71969: [PATCH] Support interactive D-Bus authentication

[Steven Allen via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Michael Albinus <michael.albinus@gmx.de> writes:
>> (I want to be able to just add a blanket ":authorize t" to all my
>> potentially privileged D-Bus calls)
>
> That's not needed anymore. I've added a warning when we get
> "org.freedesktop.DBus.Error.InteractiveAuthorizationRequired", so you
> don't need to add the :authorizable argument proactively.
>
> Best regards, Michael.

Ah, that's a good idea. IMO, that also addresses:

    Do we need to mention somewhere in the doc, that this is needed
    starting with systemd 256?

(that is, no, we probably don't need to call it out explicitly)

Thanks for getting this all merged and thanks for writing the tests
(forgot you asked me to do that, sorry).

- Steven

bug#71969: [PATCH] Support interactive D-Bus authentication

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Steven Allen <steven@stebalien.com> writes:

Hi Steven,

> Ah, that's a good idea. IMO, that also addresses:
>
>     Do we need to mention somewhere in the doc, that this is needed
>     starting with systemd 256?
>
> (that is, no, we probably don't need to call it out explicitly)

OK. But perhaps, you could add your bluez example to dbus.texi? It was
very helpful for me, and other people might profit from it as well.

> Thanks for getting this all merged and thanks for writing the tests
> (forgot you asked me to do that, sorry).
>
> - Steven

Best regards, Michael.

bug#71969: [PATCH] Support interactive D-Bus authentication

[Steven Allen via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Michael Albinus <michael.albinus@gmx.de> writes:

> Steven Allen <steven@stebalien.com> writes:
>
> Hi Steven,
>
>> Ah, that's a good idea. IMO, that also addresses:
>>
>>     Do we need to mention somewhere in the doc, that this is needed
>>     starting with systemd 256?
>>
>> (that is, no, we probably don't need to call it out explicitly)
>
> OK. But perhaps, you could add your bluez example to dbus.texi? It was
> very helpful for me, and other people might profit from it as well.

Can do (that and fixing the non-nils), but I may not get to it for a few days.