[Freeipa-devel] [PATCH] - Add DRM to IPA

Ade Lee alee at redhat.com
Wed Aug 13 18:05:51 UTC 2014


New patch attached which all the issues noted below. Rebased to master.

Please review,
Thanks,

Ade

On Mon, 2014-08-11 at 16:54 +0200, Petr Viktorin wrote:
> On 08/09/2014 01:36 AM, Rob Crittenden wrote:
> > Ade Lee wrote:
> >> Attached is a new patch.  I believe I have addressed all the issues
> >> raided by pviktori, edewata and rcrit.
> Arrrrr!
> >> Please let me know if I missed something!
> >>
> >> Incidentally, to get all this to work, you should use the latest Dogtag
> >> 10.2 build, which also contains a fix for pkidestroy that is not yet
> >> merged in.  A COPR build is currently underway at:
> >>
> >> http://copr.fedoraproject.org/coprs/vakwetu/dogtag/build/24804/
> >
> > Some whitespace issues:
> >
> > Applying: Add a DRM to IPA
> > /home/rcrit/redhat/freeipa/.git/rebase-apply/patch:3774: trailing
> > whitespace.
> >          This relies on the DRM client to generate a wrapping key
> > /home/rcrit/redhat/freeipa/.git/rebase-apply/patch:3292: new blank line
> > at EOF.
> > +
> > warning: 2 lines add whitespace errors.
> > lying: Add a DRM to IPA
> > /home/rcrit/redhat/freeipa/.git/rebase-apply/patch:3774: trailing
> > whitespace.
> >          This relies on the DRM client to generate a wrapping key
> > /home/rcrit/redhat/freeipa/.git/rebase-apply/patch:3292: new blank line
> > at EOF.
> > +
> > warning: 2 lines add whitespace errors.
> >
fixed 

> > I do hope you're planning on adding a minimum build dep at some point?
> >

yes - as soon as we have an official-ish dogtag build.

> > Still seeing AVCs during install:
> >
> > ----
> > time->Fri Aug  8 19:13:35 2014
> > type=SYSCALL msg=audit(1407539615.743:1503): arch=c000003e syscall=1
> > success=no exit=-13 a0=3 a1=210cb30 a2=2d a3=7fff1caa83f0 items=0
> > ppid=12121 pid=12307 auid=4294967295 uid=994 gid=993 euid=994 suid=994
> > fsuid=994 egid=993 sgid=993 fsgid=993 tty=(none) ses=4294967295
> > comm="cp" exe="/usr/bin/cp" subj=system_u:system_r:pki_tomcat_t:s0
> > key=(null)
> > type=AVC msg=audit(1407539615.743:1503): avc:  denied  { setfscreate }
> > for  pid=12307 comm="cp" scontext=system_u:system_r:pki_tomcat_t:s0
> > tcontext=system_u:system_r:pki_tomcat_t:s0 tclass=process
> > ----
> > time->Fri Aug  8 19:13:35 2014
> > type=SYSCALL msg=audit(1407539615.743:1504): arch=c000003e syscall=190
> > success=no exit=-13 a0=4 a1=7fff1caa8590 a2=210c8f0 a3=2d items=0
> > ppid=12121 pid=12307 auid=4294967295 uid=994 gid=993 euid=994 suid=994
> > fsuid=994 egid=993 sgid=993 fsgid=993 tty=(none) ses=4294967295
> > comm="cp" exe="/usr/bin/cp" subj=system_u:system_r:pki_tomcat_t:s0
> > key=(null)
> > type=AVC msg=audit(1407539615.743:1504): avc:  denied  { relabelfrom }
> > for  pid=12307 comm="cp" name="CS.cfg.bak.20140808191335" dev="dm-0"
> > ino=430828 scontext=system_u:system_r:pki_tomcat_t:s0
> > tcontext=system_u:object_r:pki_tomcat_etc_rw_t:s0 tclass=file
> > ----
> > time->Fri Aug  8 19:13:35 2014
> > type=SYSCALL msg=audit(1407539615.744:1505): arch=c000003e syscall=88
> > success=no exit=-13 a0=7fffd3c0daa7 a1=7fffd3c0daea a2=0 a3=7fffd3c0b9b0
> > items=0 ppid=12121 pid=12308 auid=4294967295 uid=994 gid=993 euid=994
> > suid=994 fsuid=994 egid=993 sgid=993 fsgid=993 tty=(none) ses=4294967295
> > comm="ln" exe="/usr/bin/ln" subj=system_u:system_r:pki_tomcat_t:s0
> > key=(null)
> > type=AVC msg=audit(1407539615.744:1505): avc:  denied  { create } for
> > pid=12308 comm="ln" name="CS.cfg.bak"
> > scontext=system_u:system_r:pki_tomcat_t:s0
> > tcontext=system_u:object_r:pki_tomcat_etc_rw_t:s0 tclass=lnk_file
> >

