GNU bug report logs - #62952
28.2.50; secrets.el unlocking items

Previous Next

Package: emacs;

Reported by: "Philipp Uhl" <git <at> ph-uhl.com>

Date: Wed, 19 Apr 2023 19:59:02 UTC

Severity: normal

Found in version 28.2.50

Fixed in version 30.1

Done: Michael Albinus <michael.albinus <at> gmx.de>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 62952 in the body.
You can then email your comments to 62952 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#62952; Package emacs. (Wed, 19 Apr 2023 19:59:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to "Philipp Uhl" <git <at> ph-uhl.com>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Wed, 19 Apr 2023 19:59:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: "Philipp Uhl" <git <at> ph-uhl.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 28.2.50; secrets.el unlocking items
Date: Wed, 19 Apr 2023 14:23:17 +0200
The secrets.el implementation lacks support for unlocking specific
items. It only unlocks collections. This does not work well with certain
password managers (e.g. in my case KeepassXC, accessed through secret
service). When receiving a secret through

(secrets-get-secret  "MyPws" "MyEntry")

with the setting "Confirm when passwords are retrieved by clients"
turned on in KeepassXC, secrets-get-secret will just say IsLocked.

Instead, secrets-get-secret should try to unlock the entry itself before
retrieving.

Here is a proof of concept:

