[WIP] CLI Kerberos authentication #1402

AndreaGiardini · 2014-12-08T13:29:19Z

This is a work in progress pull request to implement kerberos authentication via pulp CLI.

AndreaGiardini · 2014-12-08T13:31:01Z

https://github.com/AndreaGiardini/pulp/blob/master/client_lib/pulp/client/launcher.py#L71
https://github.com/AndreaGiardini/pulp/blob/master/client_admin/pulp/client/admin/admin_auth.py#L50

These lines create problem with the kerberos authentication (no password needed for kerb)...
Ideas? Additional kerberos check?

bmbouter · 2014-12-08T14:38:25Z

@AndreaGiardini I think these should have additional checks to ensure that kerberos is not being used.

https://github.com/AndreaGiardini/pulp/blob/master/client_lib/pulp/client/launcher.py#L71
https://github.com/AndreaGiardini/pulp/blob/master/client_admin/pulp/client/admin/admin_auth.py#L50

bmbouter · 2014-12-08T14:38:59Z

@AndreaGiardini I think release notes in master.rst should be written introducing this feature.

bmbouter · 2014-12-08T14:55:01Z

@AndreaGiardini It looks like python-krbV is on EL5, EL6, and FC19+, but not on EL7. Given that, I think the subprocess call to klist is good enough for now since we shouldn't pick up python-krbV yet.

One thing that you could do would be to ask the maintainer (mikeb) if he would be willing to add python-krbV 1.0.90-1 to EPEL7. Would you want to do that?

AndreaGiardini · 2014-12-08T15:06:13Z

Note: Pic module needs to be able to send kerberos requests well

bmbouter · 2014-12-08T15:40:06Z

@AndreaGiardini After looking into it python-kerberos should be available on all distros EL5, EL6, EL7, and FC19+. I see it listed on EL5 and FC19+ at this page. I also manually checked an EL6 box and an EL7 box and they both could receive python-kerberos from rhel-os.

Given that it is available everywhere I think it should be included in the spec file as a dependency that gets installed along with python-pulp-bindings. You can do that in this area of the spec file.

bmbouter · 2014-12-08T15:42:01Z

@AndreaGiardini Add it to PIC if you like, but I don't think it's a requirement. Generally, I think PIC should go away, and a generic tool like drest should be used instead but that is a whole other issue altogether.

bmbouter · 2014-12-11T21:12:32Z

@AndreaGiardini I read through this and it looks right overall. Its going to need some hand testing, and I'm not able to get to that today, but I wanted to let you know that it looks like you did the right thing. I know that it needs release notes in master.rst and the auth docs in general need to be updated to explain that Pulp now supports kerberos in bindings , pulp-admin, and pulp-consumer. I think it's probably smart to wait on writing the tests until we can verify correctness with hand testing. I hope to get to it soon, maybe tomorrow.

AndreaGiardini · 2015-01-12T10:54:11Z

Any update?

bowlofeggs · 2015-01-16T18:41:13Z

Hi @AndreaGiardini! I apologize for the delay. I am planning to check this out, but it may be a few more weeks before I have time to do a thorough analysis. Since this is an authentication change, it warrants more scrutiny than usual. Don't worry, we haven't forgotten it, and we are also very excited and happy that you have taken this work on!

pulpbot · 2015-01-20T18:29:25Z

Can one of the admins verify this patch?

bowlofeggs · 2015-02-03T20:41:49Z

@AndreaGiardini would you mind rebasing this PR down to one commit? I'm deploying a Kerberos domain right now and I'm testing this code. Sorry it took so long, but I'm hoping to get this done soon!

bowlofeggs · 2015-02-03T20:45:50Z

ok test

bowlofeggs · 2015-02-03T21:26:35Z

There does appear to be a small problem - if the user has no credential cache, and they also do not have an SSL certificate, pulp-admin has a traceback:

