Project

General

Profile

バグ #842

OpenSSL::Cipher#key= に長すぎるキーを指定しないようにする

Added by rhen ium over 3 years ago. Updated almost 3 years ago.

Status:
終了
Priority:
通常
Target version:
プラグイン名:
ブランチ:
クラッシュする:

Description

以前は OpenSSL::Cipher#key= に長すぎる文字列を渡した場合、勝手に(bf-ecb なら)16 バイトに切り詰められていたのですが、RDoc にも言及がなくどう考えても不自然な挙動なので、2 日前に r55146 (https://github.com/ruby/ruby/commit/ce635262f53b760284d56bb1027baebaaec175d1) で弾かれるように変更しました。
mikutter では SecureRandom.hex(= 32 バイト)を直接渡しているため、この影響を受けてしまっています。これは mikutter 側で修正するべきだと思うのですがどうでしょうか。

diff --git a/core/service_keeper.rb b/core/service_keeper.rb
index cdf18fa..fc4775e 100644
--- a/core/service_keeper.rb
+++ b/core/service_keeper.rb
@@ -15,7 +15,8 @@ class Service
     @@service_lock = Monitor.new

     def key
-      UserConfig[:account_crypt_key] ||= SecureRandom.hex end
+      key = UserConfig[:account_crypt_key] ||= SecureRandom.hex(8)
+      key[0, 16] end

     # 全てのアカウント情報をオブジェクトとして返す
     # ==== Return

再現手順

rubyのHEADでmikutterを起動する


Related issues

Related to 機能 #877: Ruby 2.4に対応するため、gtk2 3.0.9を利用する終了2016-09-03

Actions

Associated revisions

Revision 53db5173 (diff)
Added by toshi_a 初音 about 3 years ago

アカウント情報を暗号化するキーの長さが誤っている refs #842

Revision 44cf625a (diff)
Added by toshi_a 初音 almost 3 years ago

アカウント情報を暗号化するキーの長さが誤っている refs #842

History

#1

Updated by toshi_a 初音 over 3 years ago

  • Assignee set to toshi_a 初音
#2

Updated by toshi_a 初音 over 3 years ago

  • Assignee changed from toshi_a 初音 to rhen ium

丁度2倍になっているところから、SecureRandom.hexの戻り値が16進表記なのを失念していて、倍のサイズを渡したのだろうと推察できますね。

  • SecureRandom.hexだと16進数の文字列しか得られないため、キーのパターンが半分になってしまう
  • キーのサイズを指定すると、今回指摘されたようなミスが発生してしまうのではないか

と思って解決策を探っていたところ、 OpenSSL::Cipher#random_key を使ったほうが良いのではないかと思いました。釈迦に説法だと思うので詳しくは割愛しますが、どう思いますか?

#3

Updated by rhen ium over 3 years ago

  • Assignee changed from rhen ium to toshi_a 初音

ここでは UserConfig から読み込んだキーで復号したいので OpenSSL::Cipher#random_key は使えないと思います。

SecureRandom.random_bytes(16) で 16 バイトのランダムな文字列が得られますが、YAML ってバイナリデータ大丈夫なんでしたっけ…

bf-ecb のキー長は可変なので、適当な定数を置いて

KEY_LEN = 16

def key
  key = UserConfig[:account_crypt_key] ||= SecureRandom.random_bytes(KEY_LEN)
  key[0, KEY_LEN] end

def encrypt(str)
  cipher = OpenSSL::Cipher.new('bf-ecb').encrypt
  cipher.key_len = KEY_LEN
  cipher.key = key
  cipher.update(str) << cipher.final end

のようにするのが良さそうです。

#4

Updated by toshi_a 初音 about 3 years ago

  • Status changed from 新規 to レビュー待ち
  • Assignee changed from toshi_a 初音 to rhen ium
  • 再現手順 updated (diff)

薄い本…OSC…とだらだらしていたらえらく引っ張ってしまいました。

ブランチ topic/842-too-long-cipher-key に、貰ったアドバイスを参考にした変更を入れました。確認してみてください。
YAMLはバイナリデータ突っ込んでもBASE64か何かでエンコードしてよしなに扱うことができたような …と思って試したら、Rubyでも問題なかったので利用しました。まあそれも含めて結局提案してもらったとおりですね。

#5

Updated by toshi_a 初音 about 3 years ago

  • Related to 機能 #877: Ruby 2.4に対応するため、gtk2 3.0.9を利用する added
#6

Updated by rhen ium about 3 years ago

  • Assignee changed from rhen ium to toshi_a 初音

遅くなってごめんなさい。大丈夫だと思います。

#7

Updated by toshi_a 初音 almost 3 years ago

  • Status changed from レビュー待ち to 終了

3.3にmergeしました

Also available in: Atom PDF