No SSL support for IOCP reactor

[justinj]

I get the following errors when trying to start an HTTP service over SSL using
the IOCP reactor.  I'm running Windows 2000 and the latest Twisted from subversion.


C:\svn\RatControl>twistd -r iocp -f ratcontrol.tap
C:\svn\RatControl\nevow\rend.py:41: DeprecationWarning: renderer.Renderer has be
en deprecated. Use rend.Page instead.
  NotFound = FourOhFour(), ()
2004/04/09 14:33 Central Daylight Time [-] Log opened.
2004/04/09 14:33 Central Daylight Time [-] twistd 1.2.1alpha2 (C:\Python23\pytho
n.exe 2.3.3) starting up
2004/04/09 14:33 Central Daylight Time [-] reactor class: <class 'twisted.intern
et.iocpreactor.proactor.Proactor'>
2004/04/09 14:33 Central Daylight Time [-] Traceback (most recent call last):
2004/04/09 14:33 Central Daylight Time [-]   File "C:\Python23\scripts\twistd.py
", line 36, in ?
2004/04/09 14:33 Central Daylight Time [-]     run()
2004/04/09 14:33 Central Daylight Time [-]   File "C:\Python23\lib\site-packages
\twisted\scripts\twistw.py", line 61, in run
2004/04/09 14:33 Central Daylight Time [-]     app.run(runApp, ServerOptions)
2004/04/09 14:33 Central Daylight Time [-]   File "C:\Python23\lib\site-packages
\twisted\application\app.py", line 205, in run
2004/04/09 14:33 Central Daylight Time [-]     runApp(config)
2004/04/09 14:33 Central Daylight Time [-]   File "C:\Python23\lib\site-packages
\twisted\scripts\twistw.py", line 51, in runApp
2004/04/09 14:33 Central Daylight Time [-]     service.IService(application).pri
vilegedStartService()
2004/04/09 14:33 Central Daylight Time [-]   File "C:\Python23\lib\site-packages
\twisted\application\service.py", line 205, in privilegedStartService
2004/04/09 14:33 Central Daylight Time [-]     service.privilegedStartService()
2004/04/09 14:33 Central Daylight Time [-]   File "C:\Python23\lib\site-packages
\twisted\application\service.py", line 205, in privilegedStartService
2004/04/09 14:33 Central Daylight Time [-]     service.privilegedStartService()
2004/04/09 14:33 Central Daylight Time [-]   File "C:\Python23\lib\site-packages
\twisted\application\internet.py", line 57, in privilegedStartService
2004/04/09 14:33 Central Daylight Time [-]     self._port = self._getPort()
2004/04/09 14:33 Central Daylight Time [-]   File "C:\Python23\lib\site-packages
\twisted\application\internet.py", line 74, in _getPort
2004/04/09 14:33 Central Daylight Time [-]     return getattr(reactor, 'listen'+
self.method)(*self.args, **self.kwargs)
2004/04/09 14:33 Central Daylight Time [-] AttributeError: 'Proactor' object has
 no attribute 'listenSSL'

[PenguinOfDoom]

Not possible until Twisted has a SSL-as-Protocol implementation. exarkun was
working on one. I might look into it, but not in any forseeable time.

[justinj]

I'm reassigning this to me.  Let me know if you have a problem with that.

[zooko]

I would be happy if this bug were fixed.

[zooko]

Waitaminute, is this the same bug as mine? Mine looks like this:

14:04:30 DEBUG 'twisted' [-] Starting factory <foolscap.negotiate.TubConnectorClientFactory object [from jcakyusx] [to joxqfyja] at 0x01B84650>
14:04:31 DEBUG 'twisted' [twisted.internet.iocpreactor.proactor.Proactor] negotiation had internal error:
14:04:31 DEBUG 'twisted' [twisted.internet.iocpreactor.proactor.Proactor] [Failure instance: Traceback: exceptions.AttributeError: 'ClientSocket' object has no attribute 'startTLS'
--tw--c:\Python24\Lib\site-packages\twisted\python\context.py:37:callWithContext
--tw--c:\Python24\Lib\site-packages\twisted\internet\iocpreactor\ops.py:41:ovDone
--tw--c:\Python24\Lib\site-packages\twisted\internet\iocpreactor\util.py:17:helpful
--tw--c:\Python24\Lib\site-packages\twisted\internet\iocpreactor\abstract.py:159:handle_connected_readDone
--tw----- <exception caught here> ---
--tw--c:\Python24\Lib\site-packages\foolscap\negotiate.py:380:dataReceived
--tw--c:\Python24\Lib\site-packages\foolscap\negotiate.py:559:handlePLAINTEXTClient
--tw--c:\Python24\Lib\site-packages\foolscap\negotiate.py:571:startENCRYPTED
--tw--c:\Python24\Lib\site-packages\foolscap\negotiate.py:921:startTLS
--tw--]

