Bug #6934

[Authentication] Write tests for pam module and shellinabox config

Added by Tom Clegg about 4 years ago. Updated about 4 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
Deployment
Target version:
Start date:
08/07/2015
Due date:
% Done:

100%

Estimated time:
(Total: 0.00 h)
Story points:
-

Subtasks

Task #6963: Review 6934-pamResolvedTom Clegg


Related issues

Related to Arvados - Bug #6638: [Deployment] Python package backports should declare their C dependenciesResolved07/15/2015

Associated revisions

Revision f96550cc
Added by Tom Clegg about 4 years ago

Merge branch '6934-pam' refs #6934

Revision 4e76299b
Added by Tom Clegg about 4 years ago

Merge branch '6934-pam' refs #6934

Revision 4e76299b
Added by Tom Clegg about 4 years ago

Merge branch '6934-pam' refs #6934

Revision bdffa02f (diff)
Added by Tom Clegg about 4 years ago

6934: Add sdk/pam to run_upload_packages.py refs #6934

Revision bdffa02f (diff)
Added by Tom Clegg about 4 years ago

6934: Add sdk/pam to run_upload_packages.py refs #6934

Revision 75bbb895 (diff)
Added by Tom Clegg about 4 years ago

Use relative data_files paths so package is installable in a virtualenv. refs #6934

Unfortunately this also means /usr/pam-configs/arvados moves to
/usr/local/pam-configs/arvados, where pam-auth-update(8) does not see
it.

History

#1 Updated by Tom Clegg about 4 years ago

  • Category set to Deployment

#2 Updated by Tom Clegg about 4 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
  1. 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.)
  2. 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]

#3 Updated by Tom Clegg about 4 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.

#4 Updated by Tom Clegg about 4 years ago

  • Target version set to 2015-08-19 sprint

#5 Updated by Tom Clegg about 4 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.

#6 Updated by Brett Smith about 4 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 (like integration_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.

#7 Updated by Tom Clegg about 4 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?

https://arvados.org/projects/arvados/repository/revisions/c249e92657d76221cf3977145a8dfbd79e8f6d9a/show/sdk/python

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.

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.

  • 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 (like integration_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…
That's what I wanted, and I tried, but
  • 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

#8 Updated by Tom Clegg about 4 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

#9 Updated by Brett Smith about 4 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 the fpm_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.

#10 Updated by Tom Clegg about 4 years ago

  • Status changed from In Progress to Resolved

Also available in: Atom PDF