torsdag, augusti 14, 2008

Where is the Net::SSH bug

Yesterday I spent several hours trying to find the problem with our implementation of OpenSSL Cipher, that caused the Net::SSH gem to fail miserable during negotiation and password verification. After various false leads I finally found the reason for the strange behavior. But I really can't decide if it's a bug, and if it's a bug where the bug is. Is it in Ruby's interface to OpenSSL, or is it in Net::SSH?

No matter what cipher suite you use for SSH, you generally end up using a block cipher, mostly something like CBC. That means an IV (initialization vector) is needed, together with a key. The relevant parts of OpenSSL used is the EVP_CipherInit, EVP_CipherUpdate and EVP_CipherFinal family of methods. Nothing really strange there. The Ruby interface matches these methods quite closely; every time you set a key, or an IV, or some other parameter, the CipherInit method is called with the relevant data. When CipherUpdate is called, the actual enciphering or deciphering starts happening, and CipherFinal takes care of the final block.

At the point EVP_CipherFinal is called, nothing more should be done using the specific Cipher context. Specifically, no more Update operations should be used. The man page has this to say about the Final-methods:
After this function is called the encryption operation is finished and no further calls to EVP_EncryptUpdate() should be made.

Now, what I found was that same documentation is not part of the Ruby interface. And Net::SSH is actually reusing the same Cipher object after final has been called on it. Specifically, it continues the conversation, calling update a few times and then final. The general flow for a specific Cipher object in Net::SSH is basically init->update->update->final->update->update->final.

So what is so bad about this then? Well, the question is really this: what IV will the operations after the first final call be using? The assumption I made is that obviously it will use the original IV set on the object. Something else would seem absurd. But indeed, the IV used is actually the last IV-length bytes of encrypted data returned. Is this an obvious or intended effect at some level? Probably not, since the OpenSSL documentation says you shouldn't do it. The reason it works that way is because the temporary buffer used in the Cipher context isn't cleared out at the end of the call to final.

In contrast, the Java Cipher object will call reset() as part of the call to doFinal(). Where reset() will actually reset the internal buffers to use the original IV. So the solution is simple for encryption. Just save away 8 or 16 bytes of the last generated crypto text and set that manually as the IV after the call to doFinal. And what about decryption? Well, here the IV needs to be the last crypto text sent in for deciphering, not the result of the last operation.

So Net::SSH seems to work fine with JRuby now. I'm about to release a new version of JRuby-OpenSSL including these and many other things.

But the question remains. Is it a bug? If it is, is it in the Ruby OpenSSL integration, or in the Net::SSH usages of Ciphers? If it's in the Net::SSH code, why does it actually work correctly when communicating with an SSH server? Or is this behavior of using the last crypto text as IV something documented in the SSH spec?

Enlightenment would be welcome.

4 kommentarer:

Anonym sa...

Only slightly related: do you support SSH agents in the JRuby SSH implementation? If yes, which underlying library do you use for that?

Anonym sa...

The SSH spec calls for treating packets received as a stream. in CBC mode, this means the last n bytes of a packet will be used as the IV for the next packet to be decrypted. See RFC 4253 6.3 for reference.

With that said, Net::SSH is using OpenSSL incorrectly. It should never use a cipher after calling final. It should either: a) create a new cipher and rekey it, or b) only call final once the connection is closed.

Anonym sa...

The bug is in the bindings. They should "freeze" the object, so it throws an exception when you try to reuse it, or fix it in some other, more transparent way.

If the bindings would've thrown an exception, Net::SSH wouldn't even be able to use it wrongly, since it would blow up right away. Repair what you can — but when you must fail, fail noisily and as soon as possible.

Anonym sa...

Fantastic investigating. I'll look at fixing Net::SSH to not reuse the ciphers.