rfc2104 change on 2008-02-01

rfc2104 change on 2008-02-01

[Simon Josefsson]

Hi.

I have tracked down a problem sending authenticated e-mail that resulted
in randomly failed CRAM-MD5 authentication (it sometimes work, usually
after 2-3 retries).  The cause is:

2008-02-01  Kenichi Handa  <handa@m17n.org>

        * rfc2104.el (rfc2104-hexstring-to-byte-list): Renamed from
        rfc2104-hexstring-to-bitstring and changed to return a byte list.
        (rfc2104-hash): Convert the result of concat to unibyte string.

The significant patch is:

-    (funcall hash (concat k_opad (rfc2104-hexstring-to-bitstring
-				  ;; perform inner hash
-				  (funcall hash (concat k_ipad text)))))))
+    (funcall hash
+	     (encode-coding-string
+	      (concat k_opad (rfc2104-hexstring-to-byte-list
+			      ;; perform inner hash
+			      (funcall hash (concat k_ipad text))))
+	      'iso-latin-1))))

The data to be hashed contains binary data, printed it just looks like
garbage.  Presumably, the reason the operation fails is because it can't
be encoded into latin-1 which as far as I recall have some reserved
parts.  This would be consistent with only happening sometimes, since it
may happen that a string doesn't contain any reserved characters.

I have experimented with fixes, and changed the 'iso-latin-1 to 'binary
and that appears to work.

There is nothing iso-latin-1 specific about CRAM-MD5 so if that is the
right thing, I think there should be a comment to explain why.

Handa, could you explain the motivation for this change?

To resolve it, we can chose:

1) Revert the patch.

2) Change 'iso-latin-1 to 'binary.

There may be other options too.

I propose to change it to 'binary if I don't hear anything.

If there were mailing list discussions about this, I'd appreciate a
pointer to that.  I don't think I was cc'ed, and I couldn't find any
discussion now that I search for it.

Thoughts?

Thanks,
Simon

Re: rfc2104 change on 2008-02-01

[Reiner Steib]

On Fri, Apr 25 2008, Simon Josefsson wrote:

> If there were mailing list discussions about this, I'd appreciate a
> pointer to that.  

This change was mentioned (but not really discussed) in the thread
"Emacs unicode merge changes to Gnus", ...

,----[ http://thread.gmane.org/gmane.emacs.gnus.general/66251/focus=66259 ]
| > 2008-02-01  Kenichi Handa  <handa@m17n.org>
| >
| > 	* rfc2104.el (rfc2104-hexstring-to-byte-list): Renamed from
| > 	rfc2104-hexstring-to-bitstring and changed to return a byte list.
| > 	(rfc2104-hash): Convert the result of concat to unibyte string.
| 
| `rfc2104.el' is only used in mail/smtpmail.el and net/imap.el, AFAICS.
| Maybe Simon can comment on this.
`----

> I don't think I was cc'ed, and I couldn't find any discussion now
> that I search for it.

... but I failed to cc you.

Bye, Reiner.
-- 
       ,,,
      (o o)
---ooO-(_)-Ooo---  |  PGP key available  |  http://rsteib.home.pages.de/

Re: rfc2104 change on 2008-02-01

[Simon Josefsson]

Reiner Steib <reinersteib+gmane@imap.cc> writes:

> On Fri, Apr 25 2008, Simon Josefsson wrote:
>
>> If there were mailing list discussions about this, I'd appreciate a
>> pointer to that.  
>
> This change was mentioned (but not really discussed) in the thread
> "Emacs unicode merge changes to Gnus", ...
>
> ,----[ http://thread.gmane.org/gmane.emacs.gnus.general/66251/focus=66259 ]
> | > 2008-02-01  Kenichi Handa  <handa@m17n.org>
> | >
> | > 	* rfc2104.el (rfc2104-hexstring-to-byte-list): Renamed from
> | > 	rfc2104-hexstring-to-bitstring and changed to return a byte list.
> | > 	(rfc2104-hash): Convert the result of concat to unibyte string.
> | 
> | `rfc2104.el' is only used in mail/smtpmail.el and net/imap.el, AFAICS.
> | Maybe Simon can comment on this.
> `----

Thanks for the pointer!

I just tried my proposed change with Emacs 23, and it didn't work.
Neither did the old code.  So I think someone who understands how to
deal with binary data in emacs 23 needs to look at this...

>> I don't think I was cc'ed, and I couldn't find any discussion now
>> that I search for it.
>
> ... but I failed to cc you.

Not your fault, I should have paid attention to the mailing list.

/Simon

Re: rfc2104 change on 2008-02-01

[Stefan Monnier]

> I have tracked down a problem sending authenticated e-mail that resulted
> in randomly failed CRAM-MD5 authentication (it sometimes work, usually
> after 2-3 retries).  The cause is:

> 2008-02-01  Kenichi Handa  <handa@m17n.org>

>         * rfc2104.el (rfc2104-hexstring-to-byte-list): Renamed from
>         rfc2104-hexstring-to-bitstring and changed to return a byte list.

This might be the problem: why return a byte-list rather than
a unicode string?

How 'bout the patch below?


        Stefan


--- rfc2104.el.~1.14.~	2008-04-10 21:03:54.000000000 -0400
+++ rfc2104.el	2008-04-25 10:27:16.000000000 -0400
@@ -89,12 +89,12 @@
 	(rfc2104-hex-to-int (reverse (append str nil))))
     0))
 
