Re: [sup-devel] [PATCH] Converted crypto to use the gpgme gem

From: Hamish D <dmishd@gmail.com>
To: Sup developer discussion <sup-devel@rubyforge.org>
Subject: Re: [sup-devel] [PATCH] Converted crypto to use the gpgme gem
Date: Mon, 6 Dec 2010 23:31:43 +0000	[thread overview]
Message-ID: <AANLkTi=AJZGLAra20fQvEvU0EasQ_1yWpsr6+4TV3iEn@mail.gmail.com> (raw)
In-Reply-To: <1291023322-sup-8457@meteor.durcheinandertal.local>

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

> I just discovered another problem: If the secret key is not available
> (because it's on a removable media and the media is not mounted), the
> mail is sent anyway. While this is just a bit annoying for signed mail
> it definitely should not happen for encrypted mails. Current sup
> corectly fails in this case.

I have replicated this (by turning off gpg agent) but I'm confused as
to why this is happening. If I try the same steps in irb I get an
exception, and this should be caught and dealt with in the same way as
current sup does. I guess I'll have to keep trying to replicate more
and more of the path way through ... sigh.

Once I have worked out the proper logic I can then add some extra
checks for ensuring that gpg agent is running and that sup knows where
to find it. I could even have sup ask you for your gpg passphrase with
gpgme. There might be some security issues with having ruby ask you
for your passphrase I guess, but I don't think it would be worse than
gpg agent. gpg agent doesn't seem to have the suid bit set, though
maybe as a C program it can be more rigorous about overwriting your
passphrase in memory. I could always implement it as a hook with gpg
agent as the default.

> It would also be nice to have different colors for different trust
> levels. So you don't have to expand the extra information to see if a
> valid signature is trusted or not. Is this already possible with the
> current hook?

That requires code changes, but I've done that and attached a patch
(intended to go on top of the other 4 patches). Now untrusted
signatures have a blue background. (Trusted signatures have a default
background - black normally, and bad signatures have a red
background). All signatures have yellow text.

I'm quite open to a different colour scheme being chosen if someone
thinks something else would be clearer.

Hamish Downer

[-- Attachment #2: 0005-added-color-for-untrusted-cryptonotice.patch --]
[-- Type: text/x-patch, Size: 4171 bytes --]

From b99343dd358361ac47c9e00198b52df28fbd95cc Mon Sep 17 00:00:00 2001
From: Hamish Downer <dmishd@gmail.com>
Date: Mon, 6 Dec 2010 22:33:17 +0000
Subject: [PATCH 5/5] added color for untrusted cryptonotice

---
 lib/sup/colormap.rb       |    1 +
 lib/sup/crypto.rb         |   29 ++++++++++++++++++++---------
 lib/sup/message-chunks.rb |    1 +
 3 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/lib/sup/colormap.rb b/lib/sup/colormap.rb
index 7de48db..8402add 100644
--- a/lib/sup/colormap.rb
+++ b/lib/sup/colormap.rb
@@ -40,6 +40,7 @@ class Colormap
     :missing_message => { :fg => "black", :bg => "red" },
     :attachment => { :fg => "cyan", :bg => "default" },
     :cryptosig_valid => { :fg => "yellow", :bg => "default", :attrs => ["bold"] },
+    :cryptosig_valid_untrusted => { :fg => "yellow", :bg => "blue", :attrs => ["bold"] },
     :cryptosig_unknown => { :fg => "cyan", :bg => "default" },
     :cryptosig_invalid => { :fg => "yellow", :bg => "red", :attrs => ["bold"] },
     :generic_notice_patina => { :fg => "cyan", :bg => "default" },
diff --git a/lib/sup/crypto.rb b/lib/sup/crypto.rb
index b9ffb17..e532261 100644
--- a/lib/sup/crypto.rb
+++ b/lib/sup/crypto.rb
@@ -121,11 +121,15 @@ EOS
   def verified_ok? verify_result
     valid = true
     unknown = false
-    output_lines = []
+    all_output_lines = []
+    all_trusted = true
 
     verify_result.signatures.each do |signature|
-      output_lines.push(sig_output_lines(signature))
-      output_lines.flatten!
+      output_lines, trusted = sig_output_lines signature
+      all_output_lines << output_lines
+      all_output_lines.flatten!
+      all_trusted &&= trusted
+
       err_code = GPGME::gpgme_err_code(signature.status)
       if err_code == GPGME::GPG_ERR_BAD_SIGNATURE
         valid = false 
@@ -135,14 +139,18 @@ EOS
       end
     end
 
-    if output_lines.length == 0
-      Chunk::CryptoNotice.new :valid, "Encrypted message wasn't signed", output_lines
+    if all_output_lines.length == 0
+      Chunk::CryptoNotice.new :valid, "Encrypted message wasn't signed", all_output_lines
     elsif valid
-      Chunk::CryptoNotice.new(:valid, simplify_sig_line(verify_result.signatures[0].to_s), output_lines)
+      if all_trusted
+        Chunk::CryptoNotice.new(:valid, simplify_sig_line(verify_result.signatures[0].to_s), all_output_lines)
+      else
+        Chunk::CryptoNotice.new(:valid_untrusted, simplify_sig_line(verify_result.signatures[0].to_s), all_output_lines)
+      end
     elsif !unknown
-      Chunk::CryptoNotice.new(:invalid, simplify_sig_line(verify_result.signatures[0].to_s), output_lines)
+      Chunk::CryptoNotice.new(:invalid, simplify_sig_line(verify_result.signatures[0].to_s), all_output_lines)
     else
-      unknown_status output_lines
+      unknown_status all_output_lines
     end
   end
 
@@ -273,6 +281,7 @@ private
                 "key ID " + signature.fingerprint[-8..-1]
     output_lines = [time_line, first_sig]
 
+    trusted = false
     if from_key 
       # first list all the uids
       if from_key.uids.length > 1
@@ -284,13 +293,15 @@ private
       if signature.validity != GPGME::GPGME_VALIDITY_FULL && signature.validity != GPGME::GPGME_VALIDITY_MARGINAL
         output_lines << "WARNING: This key is not certified with a trusted signature!"
         output_lines << "There is no indication that the signature belongs to the owner"
+      else
+        trusted = true
       end
 
       # finally, run the hook
       output_lines << HookManager.run("sig-output",
                                {:signature => signature, :from_key => from_key})
     end
-    output_lines
+    return output_lines, trusted
   end
 
   def key_type key, fpr
diff --git a/lib/sup/message-chunks.rb b/lib/sup/message-chunks.rb
index 02d28f6..0097450 100644
--- a/lib/sup/message-chunks.rb
+++ b/lib/sup/message-chunks.rb
@@ -272,6 +272,7 @@ EOS
     def patina_color
       case status
       when :valid then :cryptosig_valid_color
+      when :valid_untrusted then :cryptosig_valid_untrusted_color
       when :invalid then :cryptosig_invalid_color
       else :cryptosig_unknown_color
       end
-- 
1.7.1


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

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