[BUG] WampRawSocketServerProtocol leaks TransportLost from dataReceived on bad magic byte (logged by Twisted as Unhandled Error)
bug
## Bug Description
**Summary**: `WampRawSocketProtocol.abort()` raises `TransportLost` whenever no WAMP session is attached, including during the rawsocket opening handshake — so a peer that sends a wrong magic byte triggers an exception that escapes `dataReceived()` and is logged by Twisted as an "Unhandled Error" with a full traceback, while the TCP socket is *not* closed by autobahn.
**Expected Behavior**:
On a bad opening-handshake magic byte, `WampRawSocketServerProtocol.dataReceived()` should close the TCP connection cleanly (`abortConnection()` / `loseConnection()`) and return without raising anything. A single concise `log.warn(…)` line per probe is appropriate; a full Twisted "Unhandled Error" stack trace per probe is not.
**Actual Behavior**:
`dataReceived()` warns about the bad magic, then calls `self.abort()`. `abort()` evaluates `self.isOpen()`, which checks `self._session is not None`; because the handshake has not completed yet, `_session` is `None`, so the `else` branch runs:
```python
else:
raise TransportLost()
```
The exception unwinds back through `dataReceived()` into Twisted's TCP reader, which logs:
```text
WampRawSocketServerProtocol: invalid magic byte (octet 1) in opening handshake: was 71, but expected 127
Unhandled Error
Traceback (most recent call last):
File ".../twisted/python/log.py", line 96, in callWithLogger ...
...
File ".../autobahn/twisted/rawsocket.py", line 268, in dataReceived
self.abort()
File ".../autobahn/twisted/rawsocket.py", line 231, in abort
raise TransportLost()
autobahn.wamp.exception.TransportLost:
```
At no point does autobahn itself close the socket — the call to `abortConnection()` / `loseConnection()` lives behind the `if self.isOpen():` branch that is never entered for a pre-handshake transport.
This is reproducible in every release I could check (v21.3.1, v22.x, v23.x, v24.4.x, v25.6.x, v25.9.x, v25.10.x, v25.12.x); `abort()` and `isOpen()` are byte-identical across them.
## Reproduction Steps
1. Start any rawsocket-listening WAMP server on a port (e.g. Crossbar with a `rawsocket` transport, or the autobahn server example).
2. From a separate process, open a TCP connection to that port and send any 4 bytes whose first byte is not `0x7F` — for example `GET ` (an HTTP-style port-scan probe) or `\x16\x03\x01\x00` (a TLS ClientHello prefix).
3. Observe the Twisted log: one "Unhandled Error" stack trace per probe; the connection is closed only because the *peer* closed its end, not because autobahn closed its own.
In the field, what surfaces this is **any TCP service scanner** (nmap `-sV` service-detection probes, masscan, vulnerability scanners, etc.) sweeping the rawsocket port: each probe yields a logged exception, and at higher rates the noise becomes operationally painful.
**Minimal Reproducible Example**:
```python
# No router, no networking — exercises the bug in isolation.
from twisted.test.proto_helpers import StringTransport
from autobahn.twisted.rawsocket import WampRawSocketServerProtocol, WampRawSocketServerFactory
from autobahn.wamp.serializer import MsgPackSerializer
factory = WampRawSocketServerFactory(lambda: None, [MsgPackSerializer()])
proto = factory.buildProtocol(("127.0.0.1", 0))
transport = StringTransport()
proto.makeConnection(transport)
# Any 4 bytes whose first octet != 0x7F:
proto.dataReceived(b"GET ")
# -> autobahn.wamp.exception.TransportLost
# (and transport.disconnecting is False — the socket was never closed)
```
## Environment
- **Package**: autobahn (autobahn-python)
- **Version**: confirmed on **21.3.1** and **25.12.2** (and every tagged release between — the relevant code is unchanged)
- **Python Version**: CPython 3.13 (also reproduces on CPython 3.11)
- **Operating System**: Gentoo Linux (kernel 6.18); not OS-specific
- **Framework**: Twisted (≥ 22.10.0 in the 25.x line)
**Relevant Logs**:
The two functions involved (master / v25_12_2 line numbers; the line numbers in v21.3.1 are 220 and 244, behaviour identical):
[`src/autobahn/twisted/rawsocket.py:279-289`](https://github.com/crossbario/autobahn-python/blob/v25_12_2/src/autobahn/twisted/rawsocket.py#L279-L289) — `isOpen()` keys on the WAMP session, not the transport:
```python
def isOpen(self):
"""
Implements :func:`autobahn.wamp.interfaces.ITransport.isOpen`
"""
return self._session is not None
```
[`src/autobahn/twisted/rawsocket.py:294-305`](https://github.com/crossbario/autobahn-python/blob/v25_12_2/src/autobahn/twisted/rawsocket.py#L294-L305) — `abort()` falls through to `raise` when no session:
```python
def abort(self):
"""
Implements :func:`autobahn.wamp.interfaces.ITransport.abort`
"""
if self.isOpen():
if hasattr(self.transport, "abortConnection"):
# ProcessProtocol lacks abortConnection()
self.transport.abortConnection()
else:
self.transport.loseConnection()
else:
raise TransportLost()
```
[`src/autobahn/twisted/rawsocket.py:331-344`](https://github.com/crossbario/autobahn-python/blob/v25_12_2/src/autobahn/twisted/rawsocket.py#L331-L344) — `dataReceived()` calls `abort()` on bad magic and **does not catch the exception**:
```python
if _magic != 127:
self.log.warn(
"WampRawSocketServerProtocol: invalid magic byte (octet 1) in"
" opening handshake: was {magic}, but expected 127",
magic=_magic,
)
self.abort()
```
The client-side `WampRawSocketClientProtocol.dataReceived()` has the symmetric pattern at line ~466 and is affected identically.
**Workaround** for operators hitting this in the field: monkey-patch `abort()` to close the transport when it exists, or filter the rawsocket port via firewall so non-WAMP traffic never reaches it. Neither is appealing.
**Screenshots**:
N/A — startup is unaffected; this surfaces as log noise during normal operation whenever something probes the rawsocket port.
**Related Issues**:
- None found by searching the repository for "invalid magic byte" or "rawsocket TransportLost".
## Checklist
- [x] I have searched existing issues to avoid duplicates
- [x] I have provided a minimal reproducible example
- [x] I have included version information
- [x] I have included error messages/logs
## Proposed Patch
Single-line fix inside `WampRawSocketProtocol.abort()`. The current guard keys on `isOpen()` (i.e. whether a WAMP session is attached), but the right question to ask is whether the *transport* is still around — if the TCP socket exists, we can close it, regardless of whether the WAMP layer ever made it that far.
The patch preserves the existing contract for callers who legitimately need to know that the transport is already gone (post-`connectionLost`), and stops `abort()` from being a thrown-exception-only no-op during the handshake.
```diff
--- a/src/autobahn/twisted/rawsocket.py
+++ b/src/autobahn/twisted/rawsocket.py
@@ -294,11 +294,19 @@
def abort(self):
"""
Implements :func:`autobahn.wamp.interfaces.ITransport.abort`
"""
- if self.isOpen():
+ # Close the underlying transport whenever it exists, even
+ # before the WAMP session has been attached. Previously
+ # ``if self.isOpen():`` returned False during the rawsocket
+ # handshake (because ``_session`` is set only after the
+ # handshake completes), so the ``else`` branch raised
+ # ``TransportLost()`` without closing the TCP socket — and the
+ # exception escaped through ``dataReceived()`` as a noisy
+ # Twisted "Unhandled Error" with a stack trace per probe.
+ if self.transport is not None:
if hasattr(self.transport, "abortConnection"):
# ProcessProtocol lacks abortConnection()
self.transport.abortConnection()
else:
self.transport.loseConnection()
else:
raise TransportLost()
```
Verified locally: `patch --dry-run` accepts the hunk cleanly against the v25_12_2 tag, the resulting file passes `ast.parse`, and the minimal reproducer above no longer raises (transport closes cleanly).
### Caller audit (all sites that call `abort()` in `rawsocket.py`)
| Caller | Before patch | After patch |
|---|---|---|
| `WampRawSocketServerProtocol.dataReceived` — bad magic byte (line 340) | raises `TransportLost`; socket stays open | socket closed cleanly; no exception |
| `WampRawSocketClientProtocol.dataReceived` — bad magic byte (~line 466) | same broken behaviour | same fix |
| `WampRawSocketProtocol.stringReceived` — `PayloadExceededError` (line 226) | works (session attached → `isOpen()` True) | unchanged |
| `WampRawSocketProtocol.stringReceived` — `SerializationError` (line 234) | works | unchanged |
| `WampRawSocketProtocol.stringReceived` — generic `Exception` (line 242) | works | unchanged |
No call site relied on `abort()` raising while the transport was still alive; that was strictly the buggy edge case.
### Suggested test (in `src/autobahn/twisted/test/test_tx_rawsocket.py`)
```python
def test_abort_on_bad_magic_closes_transport_silently(self):
from twisted.test import proto_helpers
from autobahn.twisted.rawsocket import WampRawSocketServerFactory
from autobahn.wamp.serializer import MsgPackSerializer
factory = WampRawSocketServerFactory(lambda: None, [MsgPackSerializer()])
proto = factory.buildProtocol(("127.0.0.1", 0))
transport = proto_helpers.StringTransport()
proto.makeConnection(transport)
# Any non-0x7F first byte triggers the legacy path.
proto.dataReceived(b"GET ")
# The transport must be closed; nothing should escape.
self.assertTrue(transport.disconnecting)
```
Before the patch this test fails because `dataReceived` propagates `TransportLost`; after, it passes.
0 条评论