Rob, please open BZ against selinux-policy for these.  Interestingly, 
audit2allow doesn't provide me with any output for these AVC's.

> > The new estimated time was dead on :-)
> >
> > There was a fairly long wait after "Done configuring DRM server
> > (pki-tomcatd)." and the install was done. I thought we always displayed
> > text when restarting (e.g. handled by the service wrapper) but I guess
> > not. It would be nice to know what is going on.
> >
done 

> > Re-running ipa-drm-install results in a scary error:
> >
> > ]# ipa-drm-install
> > Usage: ipa-drm-install [options] [replica_file]
> >
> > ipa-drm-install: error: DRM is already installed.
> >
> > Your system may be partly configured.
> > Run /usr/sbin/ipa_drm_install.py --uninstall to clean up.
> 
> Right, you don't want to override handle_error here. Instead, wrap the 
> body of run() in
> 
>      try:
>          ....
>      except:
>          self.log.error(self.FAIL_MESSAGE)
>          raise
> 
> (Yes, bare `except` and bare `raise`)
> 
> I used self.log.error() instead of print, because I think at least the 
> "Your system may be partly configured." part of the FAIL_MESSAGE should 
> end up in the log, not just on screen.
> 
done

> > And now onto the code...
> >
> > class drm
> >
> > _create_pem_file isnt' exactly descriptive and there is no method
> > documentation.
> >
done

> > _setup. Just a nit: do you want to hardcode the port? I think I'd prefer
> > it come via the constructor and default to 443.
> >

done

> > It may be worth beefing up the return value docs ala what John did in
> > the dogtag section. I notice, for example, you always return a tuple and
> > one value as None in store_secret. I assume there is a reason for that
> > but it isn't obvious. This happens elsewhere too.
> >
done

> > Should the copyright dates on existing files be changed? I don't think
> > they should be, but I'm hardly an expert.
> >
> > I just did a cursory look-see in the code and things generally looked
> > ok. I'm hoping Petr^3 will take a closer look.
> >
> > rob
> >
> 
> I also see a scary error I can't make heads or tails of when trying to 
> install DRM on a replica:
> 
> $ sudo ipa-drm-install
> 
> Your system may be partly configured.
> Run /usr/sbin/ipa_drm_install.py --uninstall to clean up.
> 
> HTTPConnectionPool(host='localhost', port=8080): Max retries exceeded 
> with url: /ca/rest/securityDomain/domainInfo (Caused by <class 
> 'socket.error'>: [Errno 111] Connection refused)

Do you have a replica CA installed?  The replica is trying to reach the
local replica CA install on localhost/ port 8080.

Is port 8080 somehow blocked in your local machine?
Can you get to http://localhost:8080/ca/rest/securityDomain/domainInfo ?


> pviktori at vm-234:~/freeipa{master}e819ca8∗$
> 
> The traceback is:
> 
> ipa.ipaserver.install.ipa_drm_install.DRMInstaller: DEBUG:   File 
> "/usr/lib/python2.7/site-packages/ipapython/admintool.py", line 168, in 
> execute
>      self.validate_options()
>    File 
> "/usr/lib/python2.7/site-packages/ipaserver/install/ipa_drm_install.py", 
> line 180, in validate_options
>      self.installing_replica = dogtaginstance.is_installing_replica("KRA")
>    File 
> "/usr/lib/python2.7/site-packages/ipaserver/install/dogtaginstance.py", 
> line 82, in is_installing_replica
>      info = get_security_domain()
>    File 
> "/usr/lib/python2.7/site-packages/ipaserver/install/dogtaginstance.py", 
> line 71, in get_security_domain
>      info = domain_client.get_security_domain_info()
>    File "/usr/lib/python2.7/site-packages/pki/system.py", line 95, in 
> get_security_domain_info
>      response = self.connection.get('/rest/securityDomain/domainInfo')
>    File "/usr/lib/python2.7/site-packages/pki/client.py", line 60, in get
>      data=payload)
>    File "/usr/lib/python2.7/site-packages/requests/sessions.py", line 
> 347, in get
>      return self.request('GET', url, **kwargs)
>    File "/usr/lib/python2.7/site-packages/requests/sessions.py", line 
> 335, in request
>      resp = self.send(prep, **send_kwargs)
>    File "/usr/lib/python2.7/site-packages/requests/sessions.py", line 
> 438, in send
>      r = adapter.send(request, **kwargs)
>    File "/usr/lib/python2.7/site-packages/requests/adapters.py", line 
> 327, in send
>      raise ConnectionError(e)
> 
> Perhaps I'm doing something wrong here?
> 
> 
> Regarding the code I just have a bunch of nitpicks:
> 
> There are some more logger calls that use `%` for interpolation:
> - ipa-ca-install.install_replica
done

