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

Previous Next

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.

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

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.

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)]

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.

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.

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.

Previous Next

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