Re: [Qemu-devel] [PULL v4 00/38] Test and build patches

[Fam Zheng <famz@redhat.com>]

On Fri, 09/15 13:31, Daniel P. Berrange wrote:
> On Fri, Sep 15, 2017 at 08:21:53PM +0800, Fam Zheng wrote:
> > On Fri, 09/15 12:40, Daniel P. Berrange wrote:
> > > On Fri, Sep 15, 2017 at 11:55:44AM +0100, Peter Maydell wrote:
> > > > On 15 September 2017 at 10:02, Fam Zheng <famz@redhat.com> wrote:
> > > > > The following changes since commit 04ef33052c205170c92df21ca0b4be4f3b102188:
> > > > >
> > > > >   tcg/tci: do not use ldst label (never implemented) (2017-09-11 19:24:05 +0100)
> > > > >
> > > > > are available in the git repository at:
> > > > >
> > > > >   git://github.com/famz/qemu.git tags/test-and-build-pull-request
> > > > >
> > > > > for you to fetch changes up to be78fe670401af14e6d63fce5c5467f751207871:
> > > > >
> > > > >   buildsys: Move rdma libs to per object (2017-09-15 15:05:24 +0800)
> > > > >
> > > > > ----------------------------------------------------------------
> > > > >
> > > > > ----------------------------------------------------------------
> > > > >
> > > > > Alex Bennée (4):
> > > > >   docker: ensure NOUSER for travis images
> > > > >   docker: docker.py make --no-cache skip checksum test
> > > > >   docker: don't install device-tree-compiler build-deps in travis.docker
> > > > >   docker: reduce noise when building travis.docker
> > > > >
> > > > > Fam Zheng (34):
> > > > >   docker: Update ubuntu image
> > > > >   docker: Enable features explicitly in test-full
> > > > >   tests/docker: Clean up paths
> > > > >   gitignore: Ignore vm test images
> > > > >   qemu.py: Add "wait()" method
> > > > >   scripts: Add archive-source.sh
> > > > >   tests: Add a test key pair
> > > > 
> > > > So, before I commit an ssh private key to our git repo,
> > > > can you explain why it's ok that this is public? The
> > > > commit message for the relevant patch doesn't really say.
> > > 
> > > IIUC, the public part of the key gets exposed to the guest images via
> > > cloud-init metadata. During boot the guest read this metadata and add
> > > the public key to authorized_keys. The private key is used by the test
> > > suite on the host so that it can now login to the guests.
> > > 
> > > So the risk here is that if these guests were exposed to the LAN in any
> > > way, someone could grab our private key and login to these guests.
> > > 
> > > What saves us is that the VMs are run with user mode slirp networking
> > > so AFAICT, aren't exposed to the LAN.  So as long as we don't change
> > > this to any kind of real networking, I think its acceptable to have
> > > the private key in it and doesn't expose developer's workstations to
> > > undue risk and avoids consuming system entropy to generate new keys
> > > during build.
> > 
> > The hostfwd does listen on a dynamic port on 0.0.0.0, so does vnc. I didn't
> > really care since it's for temporary guests and for me convenience outweighed a
> > bit.  The VM test is indeed less restricted than the docker ones such as in that
> > network is always available. Should it be a problem?
> 
> AFAICT there's no functional reason why it needs to listen on 0.0.0.0,
> instead of 127.0.0.1, so general security best practice says it should
> not expose this listening port on LAN interfaces for the developers
> machine, even if we think the risk is low.

Yes, makes sense, let's change it. The only disadvantage of 127.0.0.1 is if the
test is run on a remote host, you don't have to ssh to the host and proxy from
there to login to the guest. The test is automated, so accessing guest may be a
rare need outside patchew (a few months ago I frequently need to diagnose
hanging tests on patchew, no idea how this vm test will do :).

Fam