Bug #6934
closed[Authentication] Write tests for pam module and shellinabox config
Updated by Tom Clegg over 9 years ago
Refactored as a python module with a tiny wrapper that goes in /lib/security. Made logging better. Added tests. Updated run-tests and run-build-packages.
Unit tests work.
Integration tests are tricky because- You need to be root to change
/etc/pam.d/whatever
in order to make PAM use the module you're testing. So I made a Dockerfile. (TODO before it can work from run-tests: point the pam config to the appropriate port number on the docker host. Currently"docker --add-host"
makes zzzzz.arvadosapi.com point to the docker host, but you have to edit the Dockerfile to set the port.) - Python and Perl PAM bindings both crash the caller when using pam_python.so. Haven't figured out exactly why, or how closely related those two crashes are. Python crashes during pam_authenticate(): the integration tests never get an answer. Perl crashes on exit: the integration tests can pass/fail as desired, but then the process exits with a stack trace.
Successful testing looks like this:
arvados/sdk/pam$ docker build --tag=arvados:pam_test .; docker run -it --add-host zzzzz.arvadosapi.com:"$(hostname -I |awk '{print $1}')" arvados:pam_test [...] Successfully built b6e3f8549d18 tail: cannot open `/var/log/auth.log' for reading: No such file or directory tail: `/var/log/auth.log' has become accessible good: Message (type 1): Arvados API token: Result (code 0): Success Aug 11 06:22:56 44ea28b06ac4 perl: arvados_pam: ['login', 'zzzzz.arvadosapi.com:53015', u'zzzzz-2x53u-382brsig8rp3065', '::1', 'active', '3kg6k6lzmp9kj5c', u'zzzzz-tpzed-xurymjxw79nv3jz', u'Active User']: Allow badtoken: Message (type 1): Arvados API token: Aug 11 06:22:56 44ea28b06ac4 perl: arvados_pam: ['login', 'zzzzz.arvadosapi.com:53015', None, '::1', 'active', 'badtokenmp9kj5c']: Error: <HttpError 401 when requesting https://zzzzz.arvadosapi.com:53015/arvados/v1/virtual_machines?alt=json&filters=%5B%5B%22hostname%22%2C+%22%3D%22%2C+%22testvm2.shell%22%5D%5D returned "Not logged in"> Result (code 7): Authentication failure Aug 11 06:22:59 44ea28b06ac4 perl: pam_securetty(login:auth): access denied: tty '/dev/null' is not secure ! badusername: Message (type 1): Arvados API token: Aug 11 06:22:59 44ea28b06ac4 perl: arvados_pam: ['login', 'zzzzz.arvadosapi.com:53015', u'zzzzz-2x53u-382brsig8rp3065', '::1', 'baduser', '3kg6k6lzmp9kj5c', u'zzzzz-tpzed-xurymjxw79nv3jz', u'Active User']: Deny Result (code 10): User not known to the underlying authentication module === OK === *** glibc detected *** perl: munmap_chunk(): invalid pointer: 0x00000000020aec5c *** ======= Backtrace: ========= /lib/x86_64-linux-gnu/libc.so.6(+0x75be6)[0x7f400ef45be6] /usr/lib/libperl.so.5.14(perl_destruct+0x1e2d)[0x7f400f95e10d] perl(main+0x111)[0x400f51] /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xfd)[0x7f400eeeeead] perl[0x400fc1] ======= Memory map: ======== 00400000-00402000 r-xp 00000000 00:26 93 /usr/bin/perl 00601000-00602000 r--p 00001000 00:26 93 /usr/bin/perl 00602000-00603000 rw-p 00002000 00:26 93 /usr/bin/perl 01939000-02a64000 rw-p 00000000 00:00 0 [heap]
Updated by Tom Clegg over 9 years ago
(Edit: ignore this, see note-5)
The sdk/python/fpm-info.sh I added here isn't really right -- libcurl should be a dependency of pycurl, not python-arvados-python-client. In #6638 I'm working on extending this fpm-info.sh setup to specify backports' dependencies; in the meantime this shouldn't hurt anything, and should make it possible to "apt-get install libpam-arvados" on debian systems -- OTOH it might be less trouble to just take it out, and live with satisfying dependencies manually until #6638. I could go either way.
Updated by Tom Clegg over 9 years ago
Removed the package dependency stuff from this branch, except sdk/pam/fpm-info.sh, which will just be ignored until #6638 starts using it. This means the deb package won't list libpam-python as a dependency, which is a regression -- but the alternatives aren't great either (use custom fpm instruction instead of standard python recipe, or squeeze more of #6638 into this branch) and #6638 should be done soon.
So, easiest way to run the tests and generate a python package:
cd .../src/arvados git fetch git checkout origin/6934-pam # c249e92 cd .../src/arvados-dev git fetch git checkout origin/6934-pam # c18ab40 jenkins/run-tests.sh WORKSPACE=.../src/arvados [--only sdk/pam --only-install sdk/pam]
Integration tests are not trivial to run, and exit non-zero even when everything works -- I'm prepared to live with that (and not run them from run-tests) for now.
Updated by Brett Smith over 9 years ago
Tom Clegg wrote:
Removed the package dependency stuff from this branch, except sdk/pam/fpm-info.sh, which will just be ignored until #6638 starts using it.
sdk/python/fpm-info.sh
is still in the branch. Though it's functionally a noop for now, it makes me very nervous, because what's here will break our CentOS packages (by declaring a dependency on a package that doesn't exist there) if used as planned.
This means the deb package won't list libpam-python as a dependency, which is a regression -- but the alternatives aren't great either (use custom fpm instruction instead of standard python recipe, …
fpm_build
accepts arbitrary fpm options, so you should be able to just add the --depends
to the end of that line of the recipe. That's a slight variation, but all the fpm_build
lines have variation anyway (e.g., for the description), so it doesn't seem like anything's lost to me.
- Why doesn't the Dockerfile refer to a standard development or test port for the API server? Why instruct users to replace it?
- On my Docker 1.6,
docker build -name arvados:pam_test .
complains that-name
is not a valid switch. I have--tag
, which matches the current reference. - Would
arvados/pam_test
better match our Docker tagging conventions? - It doesn't seem like I need to run the steps after
docker run
in the Dockerfile instructions. What's up with those? Neither CMD in the Dockerfile seems designed to give me a shell, so I'm not sure when I would run them. - Rather than having
test_integration.py
use environment heuristics to determine whether or not it's supposed to run, what if it was just named something that's not automatically run (likeintegration_test.py
, or whatever), and invoked specifically when desired (e.g., from the Dockerfile)? It won't surprise me if someday, someone runs the PAM tests as root on an Arvados shell host… - Why is rsyslog anticipated as a Recommends of the PAM package? Won't any syslog daemon serve equally well?
Thanks.
Updated by Tom Clegg over 9 years ago
Brett Smith wrote:
sdk/python/fpm-info.sh
is still in the branch. Though it's functionally a noop for now, it makes me very nervous, because what's here will break our CentOS packages (by declaring a dependency on a package that doesn't exist there) if used as planned.
Are you for sure looking at c249e92?
This means the deb package won't list libpam-python as a dependency, which is a regression -- but the alternatives aren't great either (use custom fpm instruction instead of standard python recipe, …
fpm_build
accepts arbitrary fpm options, so you should be able to just add the--depends
to the end of that line of the recipe. That's a slight variation, but all thefpm_build
lines have variation anyway (e.g., for the description), so it doesn't seem like anything's lost to me.
Good point, added it there. (This will make the centos package uninstallable. That's preferable to installable-but-unusable until we find a way to satisfy it, right?)
- Why doesn't the Dockerfile refer to a standard development or test port for the API server? Why instruct users to replace it?
Because run_test_servers chooses a random port. But yes, the default test port (3000) would be better than a port that never works. Fixed.
- On my Docker 1.6,
docker build -name arvados:pam_test .
complains that-name
is not a valid switch. I have--tag
, which matches the current reference.
Ah yes. Apparently I typed that instead of copying and pasting. Fixed.
- Would
arvados/pam_test
better match our Docker tagging conventions?
Yes, thanks. Fixed.
- It doesn't seem like I need to run the steps after
docker run
in the Dockerfile instructions. What's up with those? Neither CMD in the Dockerfile seems designed to give me a shell, so I'm not sure when I would run them.
Indeed, I seem to have only half-updated these instructions when adding the half-working automated tests, sorry. Updating to reflect two ways of testing: one with perl (which crashes), one with the interactive "login" command.
That's what I wanted, and I tried, but
- Rather than having
test_integration.py
use environment heuristics to determine whether or not it's supposed to run, what if it was just named something that's not automatically run (likeintegration_test.py
, or whatever), and invoked specifically when desired (e.g., from the Dockerfile)? It won't surprise me if someday, someone runs the PAM tests as root on an Arvados shell host…
- it makes it more awkward to run all tests in Docker (which seemed like a good idea) -- is there something like
--test-suite "*"
? - if you run the PAM tests as root on an Arvados shell host then the tests will succeed or fail depending on whether your config points to test data, real data, or a non-existent/down API server. Is that really high on the list of bad things in store for someone who is running test suites as root on a real system? I'd say the more interesting oddity is that you can't run the integration tests without being root -- but that didn't seem super important since the only sensible place to run this test is in a container or similarly disposable system.
- most importantly, I haven't actually found anywhere I can put it such that "python setup.py test" won't run it, but "python setup.py test --test-suite some-magic-phrase" will run it.
- tests/integration.py &rarr: setup.py test runs it
- tests/integration/test_pam.py → setup.py test --test-suite=tests.integration or =tests.integration.* crash "AttributeError: 'module' object has no attribute 'integration'"
- integration/test_pam.py → setup.py test --test-suite=integration crashes "ImportError: No module named integration"
Anyway, if you know the magic spell here, I agree it would be better to avoid the guesswork.
- Why is rsyslog anticipated as a Recommends of the PAM package? Won't any syslog daemon serve equally well?
Sure. Should this just say syslogd? I initially tried "syslogd" and got busybox-syslogd, which didn't log anything for me with its default config, but I didn't come back and push on it too hard -- I might have been doing something else wrong at the time.
Now at
Updated by Tom Clegg over 9 years ago
Tom Clegg wrote:
- integration/test_pam.py → setup.py test --test-suite=integration crashes "ImportError: No module named integration"
Ah, found it. This way works if I remember to make an empty integration/__init__.py
. Updating (but using "integration_tests" instead of just "integration").
→ 3dc08cb
Updated by Brett Smith over 9 years ago
Tom Clegg wrote:
Brett Smith wrote:
sdk/python/fpm-info.sh
is still in the branch. Though it's functionally a noop for now, it makes me very nervous, because what's here will break our CentOS packages (by declaring a dependency on a package that doesn't exist there) if used as planned.Are you for sure looking at c249e92?
Nope, my bad. Sorry for the noise.
This means the deb package won't list libpam-python as a dependency, which is a regression -- but the alternatives aren't great either (use custom fpm instruction instead of standard python recipe, …
fpm_build
accepts arbitrary fpm options, so you should be able to just add the--depends
to the end of that line of the recipe. That's a slight variation, but all thefpm_build
lines have variation anyway (e.g., for the description), so it doesn't seem like anything's lost to me.Good point, added it there. (This will make the centos package uninstallable. That's preferable to installable-but-unusable until we find a way to satisfy it, right?)
Uhh, previously we just didn't build the package at all for CentOS. So, the previous state is "uninstallable because it doesn't exist."
Personally, that seems slightly preferable to me than "uninstallable because it has unsatisfiable dependencies," so I think it'd be a good change to do this build inside an if block as the old version did. But reasonable minds can disagree.
- Why is rsyslog anticipated as a Recommends of the PAM package? Won't any syslog daemon serve equally well?
Sure. Should this just say syslogd?
I think you want "system-log-daemon". That's the virtual package provided by rsyslog, busybox-syslogd, and several others.
These are good to merge. Thanks.
Updated by Tom Clegg over 9 years ago
- Status changed from In Progress to Resolved