[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



OK, the second patch fixes the problem with the first patch.

Hamish Downer

On 8 November 2010 11:21, Hamish D <dmishd@gmail.com> wrote:
> Best hang fire on this patch. It appears to crash when verifying a signature
> when the public key is not available. I'm investigating the problem and how
> to fix it cleanly and I'll resubmit once I've done that.
>
> Hamish
>
> On Nov 6, 2010 8:08 PM, "Hamish D" <dmishd@gmail.com> wrote:
>
> I often find that loading long threads of encrypted messages (I have
> several of over 10 messages and one of nearly 40) leads to lots of
> flickering as the console replaces sup, sup comes back, the console
> comes back again ... It is also very slow, and involves writing
> decrypted messages to disk (if only temporarily) which could be a
> security hole. So I've looked about and found the gpgme gem which
> provides an API to use, and allows decryption entirely in memory.
>
> So I've rewritten lib/sup/crypto.rb to use gpgme. The functionality is
> pretty much the same. Things I'm aware of that are different:
>
> * we can't set the signature algorithm, so we have to use whatever is
> set in the user's preferences
> * the gpg-args hook has been replaced by the gpg-options hook
>
> Other than that I think it is the same, although it took some work to
> get the signature output to be the same. The other main difference is
> that it's much faster and nicer now :)
>
> It could do with some testing - I don't have much in the way of
> messages that cause gpg to complain, so if you do, please try opening
> those messages with this code and see if the behaviour is reasonable -
> no crashes, given messages about why your message was bad etc.
>
> Also I guess I should ask if people are happy to use this gem. Is it
> hard to use on Macs? I guess I could rewrite this patch so it falls
> back to the gpg binary if gpgme is not available ...
>
> To install this patch on Debian/Ubuntu you can either
>
> * apt-get install libgpgme-ruby
> * apt-get install libgpgme11-dev; gem install gpgme
>
> Hamish Downer
>
From 52441d1eb749bb1e3b5026e42a334e9c8f455833 Mon Sep 17 00:00:00 2001
From: Hamish Downer <dmishd@gmail.com>
Date: Fri, 5 Nov 2010 22:30:55 +0000
Subject: [PATCH 1/2] Converted crypto to use the gpgme gem

---
 bin/sup           |   11 +++
 lib/sup/crypto.rb |  231 ++++++++++++++++++++++++++++++-----------------------
 2 files changed, 141 insertions(+), 101 deletions(-)

diff --git a/bin/sup b/bin/sup
index 10be161..ad7a0d1 100755
--- a/bin/sup
+++ b/bin/sup
@@ -10,6 +10,13 @@ rescue LoadError
   no_ncursesw = true
 end
 
+no_gpgme = false
+begin
+  require 'gpgme'
+rescue LoadError
+  no_gpgme = true
+end
+
 require 'fileutils'
 require 'trollop'
 require "sup"; Redwood::check_library_version_against "git"
@@ -23,6 +30,10 @@ if no_ncursesw
   info "No 'ncursesw' gem detected. Install it for wide character support."
 end
 
+if no_gpgme
+  info "No 'gpgme' gem detected. Install it for email encryption, decryption and signatures."
+end
+
 $opts = Trollop::options do
   version "sup v#{Redwood::VERSION}"
   banner <<EOS
diff --git a/lib/sup/crypto.rb b/lib/sup/crypto.rb
index c7b57c1..9d21ea0 100644
--- a/lib/sup/crypto.rb
+++ b/lib/sup/crypto.rb
@@ -1,3 +1,8 @@
+begin
+  require 'gpgme'
+rescue LoadError
+end
+
 module Redwood
 
 class CryptoManager
@@ -11,76 +16,79 @@ class CryptoManager
     [:encrypt, "Encrypt only"]
   )
 
-  HookManager.register "gpg-args", <<EOS
-Runs before gpg is executed, allowing you to modify the arguments (most
+  HookManager.register "gpg-options", <<EOS
+Runs before gpg is called, allowing you to modify the options (most
 likely you would want to add something to certain commands, like
---trust-model always to signing/encrypting a message, but who knows).
+{:always_trust => true} to encrypting a message, but who knows).
 
 Variables:
-args: arguments for running GPG
+operation: what operation will be done ("sign", "encrypt", "decrypt" or "verify")
+options: a dictionary of values to be passed to GPGME
 
-Return value: the arguments for running GPG
+Return value: a dictionary to be passed to GPGME
 EOS
 
   def initialize
     @mutex = Mutex.new
 
