John Dennis wrote:
Attached are the patches for the radius work, please review. * The first patch is for freeradius, it contains the C code modifications for performing a SASL GSSAPI bind to the IPA directory server using the radius service principal's keytab, for querying the radius client information from LDAP, and the autoconf modifications to support conditional compilation of the new krb and sasl code. The rpms have been build using Koji and can be accessed here: http://koji.fedoraproject.org/koji/buildinfo?buildID=26067 * The second patch is against our IPA source tree, please note this is a patch showing the final cumulative differences for easier review, not the 23 mercurial changesets comprising it. I will send Karl the changeset file or he can pull the changes from my repo depending on which he thinks is easier (contact me directly for the repo address and password). * These are the files changed in the patch: ipa-admintools/ipa-addradiusclient ipa-admintools/ipa-addradiusprofile ipa-admintools/ipa-delradiusclient ipa-admintools/ipa-delradiusprofile ipa-admintools/ipa-findradiusclient ipa-admintools/ipa-findradiusprofile ipa-admintools/ipa-radiusclientmod ipa-admintools/ipa-radiusprofilemod ipa-admintools/Makefile ipa-python/ipaclient.py ipa-python/ipautil.py ipa-python/ipavalidate.py ipa-python/radius_util.py ipa-python/rpcclient.py ipa-server/ipa-install/share/60radius.ldif ipa-server/ipa-install/share/bootstrap-template.ldif ipa-server/ipa-install/share/default-aci.ldif ipa-server/ipa-install/share/encrypted_attribute.ldif ipa-server/ipa-install/share/radius.radiusd.conf.template ipa-server/ipaserver/radiusinstance.py ipa-server/xmlrpc-server/funcs.py ipa-server/xmlrpc-server/ipaxmlrpc.py * I believe all the changes are exclusively related to radius and no code in the general IPA code base was touched otherwise. * During review I would like to draw your attention to the following items: bootstrap-template.ldif: adds radius clients and profiles containers under cn=services,cn=etc encrypted_attribute.ldif: sets the secret attribute in the radius client objectclass to be encrypted using AES. At some point the ldiff should be replaced with calls to perform an ldap modify as discussed on the mailing list. default-aci.ldif: The ACI's were modified in two ways. First the radiusprofile object class was added to the "Account Admins can manage Users and Groups" acl. Second, the "Only radius and admin can access radius service data" acl was added which denies all access to radius service data except for the admin and the radius service principal. * The command line utilities which manipulate LDAP attrbutes can: - take the attribute/value as a command line arg - take any number of argument strings containing any number of attribute/value pairs - can read attribute/value pairs from a file or stdin - can interactively prompt for attribute/values with auto completion of the attribute name and auto completion of the default value. - can delete attributes (critical as radius sometimes uses the presence or absense of an attribute as a flag, setting the attribute value to the empty string is not sufficient). * Some general utility code was added to ipautil.py which provides the following: - perform attribute/value pair auto completion on a TTY, returns set of attribute/value pairs, values will auto complete to their defaults or their previous value. - perform name auto completion on a TTY, returns a list of names. - parse attribute/value pairs from text strings (properly handles quoting and escaped quotes) - read attribute/value pairs from a file (file can contain comments) - parse name list and read names from a file (can contain comments). - format for output a list of names in standard column format
Just a few comments.1. In freeradius is it possible to have SASL2 without Kerberos? And vice-versa. What would happen in IPA if one or the other (or both) was missing? Should configure require both if either is found?
2. ipavalidate just adds a line at the end? I'm guessing you had your validation stuff in here originally and pulled it out? Would it be too much work to move some of it back in, such as the IP validation and length validation?
3. I like your UI for auto-completion. I may see if I can use that for other interactive work. The reason for validation on the front-end is to error out without having to submit the entire work. We do need additional validation on the receiving end. LDAP does impose some of this though. Most of our values are user-type data though so there isn't much to validate (e.g. phone, street, etc). Not that there isn't a lot of work to do there.
4. As you can see in each tool there is a bunch of duplicated code (all the error catching at the end for example). Do you know of a way we can generalize that in Python and put it into a client library?
5. Can you write some man pages for the tools?Nothing else jumped out at me. If I wanted to test this how might I go about it?
Description: S/MIME Cryptographic Signature