$ klist
klist: No credentials cache found (ticket cache KEYRING:persistent:1000:1000)
$ ls ~/.pulp/
admin.log  server_calls.log
$ pulp-admin rpm repo list
$ tail ~/.pulp/admin.log -n 27
2015-02-03 16:23:35,757 - INFO - BASIC selected
2015-02-03 16:23:39,240 - INFO - BASIC selected
2015-02-03 16:23:39,554 - ERROR - Client-side exception occurred
Traceback (most recent call last):
  File "/home/admin/devel/pulp/client_lib/pulp/client/extensions/core.py", line 469, in run
    exit_code = Cli.run(self, args)
  File "/usr/lib/python2.7/site-packages/okaara/cli.py", line 974, in run
    exit_code = command_or_section.execute(self.prompt, remaining_args)
  File "/home/admin/devel/pulp/client_lib/pulp/client/extensions/extensions.py", line 211, in execute
    return self.method(*arg_list, **clean_kwargs)
  File "/home/admin/devel/pulp/client_lib/pulp/client/commands/repo/cudl.py", line 329, in run
    self.display_repositories(**kwargs)
  File "/home/admin/devel/pulp/client_lib/pulp/client/commands/repo/cudl.py", line 357, in display_repositories
    repo_list = self.get_repositories(query_params, **kwargs)
  File "/home/admin/devel/pulp_rpm/extensions_admin/pulp_rpm/extensions/admin/repo_list.py", line 22, in get_repositories
    all_repos = self._all_repos(query_params, **kwargs)
  File "/home/admin/devel/pulp_rpm/extensions_admin/pulp_rpm/extensions/admin/repo_list.py", line 65, in _all_repos
    self.all_repos_cache = self.context.server.repo.repositories(query_params).response_body
  File "/home/admin/devel/pulp/bindings/pulp/bindings/repository.py", line 34, in repositories
    return self.server.GET(path, query_parameters)
  File "/home/admin/devel/pulp/bindings/pulp/bindings/server.py", line 98, in GET
    return self._request('GET', path, queries)
  File "/home/admin/devel/pulp/bindings/pulp/bindings/server.py", line 148, in _request
    response_code, response_body = self.server_wrapper.request(method, url, body)
  File "/home/admin/devel/pulp/bindings/pulp/bindings/server.py", line 377, in request
    response_body = response.read()
UnboundLocalError: local variable 'response' referenced before assignment

I'll keep testing, and I'll make more notes as I go.

Thanks again for this PR, this is great stuff!

bowlofeggs · 2015-02-03T22:20:09Z

It does appear that pulp-admin is now able to use kerberos:

[admin@pulppa ~]$ kinit
Password for admin@LOCALDOMAIN: 
[admin@pulppa ~]$ pulp-admin rpm repo list
+----------------------------------------------------------------------+
                            RPM Repositories
+----------------------------------------------------------------------+

bowlofeggs · 2015-02-03T22:31:07Z

bindings/pulp/bindings/server.py

