[sup-devel] GPGME fixes

* [sup-devel] GPGME fixes
@ 2011-02-17 14:49 Gaudenz Steinlin
  2011-02-17 18:45 ` Hamish D
  0 siblings, 1 reply; 4+ messages in thread
From: Gaudenz Steinlin @ 2011-02-17 14:49 UTC (permalink / raw)
  To: sup-devel


[-- Attachment #1.1.1: Type: text/plain, Size: 387 bytes --]

Hi

While reimporting all my messages I encountered a few problemes with
the GPGME code. Please consider the attached fixes.

The second patch depends on the first (error message unification). But
if you don't like the first I can easily produce a standalone version.

Gaudenz

-- 
Ever tried. Ever failed. No matter.
Try again. Fail again. Fail better.
~ Samuel Beckett ~

[-- Attachment #1.1.2: 0001-Encode-multipart-messages-for-crypt-operations.patch --]
[-- Type: application/octet-stream, Size: 2180 bytes --]

From ca2511220d1f0198a4e73432e87774a8fe30fba9 Mon Sep 17 00:00:00 2001
From: Gaudenz Steinlin <gaudenz@soziologie.ch>
Date: Tue, 12 Oct 2010 23:20:53 +0200
Subject: [PATCH 1/2] Encode multipart messages for crypt operations

Sup crashed when trying to encode a multipart message for crypto
operations. This encodes each part of the message separately. It also
changes the encoding for text/* parts to quoted-printable.

This only concerns the transfer encoding and does not change the charset
in any way.
---
 lib/sup/modes/edit-message-mode.rb |   26 ++++++++++++++++++++++++--
 1 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/lib/sup/modes/edit-message-mode.rb b/lib/sup/modes/edit-message-mode.rb
index f27eb5c..1bf9378 100644
--- a/lib/sup/modes/edit-message-mode.rb
+++ b/lib/sup/modes/edit-message-mode.rb
@@ -403,8 +403,11 @@ protected
     if @crypto_selector && @crypto_selector.val != :none
       from_email = Person.from_address(@header["From"]).email
       to_email = [@header["To"], @header["Cc"], @header["Bcc"]].flatten.compact.map { |p| Person.from_address(p).email }
-      m.header["Content-Transfer-Encoding"] = 'base64'
-      m.body = [m.body].pack('m')
+      if m.multipart?
+        m.each_part {|p| p = transfer_encode p}
+      else
+        m = transfer_encode m
+      end
 
       m = CryptoManager.send @crypto_selector.val, from_email, to_email, m
     end
@@ -520,6 +523,25 @@ private
       []
     end
   end
+
+  def transfer_encode msg_part
+    ## return the message unchanged if it's already encoded
+    if (msg_part.header["Content-Transfer-Encoding"] == "base64" ||
+        msg_part.header["Content-Transfer-Encoding"] == "quoted-printable")
+      return msg_part
+    end
+
+    ## encode to quoted-printable for all text/* MIME types,
+    ## use base64 otherwise
+    if msg_part.header["Content-Type"] =~ /text\/.*/
+      msg_part.header["Content-Transfer-Encoding"] = 'quoted-printable'
+      msg_part.body = [msg_part.body].pack('M')
+    else
+      msg_part.header["Content-Transfer-Encoding"] = 'base64'
+      msg_part.body = [msg_part.body].pack('m')
+    end
+    msg_part
+  end
 end
 
 end
-- 
1.7.2.3


[-- Attachment #1.1.3: 0001-Unify-formatting-of-GPGME-error-messages.patch --]
[-- Type: application/octet-stream, Size: 2413 bytes --]

From b52a306ed119715e5dac26f35a406ecdd4c5fb87 Mon Sep 17 00:00:00 2001
From: Gaudenz Steinlin <gaudenz@soziologie.ch>
Date: Thu, 17 Feb 2011 15:09:23 +0100
Subject: [PATCH 1/3] Unify formatting of GPGME error messages

Create a private function to turn a an exception into a "human friendly"
error message and to add a entry to the logfile. Use this function for
all error paths.
---
 lib/sup/crypto.rb |   17 ++++++++++-------
 1 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/lib/sup/crypto.rb b/lib/sup/crypto.rb
index 2bfb475..a3771bf 100644
--- a/lib/sup/crypto.rb
+++ b/lib/sup/crypto.rb
@@ -75,8 +75,7 @@ EOS
     begin
       sig = GPGME.detach_sign(format_payload(payload), gpg_opts)
     rescue GPGME::Error => exc
-      info "Error while running gpg: #{exc.message}"
-      raise Error, "GPG command failed. See log for details."
+      raise Error, gpgme_exc_msg(exc.message)
     end
 
     envelope = RMail::Message.new
@@ -103,8 +102,7 @@ EOS
     begin
       cipher = GPGME.encrypt(recipients, format_payload(payload), gpg_opts)
     rescue GPGME::Error => exc
-      info "Error while running gpg: #{exc.message}"
-      raise Error, "GPG command failed. See log for details."
+      raise Error, gpgme_exc_msg(exc.message)
     end
 
     encrypted_payload = RMail::Message.new
@@ -183,7 +181,7 @@ EOS
     begin
       ctx.verify(sig_data, signed_text_data, plain_data)
     rescue GPGME::Error => exc
-      return unknown_status exc.message
+      return unknown_status [gpgme_exc_msg(exc.message)]
     end
     self.verified_ok? ctx.verify_result
   end
@@ -201,8 +199,7 @@ EOS
     begin
       ctx.decrypt_verify(cipher_data, plain_data)
     rescue GPGME::Error => exc
-      info "Error while running gpg: #{exc.message}"
-      return Chunk::CryptoNotice.new(:invalid, "This message could not be decrypted", exc.message)
+      return Chunk::CryptoNotice.new(:invalid, "This message could not be decrypted", gpgme_exc_msg(exc.message))
     end
     sig = self.verified_ok? ctx.verify_result
     plain_data.seek(0, IO::SEEK_SET)
@@ -264,6 +261,12 @@ private
     ["Can't find gpgme gem."]
   end
 
+  def gpgme_exc_msg msg
+    err_msg = "Exception in GPGME call: #{msg}"
+    info err_msg
+    err_msg
+  end
+
   ## here's where we munge rmail output into the format that signed/encrypted
   ## PGP/GPG messages should be
   def format_payload payload
-- 
1.7.2.3


[-- Attachment #1.1.4: 0002-Check-for-ArgumentError-on-signature-verification.patch --]
[-- Type: application/octet-stream, Size: 1614 bytes --]

From f86d0bb6c2cc9a1d8ef544a34f9c10f73850767c Mon Sep 17 00:00:00 2001
From: Gaudenz Steinlin <gaudenz@soziologie.ch>
Date: Thu, 17 Feb 2011 15:13:31 +0100
Subject: [PATCH 2/3] Check for ArgumentError on signature verification

Broken signatures can cause a NULL pointer which results in an
ArguementError when calling ctx.verify_result even if the previous call
to ctx.verify does not raise an exception.

The underling cause is probably a bug in GPGME.
---
 lib/sup/crypto.rb |   12 ++++++++++--
 1 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/lib/sup/crypto.rb b/lib/sup/crypto.rb
index a3771bf..cbd0751 100644
--- a/lib/sup/crypto.rb
+++ b/lib/sup/crypto.rb
@@ -183,7 +183,11 @@ EOS
     rescue GPGME::Error => exc
       return unknown_status [gpgme_exc_msg(exc.message)]
     end
-    self.verified_ok? ctx.verify_result
+    begin
+      self.verified_ok? ctx.verify_result
+    rescue ArgumentError => exc
+      return unknown_status [gpgme_exc_msg(exc.message)]
+    end
   end
 
   ## returns decrypted_message, status, desc, lines
@@ -201,7 +205,11 @@ EOS
     rescue GPGME::Error => exc
       return Chunk::CryptoNotice.new(:invalid, "This message could not be decrypted", gpgme_exc_msg(exc.message))
     end
-    sig = self.verified_ok? ctx.verify_result
+    begin
+      sig = self.verified_ok? ctx.verify_result
+    rescue ArgumentError => exc
+      sig = unknown_status [gpgme_exc_msg(exc.message)]
+    end
     plain_data.seek(0, IO::SEEK_SET)
     output = plain_data.read
     output.force_encoding Encoding::ASCII_8BIT if output.respond_to? :force_encoding
-- 
1.7.2.3


[-- Attachment #1.1.5: 0003-Check-for-valid-signature-before-signature.to_s.patch --]
[-- Type: application/octet-stream, Size: 1290 bytes --]

From 0a242c50f93319a47217dd577089732f4fbf0664 Mon Sep 17 00:00:00 2001
From: Gaudenz Steinlin <gaudenz@soziologie.ch>
Date: Thu, 17 Feb 2011 15:26:41 +0100
Subject: [PATCH 3/3] Check for valid signature before signature.to_s

In some circumstances with broken signatures signature.to_s returns nil.
Check for a valid signature before relying on this string. This avoids
an uncaught exception.
---
 lib/sup/crypto.rb |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/lib/sup/crypto.rb b/lib/sup/crypto.rb
index cbd0751..9ab8607 100644
--- a/lib/sup/crypto.rb
+++ b/lib/sup/crypto.rb
@@ -292,7 +292,13 @@ private
     ctx = GPGME::Ctx.new
     begin
       from_key = ctx.get_key(signature.fingerprint)
-      first_sig = signature.to_s.sub(/from [0-9A-F]{16} /, 'from "') + '"'
+      if GPGME::gpgme_err_code(signature.status) == GPGME::GPG_ERR_GENERAL
+        first_sig = "General error on signature verification for #{signature.fingerprint}"
+      elsif signature.to_s
+        first_sig = signature.to_s.sub(/from [0-9A-F]{16} /, 'from "') + '"'
+      else
+        first_sig = "Unknown error or empty signature"
+      end
     rescue EOFError
       from_key = nil
       first_sig = "No public key available for #{signature.fingerprint}"
-- 
1.7.2.3


[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 482 bytes --]

[-- Attachment #2: Type: text/plain, Size: 143 bytes --]

_______________________________________________
Sup-devel mailing list
Sup-devel@rubyforge.org
http://rubyforge.org/mailman/listinfo/sup-devel

^ permalink raw reply	[flat|nested] 4+ messages in thread