The maze book for programmers!
mazesforprogrammers.com

Algorithms, circle mazes, hex grids, masking, weaving, braiding, 3D and 4D grids, spheres, and more!

DRM-Free Ebook

The Buckblog

assorted ramblings by Jamis Buck

Refactoring Net::SSH: Part 5

23 October 2004 — 4-minute read

After nearly a month of refactoring Net::SSH to take advantage of dependency injection (via the Needle framework) visible progress has been made: the transport layer works!

(A quick review: the transport layer of the SSH2 protocol deals with the low-level workings of SSH. It manages the algorithm negotiation, key exchange, encryption/compression of packets, and so forth. All other parts of the SSH protocol are built on top of the abstraction provided by the transport layer.)

How the Session is Initialized

As some of you may recall, the last article in this series ended on a question as I debated what approach to take regarding the initialization of the Session class. After all of that, Eivind Eklund suggested that I just pass the container to the Session constructor and let the Session initialize itself a la carte:

  container.register( :session, :model => :prototype ) do |c,p|
    Session.new( c.logs.get(p.fullname), c )
  end

At first, I was opposed to this approach, since it didn’t feel like dependency injection to me. The fact is, it isn’t. It’s the service locator pattern, which is closely related.

However, after some thought I realized that the price of being a purist in this case would mean more work for me. A good programmer should be able to use all of the tools at his disposal, understanding where they make sense and where they don’t. In this case, the service locator was a more efficient approach due to the large number of dependencies that Session had.

Further Refactoring, and Lessons Learned

The Session class itself was a beast (see the original sources). In order to tame it a bit and make it unit-testable, I moved the version and algorithm negotiation pieces into two different classes. This trimmed it down nicely, although it is still pretty big.

This most recent refactoring has finally hammered home a lesson that I feel ready to pen (though I’m certain others have already discovered and codified it elsewhere):

If you have a private method of a class that you feel needs unit testing, that method probably belongs in a class of its own.

In other words, suppose you have a private method foo that is called by a public method bar:

  class Agnathostomata
    def bar
      ...
      foo
      ...
    end

    def foo
      ...
    end
    private :foo
  end

When it comes time to test the class, you discover that you would like to unit test the foo method directly. Your options in this case are to (a) make foo public, or (b) circumvent the access control via send or instance_eval. Neither are very attractive options.

What I discovered is that it is probably better, in many cases, to move foo (and any related functionality) into a class of its own, making foo public in the process. Then, the first class delegates that functionality to the new class, like this:

  class Spiloma
    def foo
      ...
    end
  end

  class Agnathostomata
    def initialize
      @delegate = Spiloma.new
    end

    def bar
      ...
      @delegate.foo
      ...
    end
  end

This is exactly how the new VersionNegotiator and AlgorithmNegotiator classes came to be.

Results

I had been unit testing every piece this entire time, so I felt confident that there wouldn’t be any significant bugs. However, the pieces I hadn’t tested so far were the parts that dealt with the dependency injection. Naturally, when it came time to run the integration test, those were the parts that failed first. :)

The problems weren’t significant, however. Usually only a few typos or scoping issues. There was one place where I had indicated the maximum number of bits in the key should be 8196, when it should have been 8192, and that caused me a bit of grief, but I found and fixed it.

The integration test was simple: for every combination of cryptography backend, host key, kex algorithm, cipher algorithm, HMAC algorithm, and compression algorithm, run a test that opened a new connection via the transport Session, sent a message and received a response, and then closed the connection.

Voila!

  backends.each do |backend|
    keys.each do |key|
      kexs.each do |kex|
        encryptions.each do |encryption|
          hmacs.each do |hmac|
            compressions.each do |compression|
              method_name = "test_#{backend}__#{key}__#{kex}__#{encryption}__#{hmac}__#{compression}" 
              method_name.gsub!( /-/, "_" )

              define_method( method_name ) do
                @registry.register( :crypto_backend ) { backend }
                session = @registry.transport.session

                assert_nothing_raised do
                  session.open "localhost",
                    :host_key => key,
                    :kex => kex,
                    :encryption => encryption,
                    :hmac => hmac,
                    :compression => compression
                end

                assert_equal key, session.algorithms.host_key
                assert_equal kex, session.algorithms.kex
                assert_equal encryption, session.algorithms.encryption_c2s
                assert_equal encryption, session.algorithms.encryption_s2c
                assert_equal hmac, session.algorithms.mac_c2s
                assert_equal hmac, session.algorithms.mac_s2c
                assert_equal compression, session.algorithms.compression_c2s
                assert_equal compression, session.algorithms.compression_s2c

                type = nil
                assert_nothing_raised do
                  session.send_message "#{session.class::SERVICE_REQUEST.chr}\0\0\0\14ssh-userauth" 
                  type, buffer = session.wait_for_message
                end

                assert_equal session.class::SERVICE_ACCEPT, type 
                session.close
              end

            end
          end
        end
      end
    end
  end

The result?

  $ ruby test_integration.rb
  Loaded suite test_integration
  Started
  .........................................................................
  .........................................................................
  ..............
  Finished in 21.566316 seconds.

  160 tests, 1760 assertions, 0 failures, 0 errors

It’s alive!

Future Directions

Well, as I said earlier, the transport layer is just one piece (albeit a very fundamental one) of the SSH protocol stack. So, the parts that remain to be done are:

  • User authentication
  • Connection management (channels, etc.)
  • SFTP protocol
  • Various convenience interfaces

I figure I might be about half done with the entire refactoring. Regardless, once I’m done I’ll release Net::SSH 0.2.0, and will finally be able to proceed with all the parts that are still missing from it.