GNU bug report logs - #70301
30.0.50; secrets-create-item mangles cyrillic passwords

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 70301 in the body.
You can then email your comments to 70301 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: k.ninev <at> cdots.bg
To: bug-gnu-emacs <at> gnu.org
Subject: 30.0.50; secrets-create-item mangles cyrillic passwords
Date: Tue, 09 Apr 2024 08:18:37 +0000

using this Secret Service API code in 'emacs -Q'

(require 'secrets)
(secrets-create-item "session" "my item" "парола" :method "sudo" :user
"joe" :host "remote-host")

Successfully creates an entry but the password "парола" is mangled to
"?0@>;0" and cannot be viewed both with M-x secrets-show-secrets 
and secret-tool cli tool , this does not occur with latin passwords

Tested with both gnome-keyring and keepassxc as API backends.


--text follows this line-
In GNU Emacs 30.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version
 3.24.41, cairo version 1.18.0) of 2024-04-08 built on
 arch-nspawn-197981
Repository revision: 600ac35cc6597b63306786bffd0d762f70555322
Repository branch: master
System Description: Arch Linux

Configured using:
 'configure --prefix=/usr --sysconfdir=/etc --libexecdir=/usr/lib
 --localstatedir=/var --mandir=/usr/share/man --with-gameuser=:games
 --with-modules --without-m17n-flt --without-gconf
 --with-native-compilation=yes --with-xinput2 --with-pgtk
 --without-xaw3d --with-sound=no --with-tree-sitter --without-gpm
 --without-compress-install
 '--program-transform-name=s/\([ec]tags\)/\1.emacs/'
 'CFLAGS=-march=x86-64 -mtune=generic -O2 -pipe -fno-plt -fexceptions
 -Wp,-D_FORTIFY_SOURCE=2 -Wformat -Werror=format-security
 -fstack-clash-protection -fcf-protection -g
 -ffile-prefix-map=/build/emacs-git/src=/usr/src/debug/emacs-git
 -flto=auto' 'LDFLAGS=-Wl,-O1 -Wl,--sort-common -Wl,--as-needed
 -Wl,-z,relro -Wl,-z,now -Wl,-z,pack-relative-relocs -flto=auto''

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GSETTINGS HARFBUZZ JPEG
LCMS2 LIBOTF LIBSYSTEMD LIBXML2 MODULES NATIVE_COMP NOTIFY INOTIFY
PDUMPER PGTK PNG RSVG SECCOMP SQLITE3 THREADS TIFF TOOLKIT_SCROLL_BARS
TREE_SITTER WEBP XIM GTK3 ZLIB

Important settings:
  value of $LC_MONETARY: bg_BG.UTF-8
  value of $LC_NUMERIC: bg_BG.UTF-8
  value of $LC_TIME: bg_BG.UTF-8
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Fundamental

-- 
KN

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: k.ninev <at> cdots.bg, Michael Albinus <michael.albinus <at> gmx.de>
Cc: 70301 <at> debbugs.gnu.org
Subject: Re: bug#70301: 30.0.50; secrets-create-item mangles cyrillic passwords
Date: Tue, 09 Apr 2024 12:48:37 +0300

