Sigh. Re-submitting, with correct subject.

When the controller receive loop currently accepts an OpenFlow message, it does 
not validate the message length.
As a result, a malicious or malfunctioning switch could cause send a message 
that would result in the receive loop making no forward progress.

This patch ensures that the message length passed in the OpenFlow message is 
validated against the specified minimum, and forced to that value if it is 
smaller.

Thanks to Samuel Jero (at Purdue's Dependable and Secure Distributed Systems 
Lab) for discovering this issue.

Signed-off-by: Victor J. Orlikowski <[email protected]>
---

diff --git a/ryu/controller/controller.py b/ryu/controller/controller.py
index cf9deb9..8300f1a 100644
--- a/ryu/controller/controller.py
+++ b/ryu/controller/controller.py
@@ -229,12 +229,15 @@ class Datapath(ofproto_protocol.ProtocolDesc):
    @_deactivate
    def _recv_loop(self):
        buf = bytearray()
-        required_len = ofproto_common.OFP_HEADER_SIZE
-
        count = 0
+        min_read_len = remaining_read_len = ofproto_common.OFP_HEADER_SIZE
+
        while self.state != DEAD_DISPATCHER:
            try:
-                ret = self.socket.recv(required_len)
+                read_len = min_read_len
+                if (remaining_read_len > min_read_len):
+                    read_len = remaining_read_len
+                ret = self.socket.recv(read_len)
            except SocketTimeout:
                continue
            except ssl.SSLError:
@@ -248,10 +251,16 @@ class Datapath(ofproto_protocol.ProtocolDesc):
                break

            buf += ret
-            while len(buf) >= required_len:
+            buf_len = len(buf)
+            while buf_len >= min_read_len:
                (version, msg_type, msg_len, xid) = ofproto_parser.header(buf)
-                required_len = msg_len
-                if len(buf) < required_len:
+                if (msg_len < min_read_len):
+                    # Someone isn't playing nicely; log it, and try something 
sane.
+                    LOG.debug("Message with invalid length %s received from 
switch at address %s",
+                              msg_len, self.address)
+                    msg_len = min_read_len
+                if buf_len < msg_len:
+                    remaining_read_len = (msg_len - buf_len)
                    break

                msg = ofproto_parser.msg(
@@ -268,8 +277,9 @@ class Datapath(ofproto_protocol.ProtocolDesc):
                    for handler in handlers:
                        handler(ev)

-                buf = buf[required_len:]
-                required_len = ofproto_common.OFP_HEADER_SIZE
+                buf = buf[msg_len:]
+                buf_len = len(buf)
+                remaining_read_len = min_read_len

                # We need to schedule other greenlets. Otherwise, ryu
                # can't accept new switches or handle the existing

Best,
Victor
--
Victor J. Orlikowski <> vjo@[cs.]duke.edu

Attachment: message_length_validation.patch
Description: message_length_validation.patch



------------------------------------------------------------------------------
_______________________________________________
Ryu-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ryu-devel

Reply via email to