-(defun rfc2104-hexstring-to-byte-list (str)
+(defun rfc2104-hexstring-to-bitstring (str)
   (let (out)
     (while (< 0 (length str))
       (push (rfc2104-hex-to-int (substring str -2)) out)
       (setq str (substring str 0 -2)))
-    out))
+    (apply (if (fboundp 'unibyte-string) 'unibyte-string 'string) out)))
 
 (defun rfc2104-hash (hash block-length hash-length key text)
   (let* (;; if key is longer than B, reset it to HASH(key)
@@ -111,12 +111,9 @@
     (setq k_ipad (mapcar (lambda (c) (logxor c rfc2104-ipad)) k_ipad))
     (setq k_opad (mapcar (lambda (c) (logxor c rfc2104-opad)) k_opad))
     ;; perform outer hash
-    (funcall hash
-	     (encode-coding-string
-	      (concat k_opad (rfc2104-hexstring-to-byte-list
+    (funcall hash (concat k_opad (rfc2104-hexstring-to-bitstring
 			      ;; perform inner hash
-			      (funcall hash (concat k_ipad text))))
-	      'iso-latin-1))))
+				  (funcall hash (concat k_ipad text)))))))
 
 (provide 'rfc2104)
 

Re: rfc2104 change on 2008-02-01

[Thien-Thi Nguyen]

[-- Attachment #1: Type: text/plain, Size: 404 bytes --]

() Simon Josefsson <simon@josefsson.org>
() Fri, 25 Apr 2008 15:40:39 +0200

   I just tried my proposed change with Emacs 23,
   and it didn't work.

How about changing:
(encode-coding-string foo 'iso-latin-1)

to:
(string-make-unibyte foo)

?

Better yet, attached is a rewrite for speed
(plus some incidental Commentary correction).

thi



___________________________________________________________

[-- Attachment #2: \"high-performance emacs lisp\"? hmmm... --]
[-- Type: application/emacs-lisp, Size: 4130 bytes --]

Re: rfc2104 change on 2008-02-01

[Gabor Z. Papp]

* Stefan Monnier <monnier@iro.umontreal.ca>:

| > I have tracked down a problem sending authenticated e-mail that resulted
| > in randomly failed CRAM-MD5 authentication (it sometimes work, usually
| > after 2-3 retries).  The cause is:

I had this problem:

http://article.gmane.org/gmane.emacs.gnus.general/66738

Problem still exist in cvs head (20080426).
After using your patch the problem disappeared.