GNU bug report logs -
#62952
28.2.50; secrets.el unlocking items
Previous Next
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.
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):
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):
"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):
[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):
"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):
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):
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.