-    bin = `which gpg`.chomp
-    @cmd = case bin
-    when /\S/
-      debug "crypto: detected gpg binary in #{bin}"
-      "#{bin} --quiet --batch --no-verbose --logger-fd 1 --use-agent"
-    else
-      debug "crypto: no gpg binary detected"
-      nil
+    # test if the gpgme gem is available
+    @gpgme_present = true
+    begin
+    GPGME.check_version({:protocol => GPGME::PROTOCOL_OpenPGP})
+    rescue NameError, GPGME::Error
+      @gpgme_present = false
     end
   end
 
-  def have_crypto?; !@cmd.nil? end
+  def have_crypto?; @gpgme_present end
 
   def sign from, to, payload
-    payload_fn = Tempfile.new "redwood.payload"
-    payload_fn.write format_payload(payload)
-    payload_fn.close
+    return unknown_status(cant_find_gpgme) unless @gpgme_present
 
-    sig_fn = Tempfile.new "redwood.signature"; sig_fn.close
+    gpg_opts = {:protocol => GPGME::PROTOCOL_OpenPGP, :armor => true, :textmode => true}
+    gpg_opts.merge(gen_sign_user_opts(from))
+    gpg_opts = HookManager.run("gpg-options", 
+                               {:operation => "sign", :options => gpg_opts}) || gpg_opts
 
-    sign_user_opts = gen_sign_user_opts from
-    message = run_gpg "--output #{sig_fn.path} --yes --armor --detach-sign --textmode --digest-algo sha256 #{sign_user_opts} #{payload_fn.path}", :interactive => true
-    unless $?.success?
-      info "Error while running gpg: #{message}"
+    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."
     end
 
     envelope = RMail::Message.new
-    envelope.header["Content-Type"] = 'multipart/signed; protocol=application/pgp-signature; micalg=pgp-sha256'
+    envelope.header["Content-Type"] = 'multipart/signed; protocol=application/pgp-signature'
 
     envelope.add_part payload
-    signature = RMail::Message.make_attachment IO.read(sig_fn.path), "application/pgp-signature", nil, "signature.asc"
+    signature = RMail::Message.make_attachment sig, "application/pgp-signature", nil, "signature.asc"
     envelope.add_part signature
     envelope
   end
 
   def encrypt from, to, payload, sign=false
-    payload_fn = Tempfile.new "redwood.payload"
-    payload_fn.write format_payload(payload)
-    payload_fn.close
-
-    encrypted_fn = Tempfile.new "redwood.encrypted"; encrypted_fn.close
-
-    recipient_opts = (to + [ from ] ).map { |r| "--recipient '<#{r}>'" }.join(" ")
-    sign_opts = ""
-    sign_opts = "--sign --digest-algo sha256 " + gen_sign_user_opts(from) if sign
-    message = run_gpg "--output #{encrypted_fn.path} --yes --armor --encrypt --textmode #{sign_opts} #{recipient_opts} #{payload_fn.path}", :interactive => true
-    unless $?.success?
-      info "Error while running gpg: #{message}"
+    return unknown_status(cant_find_gpgme) unless @gpgme_present
+
+    gpg_opts = {:protocol => GPGME::PROTOCOL_OpenPGP, :armor => true, :textmode => true}
+    if sign
+      gpg_opts.merge(gen_sign_user_opts(from)) 
+      gpg_opts.merge({:sign => true})
+    end
+    gpg_opts = HookManager.run("gpg-options", 
+                               {:operation => "encrypt", :options => gpg_opts}) || gpg_opts
+    recipients = to + [from]
+
+    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."
     end
 
     encrypted_payload = RMail::Message.new
     encrypted_payload.header["Content-Type"] = "application/octet-stream"
     encrypted_payload.header["Content-Disposition"] = 'inline; filename="msg.asc"'
-    encrypted_payload.body = IO.read(encrypted_fn.path)
+    encrypted_payload.body = cipher
 
     control = RMail::Message.new
     control.header["Content-Type"] = "application/pgp-encrypted"
@@ -99,70 +107,85 @@ EOS
     encrypt from, to, payload, true
   end
 
-  def verified_ok? output, rc
-    output_lines = output.split(/\n/)
-
-    if output =~ /^gpg: (.* signature from .*$)/
-      if rc == 0
-        Chunk::CryptoNotice.new :valid, $1, output_lines
-      else
-        Chunk::CryptoNotice.new :invalid, $1, output_lines
+  def verified_ok? verify_result
+    valid = true
+    unknown = false
+    output_lines = []
+
+    verify_result.signatures.each do |signature|
+      output_lines.push(sig_output_lines(signature))
+      output_lines.flatten!
+      err_code = GPGME::gpgme_err_code(signature.status)
+      if err_code == GPGME::GPG_ERR_BAD_SIGNATURE
+        valid = false 
+      elsif err_code != GPGME::GPG_ERR_NO_ERROR
+        valid = false
+        unknown = true
       end
