[Twisted-Python] getPeer() inside conch
![](https://secure.gravatar.com/avatar/1006ff3d30f83b384e0f0a2b7e1939ca.jpg?s=120&d=mm&r=g)
Hello. I'm moderately experienced with twisted, and am starting out writing an ssh server using conch. I'm writing a protocol which inherits from the recvline.HistoricRecvLine class. It has connectionMade() and lineReceived() methods so it feels a lot like the twisted.protocols.basic.LineReceiver class I'm used to using. But, it seems the recvline.HistoricRecvLine class wants me to use self.terminal rather than self.transport to talk to the client. That's fine, but it seems that self.terminal lacks a getPeer() method. How do I find out the IP address of the client inside a conch ssh server? (For starters, I wish to log all successful client connections so I know who's connecting). I searched and found the below issue. I assume it is related, but could someone confirm? http://twistedmatrix.com/trac/ticket/2453 Thanks. -- Benjamin Rutt
![](https://secure.gravatar.com/avatar/e1554622707bedd9202884900430b838.jpg?s=120&d=mm&r=g)
On Mon, Sep 28, 2009 at 8:10 PM, Benjamin Rutt <rutt.4@osu.edu> wrote:
So, after reading through the code a bit, self.terminal should be an ITerminalTransport. There are only a few ITerminalTransport implementations, several of which are also Protocol subclasses and therefore have a 'transport' attribute. I'm not 100% sure what the eventual composiiton of your application will be, but you could try self.terminal.transport.getPeer(); it might work. Technically speaking, ITerminalTransport is a subclass of ITransport, which has a getPeer method. Therefore any classes which implement ITerminalTransport without providing getPeer are buggy. It would be really helpful if you could provide a patch that addressed the issue you mention :).
![](https://secure.gravatar.com/avatar/1006ff3d30f83b384e0f0a2b7e1939ca.jpg?s=120&d=mm&r=g)
The code: print 'hello, peer %s' % (self.terminal.transport.getPeer()) Fails with: AttributeError: SSHSessionProcessProtocol instance has no attribute 'getPeer' One possible patch to twisted 8.2.0's conch/ssh/session.py's SSHSessionProcessProtocol class to add a getPeer() method would be: $ diff -u session.py.orig session.py --- session.py.orig 2009-06-10 06:30:33.000000000 -0400 +++ session.py 2009-09-29 12:30:01.622444000 -0400 @@ -13,7 +13,7 @@ import struct -from twisted.internet import protocol +from twisted.internet import protocol, address from twisted.python import log from twisted.conch.interfaces import ISession from twisted.conch.ssh import common, channel @@ -210,6 +210,10 @@ def loseConnection(self): self.session.loseConnection() + def getPeer(self): + client = self.session.conn.transport.transport.client + return address.IPv4Address('TCP', client[0], client[1]) + class SSHSessionClient(protocol.Protocol): def dataReceived(self, data): Could someone comment on this? (I'm fairly certain ssh runs over TCP, but less certain that the mouthful self.session.conn.transport.transport.client isn't invading layers of abstraction in an unfriendly way). Thanks.
![](https://secure.gravatar.com/avatar/e1554622707bedd9202884900430b838.jpg?s=120&d=mm&r=g)
On Tue, Sep 29, 2009 at 7:16 PM, Benjamin Rutt <rutt.4@osu.edu> wrote:
Oops. Looks like my reading of the code was wrong. One possible patch to twisted 8.2.0's conch/ssh/session.py's
SSHSessionProcessProtocol class to add a getPeer() method would be:
So, nominally, all those things are public (none of them starts with an underscore) but I'd have to sift through the code to understand exactly how kosher they are (whether those attributes are a part of the interfaces in question). However, (A) 'TCP' isn't quite the right answer here, because actually there's some SSH in the way, and (B) I am pretty sure there is a getPeer closer than that. In particular, SSHSessionProcessProtocol.session is a twisted.conch.session.SSHSession SSHSession subclasses twisted.conch.ssh.channel.SSHChannel SSHChannel implements getPeer already. So you *should* be able to do self.session.getPeer() and get something useful. I hope. That said, the getPeer implementation in question should really be returning an IAddress, not a tuple, so you can subclass or delegate to IPv4Address and still access 'host' on the return value. Independent of the grossness, if you can contribute some test coverage for this, we could make the implementation less gross later :). This is especially important since hopefully many of these conch APIs will change in the near future. (z3p, are you planning on coming to the sprint this week...?)
![](https://secure.gravatar.com/avatar/1006ff3d30f83b384e0f0a2b7e1939ca.jpg?s=120&d=mm&r=g)
(A/B) it looks like the SSHChannel implementation of getPeer() is faulty. If I change my two-line implementation of getPeer() inside SSHSessionProcessProtocol to simply 'return self.session.getPeer()', I get: File "/home/ruttbe/dev/python/twisted-8.2.0-inst/lib/python/twisted/conch/ssh/session.py", line 216, in getPeer return self.session.getPeer() File "/home/ruttbe/dev/python/twisted-8.2.0-inst/lib/python/twisted/conch/ssh/channel.py", line 261, in getPeer return('SSH', )+self.conn.transport.getPeer() AttributeError: SSHServerTransport instance has no attribute 'getPeer' (C) regarding adding a test, which file do you think would be suitable here? something in the twisted/test dir?
![](https://secure.gravatar.com/avatar/e1554622707bedd9202884900430b838.jpg?s=120&d=mm&r=g)
On Thu, Oct 1, 2009 at 10:21 PM, Benjamin Rutt <rutt.4@osu.edu> wrote:
OK, I give up :). Looks like that is pretty clearly a bug, though.
(C) regarding adding a test, which file do you think would be suitable here? something in the twisted/test dir?
I am not sure which exact file, but something of the form twisted/conch/test/test_<something>.py Paul Swartz (z3p) should really weigh in on this. You can find him on IRC most days, although hopefully he's reading the mailing list :).
![](https://secure.gravatar.com/avatar/7cb174aac3d9e3cbc0ebb04868f710d6.jpg?s=120&d=mm&r=g)
On Thu, Oct 1, 2009 at 10:53 PM, Glyph Lefkowitz <glyph@twistedmatrix.com> wrote:
You're correct; it's bug http://twistedmatrix.com/trac/ticket/2997 There's also http://twistedmatrix.com/trac/ticket/2453 for adding getHost()/getPeer() to SSHSessionProcessProtocol. A patch should probably be against branches/conch-old-session-tests-4047, which already includes tests for other things in twisted/conch/ssh/session.py -p -- Paul Swartz paulswartz at gmail dot com http://paulswartz.net/ AIM: z3penguin
![](https://secure.gravatar.com/avatar/e1554622707bedd9202884900430b838.jpg?s=120&d=mm&r=g)
On Mon, Sep 28, 2009 at 8:10 PM, Benjamin Rutt <rutt.4@osu.edu> wrote:
So, after reading through the code a bit, self.terminal should be an ITerminalTransport. There are only a few ITerminalTransport implementations, several of which are also Protocol subclasses and therefore have a 'transport' attribute. I'm not 100% sure what the eventual composiiton of your application will be, but you could try self.terminal.transport.getPeer(); it might work. Technically speaking, ITerminalTransport is a subclass of ITransport, which has a getPeer method. Therefore any classes which implement ITerminalTransport without providing getPeer are buggy. It would be really helpful if you could provide a patch that addressed the issue you mention :).
![](https://secure.gravatar.com/avatar/1006ff3d30f83b384e0f0a2b7e1939ca.jpg?s=120&d=mm&r=g)
The code: print 'hello, peer %s' % (self.terminal.transport.getPeer()) Fails with: AttributeError: SSHSessionProcessProtocol instance has no attribute 'getPeer' One possible patch to twisted 8.2.0's conch/ssh/session.py's SSHSessionProcessProtocol class to add a getPeer() method would be: $ diff -u session.py.orig session.py --- session.py.orig 2009-06-10 06:30:33.000000000 -0400 +++ session.py 2009-09-29 12:30:01.622444000 -0400 @@ -13,7 +13,7 @@ import struct -from twisted.internet import protocol +from twisted.internet import protocol, address from twisted.python import log from twisted.conch.interfaces import ISession from twisted.conch.ssh import common, channel @@ -210,6 +210,10 @@ def loseConnection(self): self.session.loseConnection() + def getPeer(self): + client = self.session.conn.transport.transport.client + return address.IPv4Address('TCP', client[0], client[1]) + class SSHSessionClient(protocol.Protocol): def dataReceived(self, data): Could someone comment on this? (I'm fairly certain ssh runs over TCP, but less certain that the mouthful self.session.conn.transport.transport.client isn't invading layers of abstraction in an unfriendly way). Thanks.
![](https://secure.gravatar.com/avatar/e1554622707bedd9202884900430b838.jpg?s=120&d=mm&r=g)
On Tue, Sep 29, 2009 at 7:16 PM, Benjamin Rutt <rutt.4@osu.edu> wrote:
Oops. Looks like my reading of the code was wrong. One possible patch to twisted 8.2.0's conch/ssh/session.py's
SSHSessionProcessProtocol class to add a getPeer() method would be:
So, nominally, all those things are public (none of them starts with an underscore) but I'd have to sift through the code to understand exactly how kosher they are (whether those attributes are a part of the interfaces in question). However, (A) 'TCP' isn't quite the right answer here, because actually there's some SSH in the way, and (B) I am pretty sure there is a getPeer closer than that. In particular, SSHSessionProcessProtocol.session is a twisted.conch.session.SSHSession SSHSession subclasses twisted.conch.ssh.channel.SSHChannel SSHChannel implements getPeer already. So you *should* be able to do self.session.getPeer() and get something useful. I hope. That said, the getPeer implementation in question should really be returning an IAddress, not a tuple, so you can subclass or delegate to IPv4Address and still access 'host' on the return value. Independent of the grossness, if you can contribute some test coverage for this, we could make the implementation less gross later :). This is especially important since hopefully many of these conch APIs will change in the near future. (z3p, are you planning on coming to the sprint this week...?)
![](https://secure.gravatar.com/avatar/1006ff3d30f83b384e0f0a2b7e1939ca.jpg?s=120&d=mm&r=g)
(A/B) it looks like the SSHChannel implementation of getPeer() is faulty. If I change my two-line implementation of getPeer() inside SSHSessionProcessProtocol to simply 'return self.session.getPeer()', I get: File "/home/ruttbe/dev/python/twisted-8.2.0-inst/lib/python/twisted/conch/ssh/session.py", line 216, in getPeer return self.session.getPeer() File "/home/ruttbe/dev/python/twisted-8.2.0-inst/lib/python/twisted/conch/ssh/channel.py", line 261, in getPeer return('SSH', )+self.conn.transport.getPeer() AttributeError: SSHServerTransport instance has no attribute 'getPeer' (C) regarding adding a test, which file do you think would be suitable here? something in the twisted/test dir?
![](https://secure.gravatar.com/avatar/e1554622707bedd9202884900430b838.jpg?s=120&d=mm&r=g)
On Thu, Oct 1, 2009 at 10:21 PM, Benjamin Rutt <rutt.4@osu.edu> wrote:
OK, I give up :). Looks like that is pretty clearly a bug, though.
(C) regarding adding a test, which file do you think would be suitable here? something in the twisted/test dir?
I am not sure which exact file, but something of the form twisted/conch/test/test_<something>.py Paul Swartz (z3p) should really weigh in on this. You can find him on IRC most days, although hopefully he's reading the mailing list :).
![](https://secure.gravatar.com/avatar/7cb174aac3d9e3cbc0ebb04868f710d6.jpg?s=120&d=mm&r=g)
On Thu, Oct 1, 2009 at 10:53 PM, Glyph Lefkowitz <glyph@twistedmatrix.com> wrote:
You're correct; it's bug http://twistedmatrix.com/trac/ticket/2997 There's also http://twistedmatrix.com/trac/ticket/2453 for adding getHost()/getPeer() to SSHSessionProcessProtocol. A patch should probably be against branches/conch-old-session-tests-4047, which already includes tests for other things in twisted/conch/ssh/session.py -p -- Paul Swartz paulswartz at gmail dot com http://paulswartz.net/ AIM: z3penguin
participants (3)
-
Benjamin Rutt
-
Glyph Lefkowitz
-
Paul Swartz