From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55255) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c6vKm-00019a-OO for qemu-devel@nongnu.org; Wed, 16 Nov 2016 03:12:45 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c6vKl-0003Dc-Od for qemu-devel@nongnu.org; Wed, 16 Nov 2016 03:12:44 -0500 Received: from mail-it0-x244.google.com ([2607:f8b0:4001:c0b::244]:36662) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1c6vKl-0003DH-Gz for qemu-devel@nongnu.org; Wed, 16 Nov 2016 03:12:43 -0500 Received: by mail-it0-x244.google.com with SMTP id n68so5726664itn.3 for ; Wed, 16 Nov 2016 00:12:43 -0800 (PST) MIME-Version: 1.0 In-Reply-To: References: <1475035789-685-1-git-send-email-ashish.mittal@veritas.com> <20160928214510.GA2837@stefanha-x1.localdomain> From: Stefan Hajnoczi Date: Wed, 16 Nov 2016 08:12:41 +0000 Message-ID: Content-Type: text/plain; charset=UTF-8 Subject: Re: [Qemu-devel] [PATCH v7 RFC] block/vxhs: Initial commit to add Veritas HyperScale VxHS block device support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: ashish mittal Cc: qemu-devel , Paolo Bonzini , Kevin Wolf , Markus Armbruster , "Daniel P. Berrange" , Jeff Cody , Fam Zheng , Ashish Mittal , Ketan Nilangekar , Abhijit Dey , Buddhi.Madhav@veritas.com, Venkatesha.Mg@veritas.com On Tue, Nov 15, 2016 at 10:38 PM, ashish mittal wrote: > On Wed, Sep 28, 2016 at 2:45 PM, Stefan Hajnoczi wrote: >> On Tue, Sep 27, 2016 at 09:09:49PM -0700, Ashish Mittal wrote: >> 5. >> I don't see any endianness handling or portable alignment of struct >> fields in the network protocol code. Binary network protocols need to >> take care of these issue for portability. This means libqnio compiled >> for different architectures will not work. Do you plan to support any >> other architectures besides x86? >> > > No, we support only x86 and do not plan to support any other arch. > Please let me know if this necessitates any changes to the configure > script. I think no change to ./configure is necessary. The library will only ship on x86 so other platforms will never attempt to compile the code. >> 6. >> The networking code doesn't look robust: kvset uses assert() on input >> from the network so the other side of the connection could cause SIGABRT >> (coredump), the client uses the msg pointer as the cookie for the >> response packet so the server can easily crash the client by sending a >> bogus cookie value, etc. Even on the client side these things are >> troublesome but on a server they are guaranteed security issues. I >> didn't look into it deeply. Please audit the code. >> > > By design, our solution on OpenStack platform uses a closed set of > nodes communicating on dedicated networks. VxHS servers on all the > nodes are on a dedicated network. Clients (qemu) connects to these > only after reading the server IP from the XML (read by libvirt). The > XML cannot be modified without proper access. Therefore, IMO this > problem would be relevant only if someone were to use qnio as a > generic mode of communication/data transfer, but for our use-case, we > will not run into this problem. Is this explanation acceptable? No. The trust model is that the guest is untrusted and in the worst case may gain code execution in QEMU due to security bugs. You are assuming block/vxhs.c and libqnio are trusted but that assumption violates the trust model. In other words: 1. Guest exploits a security hole inside QEMU and gains code execution on the host. 2. Guest uses VxHS client file descriptor on host to send a malicious packet to VxHS server. 3. VxHS server is compromised by guest. 4. Compromised VxHS server sends malicious packets to all other connected clients. 5. All clients have been compromised. This means both the VxHS client and server must be robust. They have to validate inputs to avoid buffer overflows, assertion failures, infinite loops, etc. Stefan