Project

General

Profile

Actions

Bug #5547

closed

Cleanup engine after a test

Added by naruse (Yui NARUSE) over 12 years ago. Updated over 12 years ago.

Status:
Closed
Target version:
-
ruby -v:
ruby 2.0.0dev (2011-11-01 trunk 33605) [i386-netbsdelf5.99.56]
Backport:
[ruby-core:40669]

Description

OpenSSL::Engine.load() loads engines and register them, and it may change the behavior of some existing methods.

For example on NetBSD 6 with cryptodev, it effects DH as folloing:
./ruby -ropenssl -e'p OpenSSL::PKey::DH.new(256).public_key.private?;p OpenSSL::Engine.load;p OpenSSL::PKey::DH.new(256).public_key.private?'
false
true
true

After loads cryptodev and register it (yes, it needs register. current ext/openssl can't register a engine),
OpenSSL::PKey::DH#private?'s behavior seems to be changed.

Whether it is a bug or not, test/openssl/test_engine.rb should be fixed.

Index: test/openssl/test_engine.rb

--- test/openssl/test_engine.rb (revision 33605)
+++ test/openssl/test_engine.rb (working copy)
@@ -8,6 +8,7 @@
OpenSSL::Engine.load
OpenSSL::Engine.engines
OpenSSL::Engine.engines

  • OpenSSL::Engine.cleanup
    end

end

Updated by MartinBosslet (Martin Bosslet) over 12 years ago

Yui NARUSE wrote:

OpenSSL::Engine.load() loads engines and register them, and it may change the behavior of some existing methods.

For example on NetBSD 6 with cryptodev, it effects DH as folloing:
./ruby -ropenssl -e'p OpenSSL::PKey::DH.new(256).public_key.private?;p OpenSSL::Engine.load;p OpenSSL::PKey::DH.new(256).public_key.private?'
false
true
true

I'm not sure whether we can ensure consistent behaviour for this, I assume it depends largely on the underlying native library.
In addition to cryptodev, I might be able to use my PKCS#11 eToken with the PKCS#11 engine to see how that would behave in
contrast.

After loads cryptodev and register it (yes, it needs register. current ext/openssl can't register a engine),
OpenSSL::PKey::DH#private?'s behavior seems to be changed.

Whether it is a bug or not, test/openssl/test_engine.rb should be fixed.

Index: test/openssl/test_engine.rb

--- test/openssl/test_engine.rb (revision 33605)
+++ test/openssl/test_engine.rb (working copy)
@@ -8,6 +8,7 @@
OpenSSL::Engine.load
OpenSSL::Engine.engines
OpenSSL::Engine.engines

  • OpenSSL::Engine.cleanup
    end

end

Yes, I agree, thanks for the patch, Yui!

Actions #2

Updated by Anonymous over 12 years ago

  • Status changed from Assigned to Closed
  • % Done changed from 0 to 100

This issue was solved with changeset r33614.
Yui, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.


  • test/openssl/test_engine.rb: call Engine::cleanup on exit.
    Patch provided by Yui Naruse, thanks!
    [Bug #5547] [ruby-core:40669]

Updated by MartinBosslet (Martin Bosslet) over 12 years ago

  • Status changed from Closed to Assigned

I'll keep it open, I still want to find out why the behaviour changes!

Updated by MartinBosslet (Martin Bosslet) over 12 years ago

  • Status changed from Assigned to Third Party's Issue
  • Assignee changed from MartinBosslet (Martin Bosslet) to naruse (Yui NARUSE)

It's OpenSSL itself that causes the weird behaviour.

On my machine with no cryptodev installed, I get the expected
output, so I would assume it's either cryptodev itself or at
least the cryptodev ENGINE implementation of OpenSSL that
causes this. Investigate further or is it safe to close this?

Updated by naruse (Yui NARUSE) over 12 years ago

Yes, the phenomenon, the result of above code will change after OpenSSL::Engine.load on NetBSD current,
itself should be cryptodev's problem.
But the test is against a principle that a test shouldn't have side effects.
It has clearly a side effect: an engine is loaded.

Actions #6

Updated by MartinBosslet (Martin Bosslet) over 12 years ago

Yui NARUSE wrote:

Yes, the phenomenon, the result of above code will change after OpenSSL::Engine.load on NetBSD current,
itself should be cryptodev's problem.
But the test is against a principle that a test shouldn't have side effects.
It has clearly a side effect: an engine is loaded.

I agree, and the side effect won't give us reproducible results anyway as your
example with cryptodev has clearly shown. The initial bug that was fixed by
test_engines_free was regardless of the underlying engine being used.

So I decided to make all tests use the "openssl" engine. It's software-based and
it should behave the same for any of us. If we get differing results from
anyone, we can at least conclude that it must be a bug in either OpenSSL itself
or in ext/openssl.

This removes side effects from differing "default engines" or at least it should
contain them in a controllable manner. Would you agree?

Updated by naruse (Yui NARUSE) over 12 years ago

Martin Bosslet wrote:

Yui NARUSE wrote:

Yes, the phenomenon, the result of above code will change after OpenSSL::Engine.load on NetBSD current,
itself should be cryptodev's problem.
But the test is against a principle that a test shouldn't have side effects.
It has clearly a side effect: an engine is loaded.

I agree, and the side effect won't give us reproducible results anyway as your
example with cryptodev has clearly shown. The initial bug that was fixed by
test_engines_free was regardless of the underlying engine being used.

So I decided to make all tests use the "openssl" engine. It's software-based and
it should behave the same for any of us. If we get differing results from
anyone, we can at least conclude that it must be a bug in either OpenSSL itself
or in ext/openssl.

This removes side effects from differing "default engines" or at least it should
contain them in a controllable manner. Would you agree?

Yeah, I agree.
Note that it should do only if possible with reasonable effort.

Updated by MartinBosslet (Martin Bosslet) over 12 years ago

  • Status changed from Third Party's Issue to Closed
  • Assignee changed from naruse (Yui NARUSE) to MartinBosslet (Martin Bosslet)

Yui NARUSE wrote:

Martin Bosslet wrote:

Yui NARUSE wrote:
...
This removes side effects from differing "default engines" or at least it should
contain them in a controllable manner. Would you agree?

Yeah, I agree.
Note that it should do only if possible with reasonable effort.

Thanks, and thanks for your comments. The effort to switch to "openssl engine"-only
was minimal. OK if I close this?

Updated by naruse (Yui NARUSE) over 12 years ago

Martin Bosslet wrote:

Yui NARUSE wrote:

Martin Bosslet wrote:

Yui NARUSE wrote:
...
This removes side effects from differing "default engines" or at least it should
contain them in a controllable manner. Would you agree?

Yeah, I agree.
Note that it should do only if possible with reasonable effort.

Thanks, and thanks for your comments. The effort to switch to "openssl engine"-only
was minimal. OK if I close this?

OK.
If another issue happens, I make another ticket.

Updated by MartinBosslet (Martin Bosslet) over 12 years ago

Yui NARUSE wrote:

OK.
If another issue happens, I make another ticket.

Great, thanks!

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0