Package: emacs;
Reported by: Steven Allen <steven <at> stebalien.com>
Date: Sat, 6 Jul 2024 07:55:02 UTC
Severity: wishlist
Tags: patch
Fixed in version 31.1
Done: Michael Albinus <michael.albinus <at> gmx.de>
Bug is archived. No further changes may be made.
Message #43 received at 71969 <at> debbugs.gnu.org (full text, mbox):
From: Steven Allen <steven <at> stebalien.com> To: Michael Albinus <michael.albinus <at> gmx.de> Cc: Eli Zaretskii <eliz <at> gnu.org>, 71969 <at> debbugs.gnu.org Subject: Re: bug#71969: [PATCH] Support interactive D-Bus authentication Date: Mon, 08 Jul 2024 15:21:30 +0200
[Message part 1 (text/plain, inline)]
Michael Albinus <michael.albinus <at> gmx.de> writes:
> Steven Allen <steven <at> 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.
[0001-Support-interactive-D-Bus-authorization.patch (text/x-patch, inline)]
From 56ff5d1ed55303526f5789bab7ce2abf968f9bfb Mon Sep 17 00:00:00 2001
From: Steven Allen <steven <at> 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'.
* 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).
+
* 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
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.