search

mirage / mirage-qubes

Mirage support for writing QubesOS AppVM unikernels
BSD 2-Clause "Simplified" License
63 stars 11 forks source link

RExec: misbehaving clients can trigger exceptions #45

Closed reynir closed 3 years ago

reynir commented 4 years ago

I noticed that RExec.read_raw and thus indirectly RExec.read and RExec.read_line raise an exception through Lwt.fail if the client sends an unexpected message type. This behavior is undocumented, and I think it's perhaps more reasonable to close the connection to the misbehaving client like I implemented here for the client https://github.com/reynir/mirage-qubes/pull/1/commits/ee21564dd3a545751a77387de6ae707333e70ce2

https://github.com/mirage/mirage-qubes/blob/master/lib/rExec.ml#L92

reynir commented 4 years ago

I think it would be better if we close the connection to the misbehaving client. We could then either return `Eof or extend the signature with an error variant `Error of string. The latter would be a breaking change, but I think it's preferable to signal an error occured. What do you think?

hannesm commented 3 years ago

I'm in favour of what you propose: do not raise an exception, but instead return an error and close the connection.