[Freeipa-devel] [PATCH] 983 add subject key identifier

[Martin Kosek]

On Wed, 2012-03-14 at 17:31 -0400, Rob Crittenden wrote:
> Martin Kosek wrote:
> > On Wed, 2012-03-07 at 17:49 -0500, Rob Crittenden wrote:
> >> Add subject key identifier to the dogtag server cert profile.
> >>
> >> This will add it on upgrades too and any new certs issued will have a
> >> subject key identifier set.
> >>
> >> If the user has customized the profile themselves then this won't be
> >> applied.
> >>
> >> rob
> >
> > NACK
> >
> > I found few issues with the patch:
> >
> > 1) There is an extraneous pdb statement:
> > +    import pdb; pdb.set_trace()
> >
> > 2) A name of config file should be put to some variable once and not
> > created every time again in enable_subject_key_identifier. It would be
> > much more readable and less error prone:
> > +            installutils.set_directive('/var/lib/%
> > s/profiles/ca/caIPAserviceCert.cfg' % PKI_INSTANCE_NAME,
> > 'policyset.serverCertSet.list', '1,2,3,4,5,6,7,8,10', quotes=False,
> > separator='=')
> > +            installutils.set_directive('/var/lib/%
> > s/profiles/ca/caIPAserviceCert.cfg' % PKI_INSTANCE_NAME,
> > 'policyset.serverCertSet.10.constraint.class_id', 'noConstraintImpl',
> > quotes=False, separator='=')
> > ...
> >
> > 3) We do not handle gracefully missing config file. This is what happens
> > when replica without CA is upgraded:
> > # rpm -Uvh --force /home/mkosek/dist-review/rpms/freeipa-*
> > Preparing...                ########################################### [100%]
> >     1:freeipa-python         ########################################### [ 17%]
> >     2:freeipa-client         ########################################### [ 33%]
> >     3:freeipa-admintools     ########################################### [ 50%]
> >     4:freeipa-server         ########################################### [ 67%]
> > Upgraded /etc/httpd/conf.d/ipa-pki-proxy.conf to version 1
> > Traceback (most recent call last):
> >    File "/usr/sbin/ipa-upgradeconfig", line 301, in<module>
> >      sys.exit(main())
> >    File "/usr/sbin/ipa-upgradeconfig", line 297, in main
> >      upgrade_ipa_profile(krbctx.default_realm)
> >    File "/usr/sbin/ipa-upgradeconfig", line 243, in upgrade_ipa_profile
> >      if ca.enable_subject_key_identifier():
> >    File "/usr/lib/python2.7/site-packages/ipaserver/install/cainstance.py", line 1079, in enable_subject_key_identifier
> >      setlist = installutils.get_directive('/var/lib/%s/profiles/ca/caIPAserviceCert.cfg' % PKI_INSTANCE_NAME, 'policyset.serverCertSet.list', separator='=')
> >    File "/usr/lib/python2.7/site-packages/ipaserver/install/installutils.py", line 429, in get_directive
> >      fd = open(filename, "r")
> > IOError: [Errno 2] No such file or directory: '/var/lib/pki-ca/profiles/ca/caIPAserviceCert.cfg'
> >     5:freeipa-server-selinux ########################################### [ 83%]
> >     6:freeipa-debuginfo      ########################################### [100%]
> >
> >       1. Martin
> >
> 
> I think this should do it.
> 
> rob

Yup, its much better. ACK. Pushed to master, ipa-2-2.

Martin