[exarkun]

There's ISSLTransport and there's ITLSTransport. These are probably not the best interface names, but the former is for transports which are using SSLv2, SSLv3, or TLSv1 and the latter is for transports which support negotiating up to one of those mechanisms. A missing startTLS method indicates the transport does not provide ITLSTransport and could be unrelated to ISSLTransport/IReactorSSL. However, in practice, these two are pretty tightly related and most likely implementing one of them will also involve doing about 95% of the work to implement the other.

[Cortex]

I also find myself in position of need for IOCPReactor and SSL. As far as I can tell, the IOCPReactor is only choice for my development on Win32 platform and PollReactor is the only one for Unix, due to a FD limits on SelectReactor. I'm not so familiar with the procedures inside Twisted development team, I assume that since this ticket has 'Enhancement' status assigned, it is not a priority. Nevertheless, what are the chances that IOCP & SSL would be available in near future ? I've tried to make it work by myself, I achieved some results, there were some data coming in and no data coming out of the server with my 'implementation' of SSL-IOCP. I'm guessing that this failure of my attempt is mostly caused by massive structural differences between twisted.internet.tcp and twisted.internet.iocpreactor.tcp ... I just couldn't link objects inside twisted.internet.ssl with objects in twisted.internet.iocpreactor.tcp. Nevertheless I felt like making quite a progress considering I spent just two hours with this. I do not correctly understand the post above, about the TLS things with things when other things, so please, can anyone advise me on why is the tcp in IOCP so much different from the 'default' one ? Is there any chance that I can do this on my own and make such twisted.internet.iocpreactor.ssl which would work or are there rather serious reasons why not event attempt to do so ?

[exarkun]

(In [26757]) Branching to 'iocp-ssl-593'

[exarkun]

(In [26830]) Back out sslverify changes; made unnecessary for this feature by the addition of getHandle

refs #593, #3818

[exarkun]

(In [26863]) Branching to 'iocp-ssl-593-2'

[exarkun]

 Build results. Please review.

At the moment, this functionality depends on a branch of pyOpenSSL, <lp:~exarkun/pyopenssl/mem-bio>. That branch will be merged as it is now unless some issue comes up.

[glyph]

I'm leaving most feedback in the category of optional / separate ticket though, since the branch is pretty good as-is, and I don't think we should delay it for these improvements. Feel free to do some of the doc-fixes before merging though, if they're easy.

1. Obviously the pyopenssl change needs to land before this can. It would be nice to have a pyopenssl release as well.
2. The local-imports change in twisted.protocols.policies apparently has no accompanying test. I'm guessing it's important because something wants to import 'policies' somewhere? I'm going to break protocol and say that this test should actually be in a separate ticket though, since there's a whole category of "this shouldn't import the reactor" testing that we don't do anywhere yet.
3. I'm not a big fan of defining methods in conditional blocks. I'd much rather do something with a decorator that expresses the optionality of a method more explicitly.
4. I think it's worth emitting a warning (UserWarning, maybe?) when the IOCP reactor is in use but the PyOpenSSL feature required to do SSL is not available. Alternately, the decorator I mentioned in the previous point could, in the case where its dependency is not present, implement the methods with an expository NotImplementedError so that failing tests in applications will explain how to fix the missing dependency rather than just seeing an AttributeError.
5. At the top of twisted.protocols.tls: "if str(e) == ..." - really? If that message is accurate, then the test could easily just be something on OpenSSL.version.__version__. Perhaps better than that: add an explicit API to test for the feature you want, since 0.10 isn't out yet ;).
6. twisted.protocols.tls.TLSMemoryBIOProtocol._flushReceiveBIO really needs a docstring to tell me overall what it's supposed to be doing. Thanks for all the comments, though.
7. It's probably a bit unfair to call IOCP 'experimental' at this point, in the choosing-reactors document. It's supported, it seems to work, a lot of people have used it in real applications, and as of this change it's not missing any major functionality. Maybe there are still bugs, but I think we're done 'experimenting' ;-).
8. twisted.protocols.tls.__doc__ reads a bit awkwardly to me.
   1. As far as I can tell there isn't actually an IReactorSSL in there anywhere. Maybe you meant ISSLTransport?
   2. the paragraph which begins "Because the reactor's SSL and TLS APIs are likely implemented in a more efficient way" seems weirdly evocative of premature optimization. It would be useful to know about that particular implementation detail, but the reason I'd use ITLSTransport.startTLS is that it's simpler; fewer names to import, etc. Also, can't we fix that extra copying at some point, by using buffers or memory views or something?
   3. A more practical explanation of why you'd want such a thing would be better. Is there some reason one would want to do TLS/TLS/TCP? A better example, I think, would be to say that you could use it with a different plaintext transport, of which we have several (before even talking about the Vertex use-case); i.e. TLS to a subprocess, TLS to a serial port, TLS to a UNIX socket, TLS to a pipe...
   4. The paragraph about "This implementation approach avoids..." - while potentially useful to a maintainer, this paragraph seems pretty irrelevant and confusing to the average consumer of API documentation, e.g. a user who wants to do some stuff with TLS. The relevant bits are the above point about being able to wrap this around transports other than TCP.
   5. The code example doesn't make it clear that Echo is meant to be used in place of the user's application protocol. I probably would have written it to use a non-Twisted import like 'from mypackage.mynetwork import MyProtocol', but it would be just as good to say that "Echo" is the relevant application protocol here.