+  ;; New function, analogously to secrets-unlock-collection, that
+  ;; specifically unlocks the item
+  (defun secrets-unlock-item (collection item)
+    "Unlock item labeled ITEM from collection labeled COLLECTION.
+  If successful, return the object path of the item."
+    (let ((item-path (secrets-item-path collection item)))
+      (unless (secrets-empty-path item-path)
+        (secrets-prompt
+         (cadr
+          (dbus-call-method
+           :session secrets-service secrets-path secrets-interface-service
+           "Unlock" `(:array :object-path ,item-path)))))
+      item-path))

  (defun secrets-get-secret (collection item)
    "Return the secret of item labeled ITEM in COLLECTION.
  If there are several items labeled ITEM, it is undefined which
  one is returned.  If there is no such item, return nil.

  ITEM can also be an object path, which is used if contained in COLLECTION."
-    (let ((item-path (secrets-item-path collection item)))
+    (let ((item-path (secrets-unlock-item collection item)))
      (unless (secrets-empty-path item-path)
        (dbus-byte-array-to-string
         (nth 2
              (dbus-call-method
               :session secrets-service item-path secrets-interface-item
               "GetSecret" :object-path secrets-session-path))))))

To make this function a bit more similar to how it was before, one could
concider to explicitly wait for the IsLocked event before unlocking the
item. That way, if the password manager does not support unlocking of
items, this would not be braking.

Cheers,
-----------------------------
  Philipp Uhl
  git <at> ph-uhl.com




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#62952; Package emacs. (Thu, 20 Apr 2023 11:24:02 GMT) Full text and rfc822 format available.

Message #8 received at 62952 <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: "Philipp Uhl" <git <at> ph-uhl.com>
Cc: 62952 <at> debbugs.gnu.org
Subject: Re: bug#62952: 28.2.50; secrets.el unlocking items
Date: Thu, 20 Apr 2023 13:23:34 +0200
"Philipp Uhl" <git <at> ph-uhl.com> writes:

Hi Philipp,

> The secrets.el implementation lacks support for unlocking specific
> items. It only unlocks collections. This does not work well with certain
> password managers (e.g. in my case KeepassXC, accessed through secret
> service). When receiving a secret through
>
> (secrets-get-secret  "MyPws" "MyEntry")
>
> with the setting "Confirm when passwords are retrieved by clients"
> turned on in KeepassXC, secrets-get-secret will just say IsLocked.

Thanks for the report.

> Instead, secrets-get-secret should try to unlock the entry itself before
> retrieving.
>
> Here is a proof of concept:
>
> +  ;; New function, analogously to secrets-unlock-collection, that
> +  ;; specifically unlocks the item
> +  (defun secrets-unlock-item (collection item)
> +    "Unlock item labeled ITEM from collection labeled COLLECTION.
> +  If successful, return the object path of the item."
> +    (let ((item-path (secrets-item-path collection item)))
> +      (unless (secrets-empty-path item-path)
> +        (secrets-prompt
> +         (cadr
> +          (dbus-call-method
> +           :session secrets-service secrets-path secrets-interface-service
> +           "Unlock" `(:array :object-path ,item-path)))))
> +      item-path))
>
>   (defun secrets-get-secret (collection item)
>     "Return the secret of item labeled ITEM in COLLECTION.
>   If there are several items labeled ITEM, it is undefined which
>   one is returned.  If there is no such item, return nil.
>
>   ITEM can also be an object path, which is used if contained in COLLECTION."
> -    (let ((item-path (secrets-item-path collection item)))
> +    (let ((item-path (secrets-unlock-item collection item)))
>       (unless (secrets-empty-path item-path)
>         (dbus-byte-array-to-string
>          (nth 2
>               (dbus-call-method
>                :session secrets-service item-path secrets-interface-item
>                "GetSecret" :object-path secrets-session-path))))))
>
> To make this function a bit more similar to how it was before, one could
> concider to explicitly wait for the IsLocked event before unlocking the
> item. That way, if the password manager does not support unlocking of
> items, this would not be braking.

LGTM. Well, I don't know how relevant it is to wait for the IsLocked
event. If you have use cases where it is needed, we shall do.

When we add secrets-unlock-item, we should also add secrets-lock-item as
counterpart. Like we have done it with secrets-(un)?lock-collection.
Would you like to add this function? Bonus points for respective tests
in secrets-tests.el.

All these changes exceed the limit for tiny changes in Emacs, which
could be submitted w/o legal work. Would you like to sign FSF copyright
papers in order to contribute to Emacs? See
<https://git.savannah.gnu.org/cgit/gnulib.git/plain/doc/Copyright/conditions.text>
which explains the reasons.

> Cheers,
>   Philipp Uhl

Best regards, Michael.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#62952; Package emacs. (Tue, 02 May 2023 10:06:01 GMT) Full text and rfc822 format available.

Message #11 received at 62952 <at> debbugs.gnu.org (full text, mbox):

From: "Philipp Uhl" <git <at> ph-uhl.com>
To: "Michael Albinus" <michael.albinus <at> gmx.de>
Cc: 62952 <at> debbugs.gnu.org
Subject: Re: bug#62952: 28.2.50; secrets.el unlocking items
Date: Tue, 02 May 2023 12:05:03 +0200
[Message part 1 (text/plain, inline)]
Hi Michael,

thanks for your response. Here is the secrets-lock-item function:

(defun secrets-lock-item (collection item)
    "Lock collection item labeled ITEM in COLLECTION.
If successful, return the object path of the item. Does not lock
the collection."
    (let ((item-path (secrets-item-path collection item)))
      (unless (secrets-empty-path item-path)
        (secrets-prompt
         (cadr
          (dbus-call-method
           :session secrets-service secrets-path secrets-interface-service
           "Lock" `(:array :object-path ,item-path)))))
      item-path))

> Bonus points for respective tests
> in secrets-tests.el.

I didn't find any secrets-tests.el in the Emacs repository. Also I am not really familiar with writing test code in Elisp. But I did manually test the code and it works.

> Well, I don't know how relevant it is to wait for the IsLocked
> event. If you have use cases where it is needed, we shall do.

I don't. For my purposes the code as shown before suffices.

> All these changes exceed the limit for tiny changes in Emacs, which
> could be submitted w/o legal work. Would you like to sign FSF copyright
> papers in order to contribute to Emacs? See
> <https://git.savannah.gnu.org/cgit/gnulib.git/plain/doc/Copyright/conditions.text>
> which explains the reasons.

Yes. Would digitally suffice? What exactly do I have to sign?

Cheers, Philipp

-----------------------------
  Philipp Uhl
  git <at> ph-uhl.com

Am Do, 20. Apr 2023, um 13:23, schrieb Michael Albinus:
> "Philipp Uhl" <git <at> ph-uhl.com> writes:
>
> Hi Philipp,
>
>> The secrets.el implementation lacks support for unlocking specific
>> items. It only unlocks collections. This does not work well with certain
>> password managers (e.g. in my case KeepassXC, accessed through secret
>> service). When receiving a secret through
>>
>> (secrets-get-secret  "MyPws" "MyEntry")
>>
>> with the setting "Confirm when passwords are retrieved by clients"
>> turned on in KeepassXC, secrets-get-secret will just say IsLocked.
>
> Thanks for the report.
>
>> Instead, secrets-get-secret should try to unlock the entry itself before
>> retrieving.
>>
>> Here is a proof of concept:
>>
>> +  ;; New function, analogously to secrets-unlock-collection, that
>> +  ;; specifically unlocks the item
>> +  (defun secrets-unlock-item (collection item)
>> +    "Unlock item labeled ITEM from collection labeled COLLECTION.
>> +  If successful, return the object path of the item."
>> +    (let ((item-path (secrets-item-path collection item)))
>> +      (unless (secrets-empty-path item-path)
>> +        (secrets-prompt
>> +         (cadr
>> +          (dbus-call-method
>> +           :session secrets-service secrets-path secrets-interface-service
>> +           "Unlock" `(:array :object-path ,item-path)))))
>> +      item-path))
>>
>>   (defun secrets-get-secret (collection item)
>>     "Return the secret of item labeled ITEM in COLLECTION.
>>   If there are several items labeled ITEM, it is undefined which
>>   one is returned.  If there is no such item, return nil.
>>
>>   ITEM can also be an object path, which is used if contained in COLLECTION."
>> -    (let ((item-path (secrets-item-path collection item)))
>> +    (let ((item-path (secrets-unlock-item collection item)))
>>       (unless (secrets-empty-path item-path)
>>         (dbus-byte-array-to-string
>>          (nth 2
>>               (dbus-call-method
>>                :session secrets-service item-path secrets-interface-item
>>                "GetSecret" :object-path secrets-session-path))))))
>>
>> To make this function a bit more similar to how it was before, one could
>> concider to explicitly wait for the IsLocked event before unlocking the
>> item. That way, if the password manager does not support unlocking of
>> items, this would not be braking.
>
> LGTM. Well, I don't know how relevant it is to wait for the IsLocked
> event. If you have use cases where it is needed, we shall do.
>
> When we add secrets-unlock-item, we should also add secrets-lock-item as
> counterpart. Like we have done it with secrets-(un)?lock-collection.
> Would you like to add this function? Bonus points for respective tests
> in secrets-tests.el.
>
> All these changes exceed the limit for tiny changes in Emacs, which
> could be submitted w/o legal work. Would you like to sign FSF copyright
> papers in order to contribute to Emacs? See
> <https://git.savannah.gnu.org/cgit/gnulib.git/plain/doc/Copyright/conditions.text>
> which explains the reasons.
>
>> Cheers,
>>   Philipp Uhl
>
> Best regards, Michael.
[Message part 2 (text/html, inline)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#62952; Package emacs. (Tue, 02 May 2023 11:46:01 GMT) Full text and rfc822 format available.

Message #14 received at 62952 <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: "Philipp Uhl" <git <at> ph-uhl.com>
Cc: 62952 <at> debbugs.gnu.org
Subject: Re: bug#62952: 28.2.50; secrets.el unlocking items
Date: Tue, 02 May 2023 13:44:47 +0200
"Philipp Uhl" <git <at> ph-uhl.com> writes:

> Hi Michael,

Hi Philipp,

> thanks for your response. Here is the secrets-lock-item function:

Thanks. I'll check it next days.

>> Bonus points for respective tests
>> in secrets-tests.el.
>
> I didn't find any secrets-tests.el in the Emacs repository. Also I am
> not really familiar with writing test code in Elisp. But I did
> manually test the code and it works.

See test/lisp/net/secrets-tests.el in the git repository. Tests are
performed using ERT, see (info "(ert) Top") for the manual.

>> Well, I don't know how relevant it is to wait for the IsLocked
>> event. If you have use cases where it is needed, we shall do.
>
> I don't. For my purposes the code as shown before suffices.

Good. So we can leave it out, until somebody hollers.

>> Would you like to sign FSF copyright papers in order to contribute to
>> Emacs?
>
> Yes. Would digitally suffice? What exactly do I have to sign?

Template sent off-list.

> Cheers, Philipp

Best regards, Michael.




Reply sent to Michael Albinus <michael.albinus <at> gmx.de>:
You have taken responsibility. (Mon, 08 May 2023 11:43:01 GMT) Full text and rfc822 format available.

Notification sent to "Philipp Uhl" <git <at> ph-uhl.com>:
bug acknowledged by developer. (Mon, 08 May 2023 11:43:02 GMT) Full text and rfc822 format available.

Message #19 received at 62952-done <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: "Philipp Uhl" <git <at> ph-uhl.com>
Cc: 62952-done <at> debbugs.gnu.org
Subject: Re: bug#62952: 28.2.50; secrets.el unlocking items
Date: Mon, 08 May 2023 13:42:44 +0200
Version: 30.1

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

Hi Philipp,

>> thanks for your response. Here is the secrets-lock-item function:
>
> Thanks. I'll check it next days.

I've played this morning with your changes. Everything looks fine (in my
environment), so I have pushed them to the Emacs master branch.

>>> Bonus points for respective tests in secrets-tests.el.
>>
>> I didn't find any secrets-tests.el in the Emacs repository. Also I am
>> not really familiar with writing test code in Elisp. But I did
>> manually test the code and it works.
>
> See test/lisp/net/secrets-tests.el in the git repository. Tests are
> performed using ERT, see (info "(ert) Top") for the manual.

I've tried to see how it works with this, but it looks like the
temporary "session" session of Gnome keyring, which I use, doesn't care
about locking/unlocking of single items. So likely it isn't worth to
extend secrets-tests.el.

I'm closing the bug.

>> Cheers, Philipp

Best regards, Michael.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#62952; Package emacs. (Tue, 09 May 2023 08:16:02 GMT) Full text and rfc822 format available.

Message #22 received at 62952-done <at> debbugs.gnu.org (full text, mbox):

From: "Philipp Uhl" <git <at> ph-uhl.com>
To: "Michael Albinus" <michael.albinus <at> gmx.de>
Cc: 62952-done <at> debbugs.gnu.org
Subject: Re: bug#62952: 28.2.50; secrets.el unlocking items
Date: Tue, 09 May 2023 10:15:09 +0200
Hi Michael,

thank you. Sounds great.

Best,
Philipp

-----------------------------
  Philipp Uhl
  git <at> ph-uhl.com

Am Mo, 8. Mai 2023, um 13:42, schrieb Michael Albinus:
> Version: 30.1
>
> Michael Albinus <michael.albinus <at> gmx.de> writes:
>
> Hi Philipp,
>
>>> thanks for your response. Here is the secrets-lock-item function:
>>
>> Thanks. I'll check it next days.
>
> I've played this morning with your changes. Everything looks fine (in my
> environment), so I have pushed them to the Emacs master branch.
>
>>>> Bonus points for respective tests in secrets-tests.el.
>>>
>>> I didn't find any secrets-tests.el in the Emacs repository. Also I am
>>> not really familiar with writing test code in Elisp. But I did
>>> manually test the code and it works.
>>
>> See test/lisp/net/secrets-tests.el in the git repository. Tests are
>> performed using ERT, see (info "(ert) Top") for the manual.
>
> I've tried to see how it works with this, but it looks like the
> temporary "session" session of Gnome keyring, which I use, doesn't care
> about locking/unlocking of single items. So likely it isn't worth to
> extend secrets-tests.el.
>
> I'm closing the bug.
>
>>> Cheers, Philipp
>
> Best regards, Michael.




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Tue, 06 Jun 2023 11:24:07 GMT) Full text and rfc822 format available.

This bug report was last modified 2 years ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.