[Pki-devel] [PATCH] 278 - handle external certs
Ade Lee
alee at redhat.com
Thu Mar 3 20:42:40 UTC 2016
Acked by Jack. Pushed to master.
On Tue, 2016-03-01 at 22:53 -0500, Ade Lee wrote:
> Found an error .. lets try again ..
>
> Ade
> On Tue, 2016-03-01 at 17:20 -0500, Ade Lee wrote:
> > Based on feedback, revamped the internals to make it look much
> > nicer.
> >
> > Ade
> > On Tue, 2016-03-01 at 13:19 -0500, Ade Lee wrote:
> > > On Mon, 2016-02-29 at 20:36 -0500, John Magne wrote:
> > > > Review Notes:
> > > >
> > > > Looks like good stuff.
> > > >
> > > > Just a couple of small picky questions.
> > > >
> > > > We can delay the final ACK to after we either decide that the
> > > > questions
> > > > are not that important or we decide to make any minor changes.
> > > >
> > > >
> > > > thanks,
> > > > jack
> > > >
> > > >
> > > > This method:
> > > >
> > > > in __init__.py :
> > > >
> > > >
> > > > + def load_external_certs(self, conf_file):
> > > > + # load external certs data
> > > > + if os.path.exists(conf_file):
> > > > + lines = open(conf_file).read().splitlines()
> > > > + for line in lines:
> > > > + parts = line.split('=', 1)
> > > > + name = parts[0]
> > > > + value = parts[1]
> > > > + self.external_certs[name] = value
> > > > + else:
> > > > + self.external_certs['num_certs'] = 0
> > > > +
> > > >
> > > > I see if there is no file, the value of num_certs is 0.
> > > >
> > > > What about in the loop that reads the file when it does exist?
> > > > What
> > > > if it's empty?
> > > > Also when doing the splitting for the values, do we need some
> > > > simple
> > > > sanity checking?
> > > >
> > > How about this --
> > >
> > > + def load_external_certs(self, conf_file):
> > > + # load external certs data
> > > + if os.path.exists(conf_file) and
> > > os.stat(conf_file).st_size
> > > > 0:
> > > + lines = open(conf_file).read().splitlines()
> > > + for line in lines:
> > > + parts = line.split('=', 1)
> > > if len(parts) != 2:
> > > // throw some exception
> > > + name = parts[0]
> > > + value = parts[1]
> > > + self.external_certs[name] = value
> > > + else:
> > > + self.external_certs['num_certs'] = 0
> > >
> > > + if 'num_certs' not in self.external_certs:
> > > // throw some exception
> > >
> > > > Same Q's for method: def update_external_cert_conf(self,
> > > > external_path, deployer):
> > > >
> > > >
> > > > def delete_external_cert(self, nickname, token):
> > > > + match_found = False
> > > > + num_certs = int(self.external_certs['num_certs'])
> > > > + if num_certs > 0:
> > > > + for num in range(0, num_certs):
> > > > + current_nick = self.external_certs[str(num) +
> > > > ".nickname"]
> > > > + current_token = self.external_certs[str(num) +
> > > > ".token"]
> > > > + if current_nick == nickname and current_token
> > > > ==
> > > > token:
> > > > + del self.external_certs[str(num) +
> > > > ".nickname"]
> > > > + del self.external_certs[str(num) +
> > > > ".token"]
> > > > + match_found = True
> > > > + continue
> > > > + if match_found:
> > > > + self.external_certs[str(num - 1) +
> > > > ".nickname"] = current_nick
> > > > + self.external_certs[str(num - 1) +
> > > > ".token"]
> > > > =
> > > > current_token
> > > > +
> > > > + self.external_certs['num_certs'] = num_certs - 1
> > > > + self.save_external_cert_data()
> > > >
> > > > I may have this wrong, but if you find a match and then
> > > > continue?
> > > > Will the "match_found if, ever get executed?
> > > >
> > > It will get executed on the next -- and every subsequent value.
> > > The purpose is to reorder subsequent values.
> > >
> > > So lets say we have values 0,1,2,3,4, and we have a match on 1.
> > > Then
> > > we will remove 1 and set 2-> 1, 3-> 2, 4->3. So the result will
> > > be
> > > 0,1,2,3
> > >
> > > > Also you could if you wanted pre-calculate those hash indexes
> > > > into
> > > > a
> > > > var and use them.
> > > >
> > > yeah - but it doesn't really help as much -- see comment above.
> > > >
> > > > Question: Here:
> > > >
> > > > nicks = self.import_certs(
> > > > + instance, cert_file, nickname, token, trust_args)
> > > > + self.update_instance_config(instance, nicks, token)
> > > > +
> > > > + self.print_message('Certificate imported for instance
> > > > %s.'
> > > > %
> > > > + instance_name)
> > > >
> > > > Question is is there somewhere in that call chain that throws
> > > > an
> > > > exception if something goes wrong?
> > >
> > > yes. I'm tested a number of cases where that does happen. The
> > > exception comes all the way from nss out.
> > >
> > > > Just checking to see if those late to lines get called (along
> > > > with
> > > > the print, even if there is an error importing certs) ??
> > > >
> > > No they don't. The exceptions cause the code to break out.
> > > >
> > > > Method:
> > > >
> > > > class InstanceExternalCertDeleteCLI(pki.cli.CLI):
> > > >
> > > > Would there be any value in allowing multiple nicknames to be
> > > > deleted?
> > > >
> > > perhaps - we can add as a refinement later if needed.
> > >
> > > > Also same at above here:
> > > >
> > > > instance = pki.server.PKIInstance(instance_name)
> > > > + instance.load()
> > > > +
> > > > + self.remove_cert(instance, nickname, token)
> > > > + instance.delete_external_cert(nickname, token)
> > > > +
> > > > + self.print_message('Certificate removed from instance
> > > > %s.'
> > > > %
> > > > + instance_name)
> > > >
> > > >
> > > > Do we get to the end if instance.load does not work?
> > > >
> > > No we don't. Exceptions propagate up.
> > >
> > > >
> > > >
> > > >
> > > >
> > > >
> > > > ----- Original Message -----
> > > > From: "Ade Lee" <alee at redhat.com>
> > > > To: pki-devel at redhat.com
> > > > Sent: Monday, February 29, 2016 9:25:30 AM
> > > > Subject: [Pki-devel] [PATCH] 278 - handle external certs
> > > >
> > > > This is to resolve ticket 1742.
> > > >
> > > > For this ticket, we need a mechanism to import third party
> > > > certs
> > > > to
> > > > clones. This patch provides a general mechanism to do this.
> > > >
> > > > A follow-on patch with documentation on how this all works is
> > > > forthcoming.
> > > >
> > > > Ade
> > > > _______________________________________________
> _______________________________________________
> Pki-devel mailing list
> Pki-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/pki-devel
More information about the Pki-devel
mailing list