> Date: Tue, 09 Apr 2024 08:18:37 +0000
> From: k.ninev--- via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
> 
> 
> using this Secret Service API code in 'emacs -Q'
> 
> (require 'secrets)
> (secrets-create-item "session" "my item" "парола" :method "sudo" :user
> "joe" :host "remote-host")
> 
> Successfully creates an entry but the password "парола" is mangled to
> "?0@>;0" and cannot be viewed both with M-x secrets-show-secrets 
> and secret-tool cli tool , this does not occur with latin passwords

Does the patch below give good results?

Michael, I think we have a similar problem in
dbus-byte-array-to-string: when MULTIBYTE is non-nil, the function
should call decode-coding-string on the unibyte string it produces
instead of converting each byte to multibyte.  Because the bytes in
the argument BYTE-ARRAY are not characters, they are raw bytes of the
UTF-8 sequence, so calling 'string' on them is not TRT.

diff --git a/lisp/net/secrets.el b/lisp/net/secrets.el
index c1a670f..63fc874 100644
--- a/lisp/net/secrets.el
+++ b/lisp/net/secrets.el
@@ -665,7 +665,8 @@ secrets-create-item
 	     ;; Secret.
 	     `(:struct :object-path ,secrets-session-path
 		       (:array :signature "y") ;; No parameters.
-		       ,(dbus-string-to-byte-array password)
+		       ,(dbus-string-to-byte-array
+                         (encode-coding-string password 'utf-8))
                        ,secrets-struct-secret-content-type)
 	     ;; Do not replace. Replace does not seem to work.
 	     nil))

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

From: k.ninev <at> cdots.bg
To: Eli Zaretskii <eliz <at> gnu.org>, Michael Albinus <michael.albinus <at> gmx.de>
Cc: 70301 <at> debbugs.gnu.org
Subject: Re: bug#70301: 30.0.50; secrets-create-item mangles cyrillic passwords
Date: Tue, 09 Apr 2024 10:09:19 +0000

Yes, the patch works for me. Thank you.

On Tuesday, April 9, 2024 12:48 PM (+03:00), Eli Zaretskii wrote:

> > Date: Tue, 09 Apr 2024 08:18:37 +0000
> > From: k.ninev--- via "Bug reports for GNU Emacs,
> >  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
> > 
> > 
> > using this Secret Service API code in 'emacs -Q'
> > 
> > (require 'secrets)
> > (secrets-create-item "session" "my item" "парола" :method "sudo" :user
> > "joe" :host "remote-host")
> > 
> > Successfully creates an entry but the password "парола" is mangled to
> > "?0@>;0" and cannot be viewed both with M-x secrets-show-secrets 
> > and secret-tool cli tool , this does not occur with latin passwords
> 
> Does the patch below give good results?
> 
> Michael, I think we have a similar problem in
> dbus-byte-array-to-string: when MULTIBYTE is non-nil, the function
> should call decode-coding-string on the unibyte string it produces
> instead of converting each byte to multibyte.  Because the bytes in
> the argument BYTE-ARRAY are not characters, they are raw bytes of the
> UTF-8 sequence, so calling 'string' on them is not TRT.
> 
> diff --git a/lisp/net/secrets.el b/lisp/net/secrets.el
> index c1a670f..63fc874 100644
> --- a/lisp/net/secrets.el
> +++ b/lisp/net/secrets.el
> @@ -665,7 +665,8 @@ secrets-create-item
>  	     ;; Secret.
>  	     `(:struct :object-path ,secrets-session-path
>  		       (:array :signature "y") ;; No parameters.
> -		       ,(dbus-string-to-byte-array password)
> +		       ,(dbus-string-to-byte-array
> +                         (encode-coding-string password 'utf-8))
>                         ,secrets-struct-secret-content-type)
>  	     ;; Do not replace. Replace does not seem to work.
>  	     nil))
> 

-- 
KN

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: k.ninev <at> cdots.bg
Cc: 70301 <at> debbugs.gnu.org, michael.albinus <at> gmx.de
Subject: Re: bug#70301: 30.0.50; secrets-create-item mangles cyrillic passwords
Date: Tue, 09 Apr 2024 13:23:58 +0300

> From: k.ninev <at> cdots.bg
> Cc: 70301 <at> debbugs.gnu.org
> Date: Tue, 09 Apr 2024 10:09:19 +0000
> 
> 
> Yes, the patch works for me. Thank you.

Thanks for testing.

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

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 70301 <at> debbugs.gnu.org, k.ninev <at> cdots.bg
Subject: Re: bug#70301: 30.0.50; secrets-create-item mangles cyrillic passwords
Date: Wed, 10 Apr 2024 17:32:43 +0200

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> gnu.org> writes:

Hi Eli,

> Does the patch below give good results?

Thanks for the patch. However, I believe we shouldn't fix it in
secrets.el, but in dbus.el.

> Michael, I think we have a similar problem in
> dbus-byte-array-to-string: when MULTIBYTE is non-nil, the function
> should call decode-coding-string on the unibyte string it produces
> instead of converting each byte to multibyte.  Because the bytes in
> the argument BYTE-ARRAY are not characters, they are raw bytes of the
> UTF-8 sequence, so calling 'string' on them is not TRT.

Yes. The following patch works for me. WDYT?

[Message part 2 (text/x-patch, inline)]

diff --git a/lisp/net/dbus.el b/lisp/net/dbus.el
index 46f85daba24..11c87e72d4e 100644
--- a/lisp/net/dbus.el
+++ b/lisp/net/dbus.el
@@ -997,17 +997,18 @@ dbus-string-to-byte-array
 STRING shall be UTF-8 coded."
   (if (zerop (length string))
       '(:array :signature "y")
-    (cons :array (mapcan (lambda (c) (list :byte c)) string))))
+    (cons :array
+          (mapcan (lambda (c) (list :byte c))
+                  (encode-coding-string string 'utf-8)))))

-(defun dbus-byte-array-to-string (byte-array &optional multibyte)
+(defun dbus-byte-array-to-string (byte-array &optional _multibyte)
   "Transform BYTE-ARRAY into UTF-8 coded string.
-BYTE-ARRAY must be a list of structure (c1 c2 ...), or a byte
-array as produced by `dbus-string-to-byte-array'.  The resulting
-string is unibyte encoded, unless MULTIBYTE is non-nil."
-  (apply
-   (if multibyte #'string #'unibyte-string)
-   (unless (equal byte-array '(:array :signature "y"))
-     (seq-filter #'characterp byte-array))))
+BYTE-ARRAY must be a unibyte list of structure (c1 c2 ...), or a byte
+array as produced by `dbus-string-to-byte-array'."
+   (if-let ((bytes (seq-filter #'characterp byte-array))
+            (string (apply #'unibyte-string bytes)))
+       (decode-coding-string string 'utf-8)
+     ""))

 (defun dbus-escape-as-identifier (string)
   "Escape an arbitrary STRING so it follows the rules for a C identifier.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: 70301 <at> debbugs.gnu.org, k.ninev <at> cdots.bg
Subject: Re: bug#70301: 30.0.50; secrets-create-item mangles cyrillic passwords
Date: Wed, 10 Apr 2024 18:44:35 +0300

> From: Michael Albinus <michael.albinus <at> gmx.de>
> Cc: k.ninev <at> cdots.bg,  70301 <at> debbugs.gnu.org
> Date: Wed, 10 Apr 2024 17:32:43 +0200
> 
> > Does the patch below give good results?
> 
> Thanks for the patch. However, I believe we shouldn't fix it in
> secrets.el, but in dbus.el.

Then we ought to update the doc string of dbus-string-to-byte-array,
which currently says the argument should be a UTF-8 encoded string.

> > Michael, I think we have a similar problem in
> > dbus-byte-array-to-string: when MULTIBYTE is non-nil, the function
> > should call decode-coding-string on the unibyte string it produces
> > instead of converting each byte to multibyte.  Because the bytes in
> > the argument BYTE-ARRAY are not characters, they are raw bytes of the
> > UTF-8 sequence, so calling 'string' on them is not TRT.
> 
> Yes. The following patch works for me. WDYT?

If you are okay with changing the behavior of
dbus-byte-array-to-string in backward-incompatible ways, I don't mind.
But maybe this should be called out in NEWS.

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

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 70301 <at> debbugs.gnu.org, k.ninev <at> cdots.bg
Subject: Re: bug#70301: 30.0.50; secrets-create-item mangles cyrillic passwords
Date: Wed, 10 Apr 2024 18:10:00 +0200

Eli Zaretskii <eliz <at> gnu.org> writes:

Hi Eli,

>> Thanks for the patch. However, I believe we shouldn't fix it in
>> secrets.el, but in dbus.el.
>
> Then we ought to update the doc string of dbus-string-to-byte-array,
> which currently says the argument should be a UTF-8 encoded string.

Yep.

>> > Michael, I think we have a similar problem in
>> > dbus-byte-array-to-string: when MULTIBYTE is non-nil, the function
>> > should call decode-coding-string on the unibyte string it produces
>> > instead of converting each byte to multibyte.  Because the bytes in
>> > the argument BYTE-ARRAY are not characters, they are raw bytes of the
>> > UTF-8 sequence, so calling 'string' on them is not TRT.
>>
>> Yes. The following patch works for me. WDYT?
>
> If you are okay with changing the behavior of
> dbus-byte-array-to-string in backward-incompatible ways, I don't mind.
> But maybe this should be called out in NEWS.

I'm aware of this incompatibility. Well, I've played several hours today
with this. Since we have no information from D-Bus about the underlying
byte array, we must assume a given context. A sequence of raw 8-bit
bytes, unibyte, representing an UTF-8 encoded string seems to be aapropriate.

If people want another conversion for a D-Bus byte array, it is still
possible. But they shouldn't use dbus-byte-array-to-string and
dbus-string-to-byte-array then.

I'll puzzle everything together by tomorrow.

Best regards, <<michael.

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

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 70301 <at> debbugs.gnu.org, k.ninev <at> cdots.bg
Subject: Re: bug#70301: 30.0.50; secrets-create-item mangles cyrillic passwords
Date: Fri, 12 Apr 2024 10:14:18 +0200

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

> I'll puzzle everything together by tomorrow.

Pushed to master. Could you, pls, check whether it works as expected?

Best regards, <<michael.

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

From: k.ninev <at> cdots.bg
To: Michael Albinus <michael.albinus <at> gmx.de>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 70301 <at> debbugs.gnu.org
Subject: Re: bug#70301: 30.0.50; secrets-create-item mangles cyrillic passwords
Date: Fri, 12 Apr 2024 08:51:38 +0000

> Pushed to master. Could you, pls, check whether it works as expected?

It works as expected, thank you.
-- 
KN

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

From: Michael Albinus <michael.albinus <at> gmx.de>
To: k.ninev <at> cdots.bg
Cc: Eli Zaretskii <eliz <at> gnu.org>, 70301-done <at> debbugs.gnu.org
Subject: Re: bug#70301: 30.0.50; secrets-create-item mangles cyrillic passwords
Date: Fri, 12 Apr 2024 10:59:56 +0200

Version: 30.1

k.ninev <at> cdots.bg writes:

Hi,

>> Pushed to master. Could you, pls, check whether it works as expected?
>
> It works as expected, thank you.

Thanks for the feedback. Closing the bug.

Best regards, Michael.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: 70301 <at> debbugs.gnu.org, k.ninev <at> cdots.bg
Subject: Re: bug#70301: 30.0.50; secrets-create-item mangles cyrillic passwords
Date: Fri, 12 Apr 2024 14:37:04 +0300

> From: Michael Albinus <michael.albinus <at> gmx.de>
> Cc: Eli Zaretskii <eliz <at> gnu.org>,  70301-done <at> debbugs.gnu.org
> Date: Fri, 12 Apr 2024 10:59:56 +0200
> 
> >> Pushed to master. Could you, pls, check whether it works as expected?
> >
> > It works as expected, thank you.
> 
> Thanks for the feedback. Closing the bug.

Thanks.  I needed to fix some inaccuracies in the documentation,
please take a look.

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

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 70301 <at> debbugs.gnu.org, k.ninev <at> cdots.bg
Subject: Re: bug#70301: 30.0.50; secrets-create-item mangles cyrillic passwords
Date: Fri, 12 Apr 2024 15:53:50 +0200

Eli Zaretskii <eliz <at> gnu.org> writes:

Hi Eli,

> I needed to fix some inaccuracies in the documentation, please take a
> look.

Thanks!

Best regards, Michael.

Previous Next

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