-    elsif output_lines.length == 0 && rc == 0
-      # the message wasn't signed
+    end
+
+    if output_lines.length == 0
       Chunk::CryptoNotice.new :valid, "Encrypted message wasn't signed", output_lines
+    elsif valid
+      Chunk::CryptoNotice.new(:valid, simplify_sig_line(verify_result.signatures[0].to_s), output_lines)
+    elsif !unknown
+      Chunk::CryptoNotice.new(:invalid, simplify_sig_line(verify_result.signatures[0].to_s), output_lines)
     else
       unknown_status output_lines
     end
   end
 
   def verify payload, signature, detached=true # both RubyMail::Message objects
-    return unknown_status(cant_find_binary) unless @cmd
+    return unknown_status(cant_find_gpgme) unless @gpgme_present
 
+    gpg_opts = {:protocol => GPGME::PROTOCOL_OpenPGP}
+    gpg_opts = HookManager.run("gpg-options", 
+                               {:operation => "verify", :options => gpg_opts}) || gpg_opts
+    ctx = GPGME::Ctx.new(gpg_opts) 
+    sig_data = GPGME::Data.from_str signature.decode
     if detached
-      payload_fn = Tempfile.new "redwood.payload"
-      payload_fn.write format_payload(payload)
-      payload_fn.close
-    end
-
-    signature_fn = Tempfile.new "redwood.signature"
-    signature_fn.write signature.decode
-    signature_fn.close
-
-    if detached
-      output = run_gpg "--verify #{signature_fn.path} #{payload_fn.path}"
+      signed_text_data = GPGME::Data.from_str(format_payload(payload))
+      plain_data = nil
     else
-      output = run_gpg "--verify #{signature_fn.path}"
+      signed_text_data = nil
+      plain_data = GPGME::Data.empty
     end
-
-    self.verified_ok? output, $?
+    begin
+      ctx.verify(sig_data, signed_text_data, plain_data)
+    rescue GPGME::Error => exc
+      return unknown_status exc.message 
+    end
+    self.verified_ok? ctx.verify_result
   end
 
   ## returns decrypted_message, status, desc, lines
   def decrypt payload, armor=false # a RubyMail::Message object
-    return unknown_status(cant_find_binary) unless @cmd
-
-    payload_fn = Tempfile.new(["redwood.payload", ".asc"])
-    payload_fn.write payload.to_s
-    payload_fn.close
-
-    output_fn = Tempfile.new "redwood.output"
-    output_fn.close
-
-    message = run_gpg "--output #{output_fn.path} --skip-verify --yes --decrypt #{payload_fn.path}", :interactive => true
-
-    unless $?.success?
-      info "Error while running gpg: #{message}"
-      return Chunk::CryptoNotice.new(:invalid, "This message could not be decrypted", message.split("\n"))
+    return unknown_status(cant_find_gpgme) unless @gpgme_present
+
+    gpg_opts = {:protocol => GPGME::PROTOCOL_OpenPGP}
+    gpg_opts = HookManager.run("gpg-options", 
+                               {:operation => "decrypt", :options => gpg_opts}) || gpg_opts
+    ctx = GPGME::Ctx.new(gpg_opts) 
+    cipher_data = GPGME::Data.from_str(format_payload(payload))
+    plain_data = GPGME::Data.empty
+    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)
     end
-
-    output = IO.read output_fn.path
+    sig = self.verified_ok? ctx.verify_result
+    plain_data.seek(0, IO::SEEK_SET)
+    output = plain_data.read
     output.force_encoding Encoding::ASCII_8BIT if output.respond_to? :force_encoding
 
+    ## TODO: test to see if it is still necessary to do a 2nd run if verify
+    ## fails.
+    #
     ## check for a valid signature in an extra run because gpg aborts if the
     ## signature cannot be verified (but it is still able to decrypt)
-    sigoutput = run_gpg "#{payload_fn.path}"
-    sig = self.verified_ok? sigoutput, $?
+    #sigoutput = run_gpg "#{payload_fn.path}"
+    #sig = self.old_verified_ok? sigoutput, $?
 
     if armor
       msg = RMail::Message.new
@@ -207,8 +230,8 @@ private
     Chunk::CryptoNotice.new :unknown, "Unable to determine validity of cryptographic signature", lines
   end
 