> - ipaserver.install.installutils.check_entropy
> 
done 

> There are some more hardcoded paths. We've just moved these to the 
> platform module recently, and the move wasn't totally thorough, so these 
> are not entirely your fault. Still, they should be fixed:
> ipaserver.install.cainstance.CAInstance.stop_tracking_agent_certificate
> (/etc/httpd/alias)
done
> ipaserver.install.dogtaginstance.HTTPD_CONFD (later when you use it, use 
> os.path.join() instead of adding strings)
done
> ipaserver.install.ipa_replica_prepare.ReplicaPrepare.copy_misc_files 
> (/etc/ipa/default.conf)
done
> ipaserver.install.cainstance.CAInstance.uninstall: 
> "-pki_instance_root=/var/lib" (not touched directly by your change but 
> it would be nice to fix)
done

> ipaserver.install.drminstance.DRMInstance.__spawn_instance: 
> "/var/lib/pki/pki-tomcat/alias/kra_backup_keys.p12", 
> "/root/kracert.p12", "/tmp"
done

> ipa-server-install, summary message: "/root/cacert.p12", "/root/drmcert.p12"
done

> ipaserver.install.dogtaginstance.DogtagInstance.ADMIN_CERT_PATH
done

> ipaserver.install.dogtaginstance.check_inst: 
> '/usr/share/pki/%s/conf/server.xml' (note, constants for template paths 
> have a _TEMPLATE suffix by convention)
done

> ipaserver.install.dogtaginstance.DogtagInstance.spawn_instance: 
> "/usr/sbin/pkispawn"
done
> ipaserver.install.dogtaginstance.DogtagInstance.uninstall: 
> "/usr/sbin/pkidestroy"
done

> ipa_drm_install.DRMInstaller.FAIL_MESSAGE: Just use `ipa-drm-install` as 
> the command; the Python module you mention is not executable.
done

> ipaserver.install.installutils.check_entropy: 
> "/proc/sys/kernel/random/entropy_avail"
> 
done

> CAInstance.__init__, DRMInstance.__init__: These docstrings are unnecessary.

done
> ipa_drm_install.DRMInstall.__init__ just calls the superclass, so it's 
> not necessary to override it.

done

> In ipa_drm_install.DRMInstall.add_options, when adding --uninstall, 
> don't give the empty string (see --no-host-dns).
> 
done
> For ipa_drm_install.DRMInstaller's INSTALLER_START_MESSAGE and 
> FAIL_MESSAGE, you can use textwrap.dedent() for nicer indentation:
> https://docs.python.org/2/library/textwrap.html#textwrap.dedent
> 
done

> In ipa_drm_install.DRMInstaller.__init__, AFAICS setting 
> "installing_replica" and "replica_file" is not needed, and the values 
> are misleading until validate_options sets the the correct values.
> 
done

> In ipa_drm_install.DRMInstaller.validate_options:
> Instead of `ra_plugin is not None and ra_plugin == "dogtag"` you can use 
> just `ra_plugin == "dogtag"`.
> Same with `if dogtag_version is not None and dogtag_version >= 10:` (you 
> explicitly convert the value to int earlier, so it can't be None here).
> Instead of `if len(self.args) < 1:` just use `if not self.args:`.

All done

> The tool should also error out if more than one replica file is given, 
> or if file(s) are given for non-replica master or for uninstalling.
> 
done.

> The commit message is slightly wrong; ipa-drm-install does not ask for a 
> replica file when it's needed and not given.

> And, please link to the ticket and design page in the commit message.
> 
done

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-2-Add-a-DRM-to-IPA.patch
Type: text/x-patch
Size: 159241 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140813/f333de77/attachment.bin>


More information about the Freeipa-devel mailing list