[Spice-devel] [PATCH spice-server v6 00/10] SASL code improvements

[Frediano Ziglio]

> 
> Hey,
> 
> There seems to have 2 series mixed together, one with 9 patches, the
> other with 10 patches, and with some identical patches between the 2.

Yes, my mistake. The */9 series is basically the same with 2 patches
squashed together for a rebase mistake (plus another mistake the
kept the 2 series).

> I looked at the branch from your repository, and the series looks good
> to me. Did not look carefully at the test code this time, but this can
> always be improved if needed.
> For the series,
> Acked-by: Christophe Fergeau <cfergeau at redhat.com>
> 
> On Tue, Jan 09, 2018 at 07:44:55AM +0000, Frediano Ziglio wrote:
> > SASL authentication code were a bit dodgy to see.
> > The series have 2 parts:
> > - a test for SASL code;
> > - some code refactory.
> > 
> > Test is not that straightforward, maybe some code should be put in
> > different files. On the other end I don't think the code that is
> > mocking SASL will be reused in other tests.
> > The refactory is mainly put all the authentication logic in one file
> > (red-stream.c) instead of having half in reds.c and the other half in
> > red-stream.c. This allows to present a single function instead of one
> > for each authentication step.
> > 
> > Changes since v5:
> > - make the client emulator more robust making possible to run on
> >   Valgrind which causes some timing and syscall difference
> >   (disconnection is detected earlier causing a write after server
> >   close the connection to fail).
> > 
> > Changes since v4:
> > - test cases where SASL APIs reply too large packets;
> > - test the way a failure is reported from server.
> > 
> > Changes since v3:
> > - tested with a MIPS machine (quite hard to do, I should write how to
> >   do it!);

A bit on this. Looks like is not so easy to have a big endian machine
available. I decided to use an emulated MIPS machine.
Basically I followed at the end this guide to install the virtual machine:
https://gist.github.com/extremecoders-re/3ddddce9416fc8b293198cd13891b68c
The disk I used is bigger (30/40GB) and used LVM with a "mips" name (the
name is also used for the VG name).
Classic ssh install, enabling, private keys setup and you have a MIPS
machine (add the -redir option to Qemu for ssh).
At this time however is way slow to compile! So better to use the userspace
Qemu:
- install qemu-user-static and reboot the machine (to make binfmt
  configuration effective);
- use qemu-nbd to mount the MIPS hda.qcow image, in my cases the system
  added the VG volume automatically (if not you can use kpartx or other
  tools);
- mount the root partition of the VM on the host;
- cd <root_mount_point>;
- chroot . (obviously being root). Yes, you are giving a bash in a MIPS
  system, is the qemu-user-static + binfmt magic!
- /bin/su - <user> ... well, you always a no root user to develop,
  isn't it?

Now you can launch the usual tools, not fast as the host but surely faster than
full emulated and you can use make -j <A LOT> to make the build much faster.

For some tests however you need a real emulated machine (but most don't
have this issue).

> > - change way to iterate tests;
> > - lot of comments;
> > - some style changes;
> > - fixed a regression for big endian machines.
> > 
> > Changes since v2:
> > - removed patch to keep mechlist without copy, not documented really
> >   fine, safer to copy it.
> > 
> > Changes since v1:
> > - rebase on master;
> > - merged some patches;
> > - improved and fixed some comments;
> > - minor styles.
> > 
> > Frediano Ziglio (10):
> >   test-sasl: Initial SASL test
> >   test-sasl: Add code to mocking functions to test state
> >   test-sasl: Base test, connect using SASL
> >   test-sasl: Add tests for different mechanism names
> >   test-sasl: Add tests for different failures and cases
> >   test-sasl: Test how to server reports the failure
> >   Handle SASL initialisation mainly in red-stream.c
> >   red-stream: Handle properly endianness in SASL code
> >   red-stream: Unify start and step passes
> >   red-stream: Encapsulate all authentication state in RedSASLAuth
> > 
> >  configure.ac             |   1 +
> >  server/red-stream.c      | 330 ++++++++++-------------
> >  server/red-stream.h      |  11 +-
> >  server/reds.c            | 119 +--------
> >  server/tests/.gitignore  |   1 +
> >  server/tests/Makefile.am |   4 +
> >  server/tests/test-sasl.c | 665
> >  +++++++++++++++++++++++++++++++++++++++++++++++
> >  7 files changed, 825 insertions(+), 306 deletions(-)
> >  create mode 100644 server/tests/test-sasl.c
> >