+        """
+        Verify if the user has a valid Kerberos ticket.
+
+        :rtype: boolean


Please also add a :return: True if the user has a valid ticket here.

bowlofeggs · 2015-02-03T22:49:22Z

client_lib/pulp/client/launcher.py

+            username = options.username
+            password = options.password
+
+            logger.info("BASIC selected")


This might also be better to log at the debug level.

bowlofeggs · 2015-02-03T23:08:46Z

It does appear that password auth does work when the server is configured to use it, so that is good. Here are my thoughts so far:

This is great!
See if you can fix the situation when there has been no auth performed (no credential cache, and no ssl cert). It'd be nice if pulp-admin just told the user they aren't logged in, or something along those lines.
We need 100% test coverage on the new code. If you use our run-tests.py, you can use the --enable-coverage flag to see the test coverage on these modules.
It would be good to document this feature in two more places: 1) add a release note in the master.rst release note file, and 2) It would be valuable to describe how to use this feature at http://pulp.readthedocs.org/en/latest/user-guide/authentication.html. Be sure to mention that the user must install those python packages.
Please rebase your commits, squashing them into a single commit.
There is a lot of copied code here, and some loops that don't guard to make sure that they were successful. It would be good to try to refactor this code to be cleaner and use helper methods so we aren't introducing so many copies of the same functionality.
@bmbouter noted that we now have a requirement that usernames cannot contain the @ symbol. I had a different take on this - I think it might be reasonable to keep the @DOMAIN on the username, and require our users to create their users that way. This way, we don't lose the ability to have e-mail addresses as usernames, and it is nice to be explicit. I think that's the way I would prefer to go. What do you both think? If we go that way, we should document that as noted in 4) above.

Thanks so much, and let me know if you have any questions or if you want to discuss anything further. I'm happy to help you get this into Pulp!

bowlofeggs · 2015-02-04T15:11:00Z

ok to test

bowlofeggs · 2015-02-18T19:12:31Z

Hello @AndreaGiardini! Apologies again for so much delay on this PR. I hope that didn't dissuade you from continuing on it. Do you think you will find some time to look at my comments soon? I'm very excited about this feature, and I appreciate the time you've invested in it so far!

AndreaGiardini · 2015-02-19T08:49:31Z

Didn't have time yet, I'm quite busy at the moment! Will have a look as soon as i can!

pulpbot · 2015-06-18T15:29:17Z

Can one of the admins verify this patch?

AndreaGiardini · 2015-06-18T15:33:10Z

Sorry guys... i just don't have time anymore to keep working on this.
If you want, feel free to take my work and complete the patch

kfiresmith · 2016-04-07T15:37:55Z

I was turned onto this incomplete patch by @rbarlow via IRC as we also had the need for Kerberos (AD) based SSO logins. I asked the lead admin here at work to take a look because my Python is extremely weak and his input was that this path is something of a dead end.

Any future efforts on implementing Kerberos auth into pulp-admin should be done using mod_auth_gssapi.so and not mod_auth_kerb.so and/or Python modules that make direct Kerberos API calls as opposed to using GSSAPI.

For now at least in our infra with Pulp 2.8, we've been able to show that AD Kerberos integration works provided you relax the current input validation on pulp-admin to allow '@' in usernames, using both mod_auth_kerb.so OR mod_auth_gssapi.so. The remaining problem is that since pulp-admin in it's current state isn't GSSAPI-aware it cannot be used in lieu of the session certificate, but merely to obtain the session certificate. So it's certainly an improvement but it's not the promised land that this pull originally set out to be.

Hopefully at some point in the future someone with more Python chops will come by and take up the task of making pulp-admin GSSAPI compatible.

RFE 1166565 - Implement Kerberos authentication

e0203a2

bowlofeggs assigned bmbouter Dec 8, 2014

AndreaGiardini added 4 commits December 9, 2014 11:01

testing commit

457164c

Merge remote branch 'upstream/master' into 1166565

bd905c3

Merge remote branch 'upstream/master' into 1166565

21b6f87

Kerberos authentication for admin and consumer

c33b313

bowlofeggs assigned bowlofeggs and unassigned bmbouter Feb 3, 2015

bowlofeggs reviewed Feb 3, 2015
View reviewed changes

AndreaGiardini closed this Sep 21, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] CLI Kerberos authentication #1402

[WIP] CLI Kerberos authentication #1402

AndreaGiardini commented Dec 8, 2014

AndreaGiardini commented Dec 8, 2014

bmbouter commented Dec 8, 2014

bmbouter commented Dec 8, 2014

bmbouter commented Dec 8, 2014

AndreaGiardini commented Dec 8, 2014

bmbouter commented Dec 8, 2014

bmbouter commented Dec 8, 2014

bmbouter commented Dec 11, 2014

AndreaGiardini commented Jan 12, 2015

bowlofeggs commented Jan 16, 2015

pulpbot commented Jan 20, 2015

bowlofeggs commented Feb 3, 2015

bowlofeggs commented Feb 3, 2015

bowlofeggs commented Feb 3, 2015

bowlofeggs commented Feb 3, 2015

bowlofeggs Feb 3, 2015

bowlofeggs Feb 3, 2015

bowlofeggs commented Feb 3, 2015

bowlofeggs commented Feb 4, 2015

bowlofeggs commented Feb 18, 2015

AndreaGiardini commented Feb 19, 2015

pulpbot commented Jun 18, 2015

AndreaGiardini commented Jun 18, 2015

kfiresmith commented Apr 7, 2016

[WIP] CLI Kerberos authentication #1402

[WIP] CLI Kerberos authentication #1402

Conversation

AndreaGiardini commented Dec 8, 2014

AndreaGiardini commented Dec 8, 2014

bmbouter commented Dec 8, 2014

bmbouter commented Dec 8, 2014

bmbouter commented Dec 8, 2014

AndreaGiardini commented Dec 8, 2014

bmbouter commented Dec 8, 2014

bmbouter commented Dec 8, 2014

bmbouter commented Dec 11, 2014

AndreaGiardini commented Jan 12, 2015

bowlofeggs commented Jan 16, 2015

pulpbot commented Jan 20, 2015

bowlofeggs commented Feb 3, 2015

bowlofeggs commented Feb 3, 2015

bowlofeggs commented Feb 3, 2015

bowlofeggs commented Feb 3, 2015

bowlofeggs Feb 3, 2015

Choose a reason for hiding this comment

bowlofeggs Feb 3, 2015

Choose a reason for hiding this comment

bowlofeggs commented Feb 3, 2015

bowlofeggs commented Feb 4, 2015

bowlofeggs commented Feb 18, 2015

AndreaGiardini commented Feb 19, 2015

pulpbot commented Jun 18, 2015

AndreaGiardini commented Jun 18, 2015

kfiresmith commented Apr 7, 2016