Bug #5547: Cleanup engine after a test

• Status: Closed
• Assignee: MartinBosslet (Martin Bosslet)
• Target version: -
• ruby -v: ruby 2.0.0dev (2011-11-01 trunk 33605) [i386-netbsdelf5.99.56]

[naruse (Yui NARUSE)]

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

[MartinBosslet (Martin Bosslet)]

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!

• 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]

[MartinBosslet (Martin Bosslet)]

• Status changed from Closed to Assigned

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

[MartinBosslet (Martin Bosslet)]

• 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?

[naruse (Yui NARUSE)]

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.

[MartinBosslet (Martin Bosslet)]

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?

[naruse (Yui NARUSE)]

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.

[MartinBosslet (Martin Bosslet)]

• 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?

[naruse (Yui NARUSE)]

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.

[MartinBosslet (Martin Bosslet)]

Yui NARUSE wrote:

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

Great, thanks!