crossbario / autobahn-testsuite

Autobahn WebSocket protocol testsuite
https://crossbar.io/autobahn/
Apache License 2.0
973 stars 82 forks source link

Problem running fuzzingserver in virtualenv #122

Closed neoxic closed 2 years ago

neoxic commented 2 years ago

I tried both Python2 and PyPy with no luck in testing my server. I follow the docs, but wstest fails in unhandled exception:

$ virtualenv -p python2 ~/wstest
created virtual environment CPython2.7.18.final.0-64 in 166ms
  creator CPython2Posix(dest=/home/seny/wstest, clear=False, no_vcs_ignore=False, global=False)
  seeder FromAppData(download=False, pip=bundle, setuptools=bundle, wheel=bundle, via=copy, app_data_dir=/home/seny/.local/share/virtualenv)
    added seed packages: pip==20.3.4, setuptools==44.1.1, wheel==0.37.1
  activators BashActivator,CShellActivator,FishActivator,NushellActivator,PowerShellActivator,PythonActivator

$ source ~/wstest/bin/activate
$ pip install autobahntestsuite
...
$ wstest -m fuzzingclient
/home/seny/wstest/lib/python2.7/site-packages/OpenSSL/crypto.py:14: CryptographyDeprecationWarning: Python 2 is no longer supported by the Python core team. Support for it is now deprecated in cryptography, and will be removed in the next release.
  from cryptography import utils, x509
Using implicit spec file 'fuzzingclient.json'
Loading spec from /home/seny/fuzzingclient.json

Using Twisted reactor class <class 'twisted.internet.epollreactor.EPollReactor'>
Using UTF8 Validator class <type 'wsaccel.utf8validator.Utf8Validator'>
Using XOR Masker classes <type 'wsaccel.xormask.XorMaskerNull'>

Autobahn Fuzzing WebSocket Client (Autobahn Testsuite Version 0.8.2 / Autobahn Version 0.10.9)
Ok, will run 517 test cases against 1 servers
...
Servers = [u'ws://127.0.0.1:9001']
Unhandled Error
Traceback (most recent call last):
  File "/home/seny/wstest/lib/python2.7/site-packages/autobahntestsuite/wstest.py", line 346, in run
    start(options, spec)
  File "/home/seny/wstest/lib/python2.7/site-packages/autobahntestsuite/wstest.py", line 280, in start
    reactor.run()
  File "/home/seny/wstest/lib/python2.7/site-packages/twisted/internet/base.py", line 1283, in run
    self.mainLoop()
  File "/home/seny/wstest/lib/python2.7/site-packages/twisted/internet/base.py", line 1292, in mainLoop
    self.runUntilCurrent()
--- <exception caught here> ---
  File "/home/seny/wstest/lib/python2.7/site-packages/twisted/internet/base.py", line 913, in runUntilCurrent
    call.func(*call.args, **call.kw)
  File "/home/seny/wstest/lib/python2.7/site-packages/twisted/internet/tcp.py", line 520, in connectionLost
    self.connector.connectionLost(reason)
  File "/home/seny/wstest/lib/python2.7/site-packages/twisted/internet/base.py", line 1171, in connectionLost
    self.factory.clientConnectionLost(self, reason)
  File "/home/seny/wstest/lib/python2.7/site-packages/autobahntestsuite/fuzzing.py", line 1292, in clientConnectionLost
    self.createReports()
  File "/home/seny/wstest/lib/python2.7/site-packages/autobahntestsuite/fuzzing.py", line 469, in createReports
    self.createMasterReportHTML(self.outdir)
  File "/home/seny/wstest/lib/python2.7/site-packages/autobahntestsuite/fuzzing.py", line 670, in createMasterReportHTML
    agent_case_report_file = self.makeAgentCaseReportFilename(agentId, caseId, ext = 'html')
  File "/home/seny/wstest/lib/python2.7/site-packages/autobahntestsuite/fuzzing.py", line 498, in makeAgentCaseReportFilename
    return self.cleanForFilename(agentId) + "_case_" + c + "." + ext
  File "/home/seny/wstest/lib/python2.7/site-packages/autobahntestsuite/fuzzing.py", line 487, in cleanForFilename
    s0 = ''.join([c if c in "abcdefghjiklmnopqrstuvwxyz0123456789" else " " for c in str.strip().lower()])
exceptions.AttributeError: 'NoneType' object has no attribute 'strip'
neoxic commented 2 years ago

This can be reproduced with a server that fails all handshakes with 400. In my case, the implementation failed a handshake due to Autobahn's Upgrade: WebSocket instead of all lowercase Upgrade: websocket.

neoxic commented 2 years ago

As per RFC 6455:

4. Opening Handshake 4.1. Client Requirements ...

  1. The request MUST contain an |Upgrade| header field whose value MUST include the "websocket" keyword.
  2. The request MUST contain a |Connection| header field whose value MUST include the "Upgrade" token.

Please note that the "websocket" keyword must be all lowercase in contrast to the "Upgrade" token.

oberstet commented 2 years ago

Please note that the "websocket" keyword must be all lowercase in contrast to the "Upgrade" token.

No, I don't believe so:

[4.1](https://datatracker.ietf.org/doc/html/rfc6455#section-4.1).  Client Requirements
...

   2.  If the response lacks an |Upgrade| header field or the |Upgrade|
       header field contains a value that is not an ASCII case-
       insensitive match for the value "websocket", the client MUST
       _Fail the WebSocket Connection_.

   3.  If the response lacks a |Connection| header field or the
       |Connection| header field doesn't contain a token that is an
       ASCII case-insensitive match for the value "Upgrade", the client
       MUST _Fail the WebSocket Connection_.

So a client should accept even accept Upgrade: wEbSoCkeT

neoxic commented 2 years ago

I guess you are actually right. I just gave it another try, and:

4.2.1. Reading the Client's Opening Handshake ....

  1. An |Upgrade| header field containing the value "websocket", treated as an ASCII case-insensitive value.
  2. A |Connection| header field that includes the token "Upgrade", treated as an ASCII case-insensitive value.

My bad.

oberstet commented 2 years ago

no problem! I remember discussion around this in the IETF WG when the RFC was still in draft ... it really should contain more examples, probably including mixed-case, and it should not use lowercase only in 1 header, and camelcase for another. well. anyways, closing then ..

neoxic commented 2 years ago

The Unhandled Error is still there though.:-) Upgrade: WebSocket was just the way to trigger it. More presicely, a 400 Bad Request response to every handshake, of course.

oberstet commented 2 years ago

yes, not ideal. I should note: the testsuite contains hundreds of tests which together cover a lot of the spec - BUT: those are all for everything that happens after the initial HTTP upgrading request/response!! the testsuite does NOT contain any tests for the HTTP handshake. I wanted to add that "later" .. never got to it .. and it would be quite waste amount of work, simply because HTTP is so complex (at the very detail level), and because a 100% compliant HTTP might not work great in practice (because of all the broken shit that insists to continue eating shit) ... anyways, no time, sorry;)

neoxic commented 2 years ago

That's ok, totally understand.:) Anyways, I'm grateful for the great yet not perfect test suite!:) Thank you!