Project

General

Profile

Actions

Bug #4445

closed

ext/openssl の verify_callback が rb_protect で保護されていない

Bug #4445: ext/openssl の verify_callback が rb_protect で保護されていない

Added by ohai (Ippei Obayashi) over 14 years ago. Updated over 14 years ago.

Status:
Closed
Target version:
ruby -v:
ruby 1.9.2p180 (2011-02-18 revision 30909) [x86_64-linux]
Backport:
[ruby-dev:43274]

Description

=begin
openssl では 証明書の検証に付加的な機能を付けるための
callback を設定できます。これをrubyから利用できるようになっていますが
rb_protect を使っていないため、openssl ライブラリ内部を飛び越えて
例外が飛ぶようになってしまう状態です。

現在ではSEGVが発生する等の問題は見つかってはいませんがメモリリークなど
起きている可能性が高いです。

とりあえず大域脱出を止めるパッチを添付します。例外を適当な場所で再送するべきかもしれませんが
それはしていません。
=end


Files

verify_cb.diff (662 Bytes) verify_cb.diff ohai (Ippei Obayashi), 02/25/2011 12:09 AM

Updated by naruse (Yui NARUSE) over 14 years ago Actions #1 [ruby-dev:43930]

  • Status changed from Open to Assigned
  • Assignee set to nahi (Hiroshi Nakamura)

Updated by nahi (Hiroshi Nakamura) over 14 years ago Actions #2 [ruby-dev:43935]

  • Target version set to 1.9.3

Updated by nahi (Hiroshi Nakamura) over 14 years ago Actions #3 [ruby-dev:43952]

  • Assignee changed from nahi (Hiroshi Nakamura) to MartinBosslet (Martin Bosslet)

Martin, can you handle this? Original reporter said that verify_cb does not use rb_protect to invoke a callback so an error raised from the callback passed directly to Ruby interpreter. Proposed patch looks good but we should check the rational of the current code (svn blame & svn log) and do some test around this.

Updated by MartinBosslet (Martin Bosslet) over 14 years ago Actions #4 [ruby-dev:43964]

Yes, I'll look into it!

Updated by nahi (Hiroshi Nakamura) over 14 years ago Actions #5 [ruby-dev:43984]

Note: #4611 and #4875 both crashes at ossl_ssl_verify_callback (1.9.2p274, 1.9.2p180.) I suspected this issue related to those issues.

Updated by MartinBosslet (Martin Bosslet) over 14 years ago Actions #6 [ruby-dev:43987]

Thanks for the input, I will keep them in mind when investigating this!

Updated by nahi (Hiroshi Nakamura) over 14 years ago Actions #7 [ruby-dev:44089]

Martin, how's the status? Can I take over this issue again? I think #4875 and #4611 relates to this issue.

Updated by MartinBosslet (Martin Bosslet) over 14 years ago Actions #8 [ruby-dev:44092]

  • Assignee changed from MartinBosslet (Martin Bosslet) to nahi (Hiroshi Nakamura)

Hiroshi Nakamura wrote:

Martin, how's the status? Can I take over this issue again? I think #4875 and #4611 relates to this issue.

Sure - if you feel it's related to the other two issues then you are clearly in a better position to design this properly. Should I look into #4923 and #4961 instead? Or are there any other urgencies where I could help?

Updated by nahi (Hiroshi Nakamura) over 14 years ago Actions #9 [ruby-dev:44096]

騒いでおいてすいません。 #4611 #4875 へのリンクを外しました。ちゃんとスタックを見たら、関係ありませんでした。

で、このチケットですが、Obayashiのおっしゃる通り、確かに今のコードはダメですね。他のcallback同様、事後に例外を上げられるといいんですが、verify callbackはOpenSSL側から何度も呼ばれるので、「事後」がうまく定義できません。

というわけで、1.9.4ではverify_callback内での例外は検証失敗扱いにしようと思います。(従来のようには例外は上がらない→SSLの場合は別途SSLErrorが上がる。X509Storeの場合はただの検証失敗になる)

ただ、1.9.3でどうするか悩んでいます。↑の変更を入れてしまいたいところですが、どこかで例外を投げているカスタマイズされたverify_callbackを見たことがあります。この時期に非互換を入れるのも。。。というわけで、挙動は従来どおりとして、rb_jump_tagの前にdeprecation警告、とかかなあ。。。

Updated by nahi (Hiroshi Nakamura) over 14 years ago Actions #10 [ruby-dev:44097]

Martin Bosslet wrote:

Sure - if you feel it's related to the other two issues then you are clearly in a better position to design this properly. Should I look into #4923 and #4961 instead? Or are there any other urgencies where I could help?

OK, I take this.

Do you think you can handle #4961? I don't think it's a release blocker since we just added tests which does not run with OpenSSL 0.9.7. It has not yet worked ever. But there could be a chance to find a easy way to fix the bug.

Updated by MartinBosslet (Martin Bosslet) over 14 years ago Actions #11 [ruby-dev:44098]

Hiroshi Nakamura wrote:

Do you think you can handle #4961? I don't think it's a release blocker since we just added tests which does not run with OpenSSL 0.9.7. It has not yet worked ever. But there could be a chance to find a easy way to fix the bug.

I tried OpenSSL.decode on the PEM data and it was valid. I'll try my best, probably debugging it directly in C will show us what fails there.

So I will concentrate on #4961, and if I can solve that, I will continue on #4923. If I can help you with anything, please let me know!

Updated by nahi (Hiroshi Nakamura) over 14 years ago Actions #12 [ruby-dev:44100]

  • Status changed from Assigned to Closed

r32537でtrunk、r32538でruby_1_9_3を修正しました。結果的には、Obayashiさん(前回呼び捨てですいません)のpatchをそのまま当て、かつ例外を投げている場合にはwarnで警告をする(しかし無視して、通常の検証失敗とする)ようにしました。テストを書いてみたらSSL接続が残ったままになり、GCで回収されるまでそのままという、いかにも危ない挙動になった(当たり前)ので、この時期ではありますが対処しました。

挙動変更は以下です: verify callbackで例外を投げた場合、従来はその例外が飛んでいましたが、代わりにwarnでやめろと警告され、SSLErrorが上がるようになります。

Obayashiさん、ご報告ありがとうございました。

Actions

Also available in: PDF Atom