[patch] system activation 3

[Havoc Pennington]

Hi,

Richard Hughes wrote:
> Latest patch attached. Some security review would be lovely. I also need
> somebody to point out where, how many and to what extent that unit tests
> are required, and then I can write them tomorrow.
> 

I won't have a chance to review this until Thursday or Friday probably.

For the unit tests, there isn't a bright line really, I would say the 
baseline is to try to at least run most of the codepaths (to find any 
segfaults or memleaks along them), and the other main thing is to be 
sure any "tricky parts" or algorithms are tested. Out-of-memory handling 
is important to test.

Testing correct behavior if the setuid helper gets some simple bad input 
would not be bad; e.g. nonexistent service name, no args at all, extra 
args, etc. - it would be embarrassing if it could be exploited in any of 
those ways! This might be doable with a simple shell script that passes 
it a variety of stuff and checks the exit code and whether it segv'd.
(I think bash must have some way to ask if a process exited normally or 
on a signal.)

Otherwise, two basic strategies you can use:

  - look at dispatch.c, which creates a BusContext (representing
    most of the system bus code) and talks to it; one virtue of this
    setup is that it tests out-of-memory codepaths. There are activation
    tests in here already, iirc, so you should be able to just do the
    same activation tests only using a helper. Of course in the test
    setup, the helper won't really be setuid and won't really change
    user, probably some env variable can control that, or just
    piggyback on whether one of the existing test-related env variables
    is set.

  - if you want to test an actual bus, look at test/name-test/Makefile.am
    and the shell script in there. This directory is called name-test but
    in fact has several tests in it it; the real distinguishing feature
    of the directory is that it starts up a bus daemon with a special
    config file, and then the tests are client apps that connect to the
    special daemon. For what you're doing, you'd probably want to start
    a slightly different special daemon with a helper configured, then
    run a client that activates some stuff.

    I'm not sure messing with this approach will be much more useful
    than dispatch.c though, I'm just mentioning it in case it is.

You can also extend existing tests as needed, e.g. there are 
config-parser.c tests; you could be sure new tags are tested, and also 
use the same setup to test the trivial config parser.

You can put any data files for testing in test/data.

Obviously it's possible to spend infinite time writing tests. The most 
important things are to be sure the primary codepaths are exercised and 
that any tricky algorithms or logic are factored out so they are 
unit-testable, and then unit tested. (Lots of times you need to refactor 
a little bit or add "hooks" to a module to be able to test.)

I think you don't have many algorithms or anything here so it's mostly 
just a matter of ensuring we do run the helper and run activation.c in 
"helper mode" during make check, which should be mostly covered by a 
dispatch.c addition.

Offhand in summary I would guess:
  - do the activation tests in dispatch.c a second time with the
    helper, should exercise most of this code
  - be sure passing an error code from helper up to the bus
    is tested by the dispatch.c tests
  - test new tags for config-parser, and test the trivial config
    parser in the same way as the old one
  - the simple shell script to pass various args to the helper

But use your judgment.

You don't have to test complicated stuff, the most useful tests in many 
ways are just the ones that verify everything runs in the normal, 
expected case; prevents many a brown-paper-bag release.

Havoc