-  def cant_find_binary
-    ["Can't find gpg binary in path."]
+  def cant_find_gpgme
+    ["Can't find gpgme gem."]
   end
 
   ## here's where we munge rmail output into the format that signed/encrypted
@@ -217,6 +240,28 @@ private
     payload.to_s.gsub(/(^|[^\r])\n/, "\\1\r\n")
   end
 
+  # remove the hex key_id and info in ()
+  def simplify_sig_line sig_line
+    sig_line = sig_line.sub(/from [0-9A-F]{16} /, "from ")
+    sig_line.sub(/\(.+\) </, "<")
+  end
+
+  def sig_output_lines signature
+    time_line = "Signature made " + signature.timestamp.strftime("%a %d %b %Y %H:%M:%S %Z") +
+                " using key ID " + signature.fingerprint[-8..-1]
+    first_sig = signature.to_s.sub(/from [0-9A-F]{16} /, 'from "') + '"'
+    output_lines = [time_line, first_sig]
+
+    ctx = GPGME::Ctx.new
+    if from_key = ctx.get_key(signature.fingerprint)
+      if from_key.uids.length > 1
+        aka_list = from_key.uids[1..-1]
+        aka_list.each { |aka| output_lines << '                aka "' + aka.uid + '"' }
+      end
+    end
+    output_lines
+  end
+
   # logic is:
   # if    gpgkey set for this account, then use that
   # elsif only one account,            then leave blank so gpg default will be user
@@ -224,30 +269,14 @@ private
   def gen_sign_user_opts from
     account = AccountManager.account_for from
     if !account.gpgkey.nil?
-      opts = "--local-user '#{account.gpgkey}'"
+      opts = {:signers => account.gpgkey}
     elsif AccountManager.user_emails.length == 1
       # only one account
-      opts = ""
+      opts = {}
     else
-      opts = "--local-user '#{from}'" 
+      opts = {:signers => from}
     end
     opts
   end
-
-  def run_gpg args, opts={}
-    args = HookManager.run("gpg-args", { :args => args }) || args
-    cmd = "LC_MESSAGES=C #{@cmd} #{args}"
-    if opts[:interactive] && BufferManager.instantiated?
-      output_fn = Tempfile.new "redwood.output"
-      output_fn.close
-      cmd += " > #{output_fn.path} 2> /dev/null"
-      debug "crypto: running: #{cmd}"
-      BufferManager.shell_out cmd
-      IO.read(output_fn.path) rescue "can't read output"
-    else
-      debug "crypto: running: #{cmd}"
-      `#{cmd} 2> /dev/null`
-    end
-  end
 end
 end
-- 
1.7.1

From 7b9a1eeeaaa25931963e2de49410d7cb0c7e6772 Mon Sep 17 00:00:00 2001
From: Hamish Downer <dmishd@gmail.com>
Date: Mon, 8 Nov 2010 22:31:01 +0000
Subject: [PATCH 2/2] catch exception when no public key present

---
 lib/sup/crypto.rb |   16 ++++++++++++----
 1 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/lib/sup/crypto.rb b/lib/sup/crypto.rb
index 9d21ea0..83176d9 100644
--- a/lib/sup/crypto.rb
+++ b/lib/sup/crypto.rb
@@ -247,19 +247,27 @@ private
   end
 
   def sig_output_lines signature
+    # It appears that the signature.to_s call can lead to a EOFError if
+    # the key is not found. So start by looking for the key.
+    ctx = GPGME::Ctx.new
+    begin
+      from_key = ctx.get_key(signature.fingerprint)
+      first_sig = signature.to_s.sub(/from [0-9A-F]{16} /, 'from "') + '"'
+    rescue EOFError => error
+      first_sig = "No public key available for #{signature.fingerprint}"
+    end
+
     time_line = "Signature made " + signature.timestamp.strftime("%a %d %b %Y %H:%M:%S %Z") +
                 " using key ID " + signature.fingerprint[-8..-1]
-    first_sig = signature.to_s.sub(/from [0-9A-F]{16} /, 'from "') + '"'
     output_lines = [time_line, first_sig]
 
-    ctx = GPGME::Ctx.new
-    if from_key = ctx.get_key(signature.fingerprint)
+    if from_key 
       if from_key.uids.length > 1
         aka_list = from_key.uids[1..-1]
         aka_list.each { |aka| output_lines << '                aka "' + aka.uid + '"' }
       end
     end
-    output_lines
+    output_lines.flatten!
   end
 
   # logic is:
-- 
1.7.1

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