9. There appear to be a few lines that are uncovered by twisted.protocols.test.test_tls. I believe these are covered if testing under the iocp reactor, but I'm not entirely sure (coverage doesn't seem to work quite right on my Windows install).

Again, except for points 1 and possibly 9, the branch looks good to land as-is. Take the rest under advisement. Please file more tickets for the stuff you're not going to do though.

[exarkun]

1.  Changes landed.
2. The thing that failed without this change is trial --reactor iocp <anything>. I also don't know how we will write tests that verify that code doesn't trigger a default reactor installation. I'm pretty confident that we'll notice (on the iocp builder at least) if this functional change is regressed.
3. Filed #3828
4. Added in r26913
5. I can't add anything to 0.9. I could switch to a version test, but pyOpenSSL's version information is kept in a string, which would make the version test complicated and error prone. I think I'd rather leave this code how it is now.
6. Added in r26914
7. There's still one test that fails with iocpreactor. We're also pretty crunched for build slaves. I think iocp is close to being supported, but I don't think it's there yet. I think "Experimental" and "Stable" probably meant something different when they were first added to the reactor choosing document. Now what we really have is "supported" and "unsupported", where "supported" means "passes the whole test suite (almost all the time)" and "unsupported" means it doesn't. Once iocpreactor gets to "supported", I'd be happy to see it switch from "Experimental" to "Stable", or for the wording in this doc to be updated.
8. Changed in r26916
   • 1. I rephrased the first paragraph in the module docstring. It talks about ISSLTransport instead of IReactorSSL now (there was /almost/ an IReactorSSL implementation previously, but I decided it wasn't really worth having).
   • 2./3. I added some more examples to that section - onion routing, unix sockets, stdio.
   • 4. I removed the paragraph and the following one.
   • 5. Changed to import from a fictional application instead of using Echo.
9. They are. What would you like to see here?

[glyph]

Pretty much ready to land at this point. Do it! :)

1. OK. Great. I can't wait to start playing with this :).
2. OK. I eventually figured that out. Just bug me to tell you about some reactor-importability-testing stuff next time we're in earshot. The only other thing I might like to see here is a comment explaining why there's a local import, since of course lots of people will not run the IOCP tests (and it looks like our builders which would tell us there was a problem are both unsupported and dead).
3. OK, added some comments there.
4. OK.
5. OK, let's just do it that way then, it works. For future reference, my recommendation would be to add an attribute that could be trivially hasattr'd rather than checking stuff about exceptions.
6. OK, looks great.
7. OK. Sounds like maybe a doc bug or something, but I'm done thinking about this for now.
8. Changed in r26916
   1. OK.
   2. OK. I am still not super happy with the 'Because...' paragraph (see previous comments about premature optimization), but at this point I'm just nitpicking; the docstring makes sense, I can tell what is going on. Once it's merged maybe I'll have a try at writing a better docstring myself and see if you like it.
   3. OK.
   4. OK.
   5. OK.
9. OK. Given that they're covered somehow, that's fine for now. I would prefer it if all of twisted.protocols.tls were covered by unit-y tests in twisted.protocols.test.test_tls, since you can use (and thus want test coverage for) all the features twisted.protocols.tls on platforms where you can't run the iocpreactor tests. And I intend to do just that ;-). If you agree, just file a separate ticket to deal with this.

One final thought: we're starting to support a pretty complex matrix of library versions. I feel like the way these are installed and supported on the various builders is, at the very least highly mysterious, and if my intutions are correct about those mysteries, pretty disorganized. I would like it if we could find someone (ha ha!) to document which builders have which versions of which libraries, and maybe explain how we support them. Also, it would be good to have one slave which just had everything installed and failed if any tests skip; it seems like a mis-detection of a library feature to skip a bunch of tests could easily disable some test coverage and I don't know that we'd ever notice. (I think I got this idea from you.)

[exarkun]

(In [26919]) Merge iocp-ssl-593-2

Author: exarkun Reviewer: glyph Fixes: #593

Add a ProtocolWrapper-based implementation of TLS and use it to extend the IO Completion Ports reactor with implementations of IReactorSSL and ITLSTransport.

One of the existing SSL test suites is also changed to remove one of its base classes (and therefore some of the tests it was running); these tests had nothing to do with SSL, they were just redundant TCP tests.