From be90709b114b6f1cd84b70b58b989a3f46712da4 Mon Sep 17 00:00:00 2001 From: elijah Date: Tue, 6 Aug 2013 01:02:29 -0700 Subject: fix crash when fetched key is rejected (e.g. too short, etc), report errors in the request, prevent most crashes by catching exceptions. --- lib/nickserver/hkp/fetch_key.rb | 2 +- lib/nickserver/hkp/fetch_key_info.rb | 30 +++++++++++++++++++----------- lib/nickserver/server.rb | 2 ++ test/files/short_key_vindex_result | 7 +++++++ test/unit/hkp_test.rb | 13 ++++++++++++- 5 files changed, 41 insertions(+), 13 deletions(-) create mode 100644 test/files/short_key_vindex_result diff --git a/lib/nickserver/hkp/fetch_key.rb b/lib/nickserver/hkp/fetch_key.rb index 6f91d8c..c24b2c7 100644 --- a/lib/nickserver/hkp/fetch_key.rb +++ b/lib/nickserver/hkp/fetch_key.rb @@ -36,7 +36,7 @@ module Nickserver; module HKP end } http.errback { - self.fail 0, http.error + self.fail 500, http.error } end diff --git a/lib/nickserver/hkp/fetch_key_info.rb b/lib/nickserver/hkp/fetch_key_info.rb index b341aad..2cfff43 100644 --- a/lib/nickserver/hkp/fetch_key_info.rb +++ b/lib/nickserver/hkp/fetch_key_info.rb @@ -17,12 +17,17 @@ module Nickserver; module HKP params = {:op => 'vindex', :search => uid, :exact => 'on', :options => 'mr', :fingerprint => 'on'} EventMachine::HttpRequest.new(Config.hkp_url).get(:query => params).callback {|http| if http.response_header.status != 200 - self.fail http.response_header.status, "Could net fetch keyinfo" + self.fail http.response_header.status, "Could net fetch keyinfo." else - self.succeed parse(uid, http.response) + keys, errors = parse(uid, http.response) + if keys.empty? + self.fail 500, errors.join("\n") + else + self.succeed keys + end end }.errback {|http| - self.fail 0, http.error + self.fail 500, http.error } self end @@ -33,33 +38,36 @@ module Nickserver; module HKP # vindex_result -- raw output from a vindex hkp query (machine readable) # # returns: - # an array of eligible keys (as HKPKeyInfo objects) matching uid. + # an array of: + # [0] -- array of eligible keys (as HKPKeyInfo objects) matching uid. + # [1] -- array of error messages # # keys are eliminated from eligibility for a number of reasons, including expiration, # revocation, uid match, key length, and so on... # def parse(uid, vindex_result) keys = [] + errors = [] now = Time.now vindex_result.scan(MATCH_PUB_KEY).each do |match| key_info = KeyInfo.new(match[0]) if key_info.uids.include?(uid) - if key_info.keylen <= 1024 - #puts 'key length is too short' + if key_info.keylen < 2048 + errors << "Ignoring key #{key_info.keyid} for #{uid}: key length is too short." elsif key_info.expired? - #puts 'ignoring expired key' + errors << "Ignoring key #{key_info.keyid} for #{uid}: key expired." elsif key_info.revoked? - #puts 'ignoring revoked key' + errors << "Ignoring key #{key_info.keyid} for #{uid}: key revoked." elsif key_info.disabled? - #puts 'ignoring disabled key' + errors << "Ignoring key #{key_info.keyid} for #{uid}: key disabled." elsif key_info.expirationdate && key_info.expirationdate < now - #puts 'ignoring expired key' + errors << "Ignoring key #{key_info.keyid} for #{uid}: key expired" else keys << key_info end end end - keys + [keys, errors] end end diff --git a/lib/nickserver/server.rb b/lib/nickserver/server.rb index 8434759..7f74564 100644 --- a/lib/nickserver/server.rb +++ b/lib/nickserver/server.rb @@ -42,6 +42,8 @@ module Nickserver else send_key(uid) end + rescue RuntimeError => exc + send_error(exc.to_s) end private diff --git a/test/files/short_key_vindex_result b/test/files/short_key_vindex_result new file mode 100644 index 0000000..a410144 --- /dev/null +++ b/test/files/short_key_vindex_result @@ -0,0 +1,7 @@ +info:1:1 +pub:9A753A6B:17:1024:1252683790:: +uid:Tom%C3%A1s Touceda :1368645663:: +uid:Tomas Touceda :1361376077:: +uid:Tom%C3%A1s Touceda :1252683790:: +uid:Tom%C3%A1s Touceda :1270489372:: +uid:Tom%C3%A1s Touceda :1306498697:: \ No newline at end of file diff --git a/test/unit/hkp_test.rb b/test/unit/hkp_test.rb index 1b7b2c9..c211690 100644 --- a/test/unit/hkp_test.rb +++ b/test/unit/hkp_test.rb @@ -21,7 +21,7 @@ class HkpTest < MiniTest::Unit::TestCase def test_key_info_reject_keysize fetch_key_info :hkp_vindex_result, 'frog@leap.se' do |keys| - assert_equal 1, keys.length, 'should find one key' + assert_equal 1, keys.length, 'should find one key' # because short key gets ignored assert_equal '00440025', keys.first.keyid end end @@ -67,6 +67,17 @@ class HkpTest < MiniTest::Unit::TestCase end end + def test_fetch_key_too_short + uid = 'chiiph@leap.se' + key_id = '9A753A6B' + + stub_sks_vindex_reponse(uid, :body => file_content(:short_key_vindex_result)) + test_em_errback "Nickserver::HKP::FetchKey.new.get '#{uid}'" do |error| + assert_equal 500, error + end + end + + protected # -- cgit v1.2.3