Merge branch 'geni-v3' into pep8
authorThierry Parmentelat <thierry.parmentelat@inria.fr>
Fri, 13 Jan 2017 12:14:17 +0000 (13:14 +0100)
committerThierry Parmentelat <thierry.parmentelat@inria.fr>
Fri, 13 Jan 2017 12:14:17 +0000 (13:14 +0100)
* geni-v3:
  more, and more legible, debug messages in the cert verification area
  sfax509 will run openssl x509 on all parts of a gid
  ignore html and pdf files when doing stuff like make tags
  bugfix in sfi when running the discover subcommand

# Conflicts:
# sfa/trust/certificate.py
# sfa/trust/gid.py

1  2 
clientbin/sfadump.py
sfa/client/sfi.py
sfa/trust/certificate.py
sfa/trust/credential.py
sfa/trust/gid.py

diff --combined clientbin/sfadump.py
@@@ -1,15 -1,12 +1,14 @@@
  #! /usr/bin/env python
- from __future__ import with_statement
  
  import sys
 -import os, os.path
 +import os
 +import os.path
  import tempfile
  from argparse import ArgumentParser
  
  from sfa.util.sfalogging import logger
 -from sfa.util.faults import CredentialNotVerifiable, CertMissingParent #, ChildRightsNotSubsetOfParent
 +# , ChildRightsNotSubsetOfParent
 +from sfa.util.faults import CredentialNotVerifiable, CertMissingParent
  
  from sfa.trust.certificate import Certificate
  from sfa.trust.credential import Credential
@@@ -17,34 -14,26 +16,34 @@@ from sfa.trust.gid import GI
  
  from sfa.storage.record import Record
  
 +
  def determine_sfa_filekind(fn):
  
 -    if fn.endswith('.gid'): return 'gid'
 -    elif fn.endswith('.cert'): return 'certificate'
 -    elif fn.endswith('cred'): return 'credential'
 +    if fn.endswith('.gid'):
 +        return 'gid'
 +    elif fn.endswith('.cert'):
 +        return 'certificate'
 +    elif fn.endswith('cred'):
 +        return 'credential'
  
      try:
 -        cred=Credential(filename=fn)
 +        cred = Credential(filename=fn)
          return 'credential'
 -    except: pass
 +    except:
 +        pass
  
 -    try: 
 -        gid=GID(filename=fn)
 -        if gid.uuid: return 'gid'
 -    except: pass
 +    try:
 +        gid = GID(filename=fn)
 +        if gid.uuid:
 +            return 'gid'
 +    except:
 +        pass
  
      try:
 -        cert = Certificate(filename = fn)
 +        cert = Certificate(filename=fn)
          return 'certificate'
 -    except: pass
 +    except:
 +        pass
  
      # to be completed
  #    if "gidCaller" in dict:
  
      return "unknown"
  
 +
  def save_gid(gid):
 -   hrn = gid.get_hrn()
 -   lastpart = hrn.split(".")[-1]
 -   filename = lastpart + ".gid"
 +    hrn = gid.get_hrn()
 +    lastpart = hrn.split(".")[-1]
 +    filename = lastpart + ".gid"
  
 -   if os.path.exists(filename):
 -       print filename, ": already exists... skipping"
 -       return
 +    if os.path.exists(filename):
 +        print filename, ": already exists... skipping"
 +        return
  
 -   print filename, ": extracting gid of", hrn
 +    print filename, ": extracting gid of", hrn
 +
 +    gid.save_to_file(filename, save_parents=True)
  
 -   gid.save_to_file(filename, save_parents = True)
  
  def extract_gids(cred, extract_parents):
 -   gidCaller = cred.get_gid_caller()
 -   if gidCaller:
 -       save_gid(gidCaller)
 +    gidCaller = cred.get_gid_caller()
 +    if gidCaller:
 +        save_gid(gidCaller)
  
 -   gidObject = cred.get_gid_object()
 -   if gidObject and ((gidCaller == None) or (gidCaller.get_hrn() != gidObject.get_hrn())):
 -       save_gid(gidObject)
 +    gidObject = cred.get_gid_object()
 +    if gidObject and ((gidCaller == None) or (gidCaller.get_hrn() != gidObject.get_hrn())):
 +        save_gid(gidObject)
  
 -   # no such method Credential.get_parent
 +    # no such method Credential.get_parent
  #   if extract_parents:
  #       parent = cred.get_parent()
  #       if parent:
  #           extract_gids(parent, extract_parents)
  
 -def verify_input_object (obj, kind, options):
 +
 +def verify_input_object(obj, kind, options):
      if options.trusted_roots:
          print "CHECKING...",
 -        message= "against [" + (" + ".join(options.trusted_roots)) + "]"
 +        message = "against [" + (" + ".join(options.trusted_roots)) + "]"
          try:
 -            if kind=='credential':
 -                print "verify",message,
 +            if kind == 'credential':
 +                print "verify", message,
                  obj.verify(options.trusted_roots)
 -            elif kind in ['certificate','gid']:
 -                print "verify_chain",message,
 +            elif kind in ('certificate', 'gid'):
 +                print "verify_chain", message,
                  obj.verify_chain(options.trusted_roots)
              print "--> OK"
          except Exception as inst:
 -            print "--> KO",type(inst).__name__
 +            print "--> KO", type(inst).__name__
  
 -def handle_input (filename, options):
 +
 +def handle_input(filename, options):
      kind = determine_sfa_filekind(filename)
  
      # dump methods current do 'print' so let's go this road for now
 -    if kind=="certificate":
 -        cert=Certificate (filename=filename)
 -        print '--------------------',filename,'IS A',kind
 +    if kind == "certificate":
 +        cert = Certificate(filename=filename)
 +        print '--------------------', filename, 'IS A', kind
          cert.dump(show_extensions=options.show_extensions)
 -        verify_input_object (cert, kind, options)
 -    elif kind=="credential":
 -        cred = Credential(filename = filename)
 -        print '--------------------',filename,'IS A',kind
 -        cred.dump(dump_parents = options.dump_parents, show_xml=options.show_xml)
 +        verify_input_object(cert, kind, options)
 +    elif kind == "credential":
 +        cred = Credential(filename=filename)
 +        print '--------------------', filename, 'IS A', kind
 +        cred.dump(dump_parents=options.dump_parents, show_xml=options.show_xml)
          if options.extract_gids:
 -            print '--------------------',filename,'embedded GIDs'
 -            extract_gids(cred, extract_parents = options.dump_parents)
 -        verify_input_object (cred, kind, options)
 -    elif kind=="gid":
 -        gid = GID(filename = filename)
 -        print '--------------------',filename,'IS A',kind
 -        gid.dump(dump_parents = options.dump_parents)
 -        verify_input_object (gid, kind, options)
 +            print '--------------------', filename, 'embedded GIDs'
 +            extract_gids(cred, extract_parents=options.dump_parents)
 +        verify_input_object(cred, kind, options)
 +    elif kind == "gid":
 +        gid = GID(filename=filename)
 +        print '--------------------', filename, 'IS A', kind
 +        gid.dump(dump_parents=options.dump_parents)
 +        verify_input_object(gid, kind, options)
      else:
 -        print "%s: unknown filekind '%s'"% (filename,kind)
 +        print "%s: unknown filekind '%s'" % (filename, kind)
 +
  
  def main():
      usage = """%(prog)s file1 [ .. filen]
  display info on input files"""
      parser = ArgumentParser(usage=usage)
  
 -    parser.add_argument("-g", "--extract-gids", action="store_true", dest="extract_gids", 
 +    parser.add_argument("-g", "--extract-gids", action="store_true", dest="extract_gids",
                          default=False, help="Extract GIDs from credentials")
 -    parser.add_argument("-p", "--dump-parents", action="store_true", dest="dump_parents", 
 +    parser.add_argument("-p", "--dump-parents", action="store_true", dest="dump_parents",
                          default=False, help="Show parents")
 -    parser.add_argument("-e", "--extensions", action="store_true", 
 -                        dest="show_extensions", default="False", help="Show certificate extensions")
 -    parser.add_argument("-v", "--verbose", action='count', 
 +    parser.add_argument("-e", "--extensions", action="store_true",
 +                        dest="show_extensions", default="False",
 +                        help="Show certificate extensions")
 +    parser.add_argument("-v", "--verbose", action='count',
                          dest='verbose', default=0, help="More and more verbose")
 -    parser.add_argument("-x", "--xml", action='store_true', 
 +    parser.add_argument("-x", "--xml", action='store_true',
                          dest='show_xml', default=False, help="dumps xml tree (cred. only)")
      parser.add_argument("-c", "--check", action='append', dest='trusted_roots',
 -                        help="cumulative list of trusted GIDs - when provided, the input is verify'ed against these")
 -    parser.add_argument("filenames",metavar='F',nargs='+',help="filenames to dump")
 +                        help="cumulative list of trusted GIDs - "
 +                        "when provided, the input is verify'ed against these")
 +    parser.add_argument("filenames", metavar='F', nargs='+',
 +                        help="filenames to dump")
      options = parser.parse_args()
  
      logger.setLevelFromOptVerbose(options.verbose)
 -    for filename in options.filenames: 
 -        handle_input(filename,options)
 +    for filename in options.filenames:
 +        handle_input(filename, options)
  
 -if __name__=="__main__":
 -   main()
 +if __name__ == "__main__":
 +    main()
diff --combined sfa/client/sfi.py
@@@ -8,8 -8,7 +8,8 @@@ from __future__ import print_functio
  import sys
  sys.path.append('.')
  
 -import os, os.path
 +import os
 +import os.path
  import socket
  import re
  import datetime
@@@ -53,11 -52,9 +53,11 @@@ CM_PORT = 1234
  DEFAULT_RSPEC_VERSION = "GENI 3"
  
  from sfa.client.common import optparse_listvalue_callback, optparse_dictvalue_callback, \
 -    terminal_render, filter_records 
 +    terminal_render, filter_records
  
  # display methods
 +
 +
  def display_rspec(rspec, format='rspec'):
      if format in ['dns']:
          tree = etree.parse(StringIO(rspec))
      print(result)
      return
  
 +
  def display_list(results):
      for result in results:
          print(result)
  
 +
  def display_records(recordList, dump=False):
      ''' Print all fields in the record'''
      for record in recordList:
          display_record(record, dump)
  
 +
  def display_record(record, dump=False):
      if dump:
          record.dump(sort=True)
@@@ -115,18 -109,14 +115,18 @@@ def credential_printable(cred)
      result += "rights={}\n".format(rights)
      return result
  
 +
  def show_credentials(cred_s):
 -    if not isinstance(cred_s, list): cred_s = [cred_s]
 +    if not isinstance(cred_s, list):
 +        cred_s = [cred_s]
      for cred in cred_s:
          print("Using Credential {}".format(credential_printable(cred)))
  
 -########## save methods
 +# save methods
 +
 +# raw
 +
  
 -### raw
  def save_raw_to_file(var, filename, format='text', banner=None):
      if filename == '-':
          _save_raw_to_file(var, sys.stdout, format, banner)
              _save_raw_to_file(var, fileobj, format, banner)
          print("(Over)wrote {}".format(filename))
  
 +
  def _save_raw_to_file(var, f, format, banner):
      if format == "text":
 -        if banner: f.write(banner+"\n")
 +        if banner:
 +            f.write(banner + "\n")
          f.write("{}".format(var))
 -        if banner: f.write('\n'+banner+"\n")
 +        if banner:
 +            f.write('\n' + banner + "\n")
      elif format == "pickled":
          f.write(pickle.dumps(var))
      elif format == "json":
          # this should never happen
          print("unknown output format", format)
  
 -### 
 +###
 +
 +
  def save_rspec_to_file(rspec, filename):
      if not filename.endswith(".rspec"):
          filename = filename + ".rspec"
          f.write("{}".format(rspec))
      print("(Over)wrote {}".format(filename))
  
 +
  def save_record_to_file(filename, record_dict):
      record = Record(dict=record_dict)
      xml = record.save_as_xml()
          f.write(xml)
      print("(Over)wrote {}".format(filename))
  
 +
  def save_records_to_file(filename, record_dicts, format="xml"):
      if format == "xml":
          for index, record_dict in enumerate(record_dicts):
              f.write("<recordlist>\n")
              for record_dict in record_dicts:
                  record_obj = Record(dict=record_dict)
 -                f.write('<record hrn="' + record_obj.hrn + '" type="' + record_obj.type + '" />\n')
 +                f.write('<record hrn="' + record_obj.hrn +
 +                        '" type="' + record_obj.type + '" />\n')
              f.write("</recordlist>\n")
              print("(Over)wrote {}".format(filename))
  
          print("unknown output format", format)
  
  # minimally check a key argument
 +
 +
  def check_ssh_key(key):
      good_ssh_key = r'^.*(?:ssh-dss|ssh-rsa)[ ]+[A-Za-z0-9+/=]+(?: .*)?$'
      return re.match(good_ssh_key, key, re.IGNORECASE)
  
  # load methods
 +
 +
  def normalize_type(type):
      if type.startswith('au'):
          return 'authority'
          print('unknown type {} - should start with one of au|us|sl|no|ag|al'.format(type))
          return None
  
 +
  def load_record_from_opts(options):
      record_dict = {}
      if hasattr(options, 'xrn') and options.xrn:
          except IOError:
              pubkey = options.key
          if not check_ssh_key(pubkey):
 -            raise SfaInvalidArgument(name='key', msg="Could not find file, or wrong key format")
 +            raise SfaInvalidArgument(
 +                name='key', msg="Could not find file, or wrong key format")
          record_dict['reg-keys'] = [pubkey]
      if hasattr(options, 'slices') and options.slices:
          record_dict['slices'] = options.slices
  
      # handle extra settings
      record_dict.update(options.extras)
 -    
 +
      return Record(dict=record_dict)
  
 +
  def load_record_from_file(filename):
      with codecs.open(filename, encoding="utf-8", mode="r") as f:
          xml_str = f.read()
      return Record(xml=xml_str)
  
  import uuid
 +
 +
  def unique_call_id(): return uuid.uuid4().urn
  
 -########## a simple model for maintaing 3 doc attributes per command (instead of just one)
 +# a simple model for maintaing 3 doc attributes per command (instead of just one)
  # essentially for the methods that implement a subcommand like sfi list
  # we need to keep track of
  # (*) doc         a few lines that tell what it does, still located in __doc__
  # (*) args_string a simple one-liner that describes mandatory arguments
  # (*) example     well, one or several releant examples
 -# 
 +#
  # since __doc__ only accounts for one, we use this simple mechanism below
  # however we keep doc in place for easier migration
  
  from functools import wraps
  
  # we use a list as well as a dict so we can keep track of the order
 -commands_list=[]
 -commands_dict={}
 +commands_list = []
 +commands_dict = {}
 +
  
  def declare_command(args_string, example, aliases=None):
 -    def wrap(m): 
 -        name=getattr(m, '__name__')
 -        doc=getattr(m, '__doc__', "-- missing doc --")
 -        doc=doc.strip(" \t\n")
 +    def wrap(m):
 +        name = getattr(m, '__name__')
 +        doc = getattr(m, '__doc__', "-- missing doc --")
 +        doc = doc.strip(" \t\n")
          commands_list.append(name)
 -        # last item is 'canonical' name, so we can know which commands are aliases
 -        command_tuple=(doc, args_string, example, name)
 -        commands_dict[name]=command_tuple
 +        # last item is 'canonical' name, so we can know which commands are
 +        # aliases
 +        command_tuple = (doc, args_string, example, name)
 +        commands_dict[name] = command_tuple
          if aliases is not None:
              for alias in aliases:
                  commands_list.append(alias)
 -                commands_dict[alias]=command_tuple
 +                commands_dict[alias] = command_tuple
 +
          @wraps(m)
          def new_method(*args, **kwds): return m(*args, **kwds)
          return new_method
  
  
  def remove_none_fields(record):
 -    none_fields=[ k for (k, v) in record.items() if v is None ]
 -    for k in none_fields: del record[k]
 +    none_fields = [k for (k, v) in record.items() if v is None]
 +    for k in none_fields:
 +        del record[k]
  
  ##########
  
 +
  class Sfi:
 -    
 +
      # dirty hack to make this class usable from the outside
 -    required_options=['verbose',  'debug',  'registry',  'sm',  'auth',  'user', 'user_private_key']
 +    required_options = ['verbose',  'debug',  'registry',
 +                        'sm',  'auth',  'user', 'user_private_key']
  
      @staticmethod
      def default_sfi_dir():
 -        if os.path.isfile("./sfi_config"): 
 +        if os.path.isfile("./sfi_config"):
              return os.getcwd()
          else:
              return os.path.expanduser("~/.sfi/")
          pass
  
      def __init__(self, options=None):
 -        if options is None: options=Sfi.DummyOptions()
 +        if options is None:
 +            options = Sfi.DummyOptions()
          for opt in Sfi.required_options:
              if not hasattr(options, opt):
                  setattr(options, opt, None)
          self.authority = None
          self.logger = sfi_logger
          self.logger.enable_console()
 -        ### various auxiliary material that we keep at hand 
 -        self.command=None
 -        # need to call this other than just 'config' as we have a command/method with that name
 +        # various auxiliary material that we keep at hand
 +        self.command = None
 +        # need to call this other than just 'config' as we have a
 +        # command/method with that name
          self.config_instance = None
          self.config_file = None
          self.client_bootstrap = None
  
 -    ### suitable if no reasonable command has been provided
 +    # suitable if no reasonable command has been provided
      def print_commands_help(self, options):
          verbose = getattr(options, 'verbose')
          format3 = "%10s %-35s %s"
          format3offset = 47
 -        line = 80*'-'
 +        line = 80 * '-'
          if not verbose:
 -            print(format3%("command", "cmd_args", "description"))
 +            print(format3 % ("command", "cmd_args", "description"))
              print(line)
          else:
              print(line)
              try:
                  (doc, args_string, example, canonical) = commands_dict[command]
              except:
 -                print("Cannot find info on command %s - skipped"%command)
 +                print("Cannot find info on command %s - skipped" % command)
                  continue
              if verbose:
                  print(line)
 -            if command==canonical:
 +            if command == canonical:
                  doc = doc.replace("\n", "\n" + format3offset * ' ')
                  print(format3 % (command, args_string, doc))
                  if verbose:
                      self.create_parser_command(command).print_help()
              else:
 -                print(format3 % (command, "<<alias for %s>>"%canonical, ""))
 -            
 -    ### now if a known command was found we can be more verbose on that one
 +                print(format3 % (command, "<<alias for %s>>" % canonical, ""))
 +
 +    # now if a known command was found we can be more verbose on that one
      def print_help(self):
          print("==================== Generic sfi usage")
          self.sfi_parser.print_help()
                                usage="sfi [sfi_options] command [cmd_options] [cmd_args]",
                                description="Commands: {}".format(" ".join(commands_list)))
          parser.add_option("-r", "--registry", dest="registry",
 -                         help="root registry", metavar="URL", default=None)
 +                          help="root registry", metavar="URL", default=None)
          parser.add_option("-s", "--sliceapi", dest="sm", default=None, metavar="URL",
 -                         help="slice API - in general a SM URL, but can be used to talk to an aggregate")
 +                          help="slice API - in general a SM URL, but can be used to talk to an aggregate")
          parser.add_option("-R", "--raw", dest="raw", default=None,
                            help="Save raw, unparsed server response to a file")
          parser.add_option("", "--rawformat", dest="rawformat", type="choice",
                            help="raw file format ([text]|pickled|json)", default="text",
 -                          choices=("text","pickled","json"))
 +                          choices=("text", "pickled", "json"))
          parser.add_option("", "--rawbanner", dest="rawbanner", default=None,
                            help="text string to write before and after raw output")
          parser.add_option("-d", "--dir", dest="sfi_dir",
 -                         help="config & working directory - default is %default",
 -                         metavar="PATH", default=Sfi.default_sfi_dir())
 +                          help="config & working directory - default is %default",
 +                          metavar="PATH", default=Sfi.default_sfi_dir())
          parser.add_option("-u", "--user", dest="user",
 -                         help="user name", metavar="HRN", default=None)
 +                          help="user name", metavar="HRN", default=None)
          parser.add_option("-a", "--auth", dest="auth",
 -                         help="authority name", metavar="HRN", default=None)
 +                          help="authority name", metavar="HRN", default=None)
          parser.add_option("-v", "--verbose", action="count", dest="verbose", default=0,
 -                         help="verbose mode - cumulative")
 +                          help="verbose mode - cumulative")
          parser.add_option("-D", "--debug",
                            action="store_true", dest="debug", default=False,
                            help="Debug (xml-rpc) protocol messages")
          # would it make sense to use ~/.ssh/id_rsa as a default here ?
          parser.add_option("-k", "--private-key",
 -                         action="store", dest="user_private_key", default=None,
 -                         help="point to the private key file to use if not yet installed in sfi_dir")
 +                          action="store", dest="user_private_key", default=None,
 +                          help="point to the private key file to use if not yet installed in sfi_dir")
          parser.add_option("-t", "--timeout", dest="timeout", default=None,
 -                         help="Amout of time to wait before timing out the request")
 -        parser.add_option("-h", "--help", 
 -                         action="store_true", dest="help", default=False,
 -                         help="one page summary on commands & exit")
 +                          help="Amout of time to wait before timing out the request")
 +        parser.add_option("-h", "--help",
 +                          action="store_true", dest="help", default=False,
 +                          help="one page summary on commands & exit")
          parser.disable_interspersed_args()
  
          return parser
 -        
  
      def create_parser_command(self, command):
          if command not in commands_dict:
 -            msg="Invalid command\n"
 -            msg+="Commands: "
 -            msg += ','.join(commands_list)            
 +            msg = "Invalid command\n"
 +            msg += "Commands: "
 +            msg += ','.join(commands_list)
              self.logger.critical(msg)
              sys.exit(2)
  
          (_, args_string, __, canonical) = commands_dict[command]
  
          parser = OptionParser(add_help_option=False,
 -                              usage="sfi [sfi_options] {} [cmd_options] {}"\
 +                              usage="sfi [sfi_options] {} [cmd_options] {}"
                                .format(command, args_string))
 -        parser.add_option("-h","--help",dest='help',action='store_true',default=False,
 -                           help="Summary of one command usage")
 +        parser.add_option("-h", "--help", dest='help', action='store_true', default=False,
 +                          help="Summary of one command usage")
  
          if canonical in ("config"):
              parser.add_option('-m', '--myslice', dest='myslice', action='store_true', default=False,
                                help='how myslice config variables as well')
  
          if canonical in ("version"):
 -            parser.add_option("-l","--local",
 +            parser.add_option("-l", "--local",
                                action="store_true", dest="version_local", default=False,
                                help="display version of the local client")
  
          if canonical in ("version", "trusted", "introspect"):
 -            parser.add_option("-R","--registry_interface",
 +            parser.add_option("-R", "--registry_interface",
                                action="store_true", dest="registry_interface", default=False,
                                help="target the registry interface instead of slice interface")
  
          if canonical in ("register", "update"):
 -            parser.add_option('-x', '--xrn', dest='xrn', metavar='<xrn>', help='object hrn/urn (mandatory)')
 -            parser.add_option('-t', '--type', dest='type', metavar='<type>', help='object type (2 first chars is enough)', default=None)
 -            parser.add_option('-e', '--email', dest='email', default="",  help="email (mandatory for users)") 
 -            parser.add_option('-n', '--name', dest='name', default="",  help="name (optional for authorities)") 
 -            parser.add_option('-k', '--key', dest='key', metavar='<key>', help='public key string or file', 
 +            parser.add_option('-x', '--xrn', dest='xrn',
 +                              metavar='<xrn>', help='object hrn/urn (mandatory)')
 +            parser.add_option('-t', '--type', dest='type', metavar='<type>',
 +                              help='object type (2 first chars is enough)', default=None)
 +            parser.add_option('-e', '--email', dest='email',
 +                              default="",  help="email (mandatory for users)")
 +            parser.add_option('-n', '--name', dest='name',
 +                              default="",  help="name (optional for authorities)")
 +            parser.add_option('-k', '--key', dest='key', metavar='<key>', help='public key string or file',
                                default=None)
              parser.add_option('-s', '--slices', dest='slices', metavar='<slices>', help='Set/replace slice xrns',
                                default='', type="str", action='callback', callback=optparse_listvalue_callback)
 -            parser.add_option('-r', '--researchers', dest='reg_researchers', metavar='<researchers>', 
 -                              help='Set/replace slice researchers - use -r none to reset', default=None, type="str", action='callback', 
 +            parser.add_option('-r', '--researchers', dest='reg_researchers', metavar='<researchers>',
 +                              help='Set/replace slice researchers - use -r none to reset', default=None, type="str", action='callback',
                                callback=optparse_listvalue_callback)
              parser.add_option('-p', '--pis', dest='reg_pis', metavar='<PIs>', help='Set/replace Principal Investigators/Project Managers',
                                default='', type="str", action='callback', callback=optparse_listvalue_callback)
 -            parser.add_option('-X','--extra',dest='extras',default={},type='str',metavar="<EXTRA_ASSIGNS>",
 -                               action="callback", callback=optparse_dictvalue_callback, nargs=1,
 -                               help="set extra/testbed-dependent flags, e.g. --extra enabled=true")
 -
 -        # user specifies remote aggregate/sm/component                          
 -        if canonical in ("resources", "describe", "allocate", "provision", "delete", "allocate", "provision", 
 -                       "action", "shutdown", "renew", "status"):
 -            parser.add_option("-d", "--delegate", dest="delegate", default=None, 
 -                             action="store_true",
 -                             help="Include a credential delegated to the user's root"+\
 -                                  "authority in set of credentials for this call")
 +            parser.add_option('-X', '--extra', dest='extras', default={}, type='str', metavar="<EXTRA_ASSIGNS>",
 +                              action="callback", callback=optparse_dictvalue_callback, nargs=1,
 +                              help="set extra/testbed-dependent flags, e.g. --extra enabled=true")
 +
 +        # user specifies remote aggregate/sm/component
 +        if canonical in ("resources", "describe", "allocate", "provision", "delete", "allocate", "provision",
 +                         "action", "shutdown", "renew", "status"):
 +            parser.add_option("-d", "--delegate", dest="delegate", default=None,
 +                              action="store_true",
 +                              help="Include a credential delegated to the user's root" +
 +                              "authority in set of credentials for this call")
  
          # show_credential option
 -        if canonical in ("list","resources", "describe", "provision", "allocate", "register","update","remove","delete","status","renew"):
 -            parser.add_option("-C","--credential",dest='show_credential',action='store_true',default=False,
 +        if canonical in ("list", "resources", "describe", "provision", "allocate", "register",
 +                         "update", "remove", "delete", "status", "renew"):
 +            parser.add_option("-C", "--credential", dest='show_credential', action='store_true', default=False,
                                help="show credential(s) used in human-readable form")
          if canonical in ("renew"):
 -            parser.add_option("-l","--as-long-as-possible",dest='alap',action='store_true',default=False,
 +            parser.add_option("-l", "--as-long-as-possible", dest='alap', action='store_true', default=False,
                                help="renew as long as possible")
          # registy filter option
          if canonical in ("list", "show", "remove"):
                                default="all",
                                help="type filter - 2 first chars is enough ([all]|user|slice|authority|node|aggregate)")
          if canonical in ("show"):
 -            parser.add_option("-k","--key",dest="keys",action="append",default=[],
 +            parser.add_option("-k", "--key", dest="keys", action="append", default=[],
                                help="specify specific keys to be displayed from record")
 -            parser.add_option("-n","--no-details",dest="no_details",action="store_true",default=False,
 +            parser.add_option("-n", "--no-details", dest="no_details", action="store_true", default=False,
                                help="call Resolve without the 'details' option")
          if canonical in ("resources", "describe"):
              # rspec version
                                help="schema type and version of resulting RSpec (default:{})".format(DEFAULT_RSPEC_VERSION))
              # disable/enable cached rspecs
              parser.add_option("-c", "--current", dest="current", default=False,
 -                              action="store_true",  
 +                              action="store_true",
                                help="Request the current rspec bypassing the cache. Cached rspecs are returned by default")
              # display formats
              parser.add_option("-f", "--format", dest="format", type="choice",
 -                             help="display format ([xml]|dns|ip)", default="xml",
 -                             choices=("xml", "dns", "ip"))
 -            #panos: a new option to define the type of information about resources a user is interested in
 +                              help="display format ([xml]|dns|ip)", default="xml",
 +                              choices=("xml", "dns", "ip"))
 +            # panos: a new option to define the type of information about
 +            # resources a user is interested in
              parser.add_option("-i", "--info", dest="info",
 -                                help="optional component information", default=None)
 -            # a new option to retrieve or not reservation-oriented RSpecs (leases)
 +                              help="optional component information", default=None)
 +            # a new option to retrieve or not reservation-oriented RSpecs
 +            # (leases)
              parser.add_option("-l", "--list_leases", dest="list_leases", type="choice",
 -                                help="Retrieve or not reservation-oriented RSpecs ([resources]|leases|all)",
 -                                choices=("all", "resources", "leases"), default="resources")
 -
 +                              help="Retrieve or not reservation-oriented RSpecs ([resources]|leases|all)",
 +                              choices=("all", "resources", "leases"), default="resources")
  
          if canonical in ("resources", "describe", "allocate", "provision", "show", "list", "gid"):
 -           parser.add_option("-o", "--output", dest="file",
 -                            help="output XML to file", metavar="FILE", default=None)
 +            parser.add_option("-o", "--output", dest="file",
 +                              help="output XML to file", metavar="FILE", default=None)
  
          if canonical in ("show", "list"):
 -           parser.add_option("-f", "--format", dest="format", type="choice",
 -                             help="display format ([text]|xml)", default="text",
 -                             choices=("text", "xml"))
 +            parser.add_option("-f", "--format", dest="format", type="choice",
 +                              help="display format ([text]|xml)", default="text",
 +                              choices=("text", "xml"))
  
 -           parser.add_option("-F", "--fileformat", dest="fileformat", type="choice",
 -                             help="output file format ([xml]|xmllist|hrnlist)", default="xml",
 -                             choices=("xml", "xmllist", "hrnlist"))
 +            parser.add_option("-F", "--fileformat", dest="fileformat", type="choice",
 +                              help="output file format ([xml]|xmllist|hrnlist)", default="xml",
 +                              choices=("xml", "xmllist", "hrnlist"))
          if canonical == 'list':
 -           parser.add_option("-r", "--recursive", dest="recursive", action='store_true',
 -                             help="list all child records", default=False)
 -           parser.add_option("-v", "--verbose", dest="verbose", action='store_true',
 -                             help="gives details, like user keys", default=False)
 +            parser.add_option("-r", "--recursive", dest="recursive", action='store_true',
 +                              help="list all child records", default=False)
 +            parser.add_option("-v", "--verbose", dest="verbose", action='store_true',
 +                              help="gives details, like user keys", default=False)
          if canonical in ("delegate"):
 -           parser.add_option("-u", "--user",
 -                             action="store_true", dest="delegate_user", default=False,
 -                             help="delegate your own credentials; default if no other option is provided")
 -           parser.add_option("-s", "--slice", dest="delegate_slices",action='append',default=[],
 -                             metavar="slice_hrn", help="delegate cred. for slice HRN")
 -           parser.add_option("-a", "--auths", dest='delegate_auths',action='append',default=[],
 -                             metavar='auth_hrn', help="delegate cred for auth HRN")
 -           # this primarily is a shorthand for -A my_hrn^
 -           parser.add_option("-p", "--pi", dest='delegate_pi', default=None, action='store_true',
 -                             help="delegate your PI credentials, so s.t. like -A your_hrn^")
 -           parser.add_option("-A","--to-authority",dest='delegate_to_authority',action='store_true',default=False,
 -                             help="""by default the mandatory argument is expected to be a user, 
 +            parser.add_option("-u", "--user",
 +                              action="store_true", dest="delegate_user", default=False,
 +                              help="delegate your own credentials; default if no other option is provided")
 +            parser.add_option("-s", "--slice", dest="delegate_slices", action='append', default=[],
 +                              metavar="slice_hrn", help="delegate cred. for slice HRN")
 +            parser.add_option("-a", "--auths", dest='delegate_auths', action='append', default=[],
 +                              metavar='auth_hrn', help="delegate cred for auth HRN")
 +            # this primarily is a shorthand for -A my_hrn^
 +            parser.add_option("-p", "--pi", dest='delegate_pi', default=None, action='store_true',
 +                              help="delegate your PI credentials, so s.t. like -A your_hrn^")
 +            parser.add_option("-A", "--to-authority", dest='delegate_to_authority', action='store_true', default=False,
 +                              help="""by default the mandatory argument is expected to be a user, 
  use this if you mean an authority instead""")
  
          if canonical in ("myslice"):
 -            parser.add_option("-p","--password",dest='password',action='store',default=None,
 +            parser.add_option("-p", "--password", dest='password', action='store', default=None,
                                help="specify mainfold password on the command line")
 -            parser.add_option("-s", "--slice", dest="delegate_slices",action='append',default=[],
 -                             metavar="slice_hrn", help="delegate cred. for slice HRN")
 -            parser.add_option("-a", "--auths", dest='delegate_auths',action='append',default=[],
 -                             metavar='auth_hrn', help="delegate PI cred for auth HRN")
 -            parser.add_option('-d', '--delegate', dest='delegate', help="Override 'delegate' from the config file")
 -            parser.add_option('-b', '--backend',  dest='backend',  help="Override 'backend' from the config file")
 -        
 +            parser.add_option("-s", "--slice", dest="delegate_slices", action='append', default=[],
 +                              metavar="slice_hrn", help="delegate cred. for slice HRN")
 +            parser.add_option("-a", "--auths", dest='delegate_auths', action='append', default=[],
 +                              metavar='auth_hrn', help="delegate PI cred for auth HRN")
 +            parser.add_option('-d', '--delegate', dest='delegate',
 +                              help="Override 'delegate' from the config file")
 +            parser.add_option('-b', '--backend',  dest='backend',
 +                              help="Override 'backend' from the config file")
 +
          return parser
  
 -        
      #
      # Main: parse arguments and dispatch to command
      #
      def dispatch(self, command, command_options, command_args):
          (doc, args_string, example, canonical) = commands_dict[command]
 -        method=getattr(self, canonical, None)
 +        method = getattr(self, canonical, None)
          if not method:
              print("sfi: unknown command {}".format(command))
              raise SystemExit("Unknown command {}".format(command))
      def main(self):
          self.sfi_parser = self.create_parser_global()
          (options, args) = self.sfi_parser.parse_args()
 -        if options.help: 
 +        if options.help:
              self.print_commands_help(options)
              sys.exit(1)
          self.options = options
              self.logger.critical("No command given. Use -h for help.")
              self.print_commands_help(options)
              return -1
 -    
 +
          # complete / find unique match with command set
          command_candidates = Candidates(commands_list)
          input = args[0]
          # second pass options parsing
          self.command = command
          self.command_parser = self.create_parser_command(command)
 -        (command_options, command_args) = self.command_parser.parse_args(args[1:])
 +        (command_options, command_args) = self.command_parser.parse_args(
 +            args[1:])
          if command_options.help:
              self.print_help()
              sys.exit(1)
              command_options.type = normalize_type(command_options.type)
              if not command_options.type:
                  sys.exit(1)
 -        
 -        self.read_config() 
 +
 +        self.read_config()
          self.bootstrap()
          self.logger.debug("Command={}".format(self.command))
  
              self.logger.log_exc("sfi command {} failed".format(command))
              return 1
          return retcod
 -    
 +
      ####################
      def read_config(self):
          config_file = os.path.join(self.options.sfi_dir, "sfi_config")
 -        shell_config_file  = os.path.join(self.options.sfi_dir, "sfi_config.sh")
 +        shell_config_file = os.path.join(self.options.sfi_dir, "sfi_config.sh")
          try:
              if Config.is_ini(config_file):
                  config = Config(config_file)
              else:
                  # try upgrading from shell config format
 -                fp, fn = mkstemp(suffix='sfi_config', text=True)  
 +                fp, fn = mkstemp(suffix='sfi_config', text=True)
                  config = Config(fn)
 -                # we need to preload the sections we want parsed 
 +                # we need to preload the sections we want parsed
                  # from the shell config
                  config.add_section('sfi')
 -                # sface users should be able to use this same file to configure their stuff
 +                # sface users should be able to use this same file to configure
 +                # their stuff
                  config.add_section('sface')
 -                # manifold users should be able to specify the details 
 +                # manifold users should be able to specify the details
                  # of their backend server here for 'sfi myslice'
                  config.add_section('myslice')
                  config.load(config_file)
                  shutil.move(config_file, shell_config_file)
                  # write new config
                  config.save(config_file)
 -                 
 +
          except:
 -            self.logger.critical("Failed to read configuration file {}".format(config_file))
 -            self.logger.info("Make sure to remove the export clauses and to add quotes")
 +            self.logger.critical(
 +                "Failed to read configuration file {}".format(config_file))
 +            self.logger.info(
 +                "Make sure to remove the export clauses and to add quotes")
              if self.options.verbose == 0:
                  self.logger.info("Re-run with -v for more details")
              else:
 -                self.logger.log_exc("Could not read config file {}".format(config_file))
 +                self.logger.log_exc(
 +                    "Could not read config file {}".format(config_file))
              sys.exit(1)
 -     
 +
          self.config_instance = config
          errors = 0
          # Set SliceMgr URL
          if (self.options.sm is not None):
 -           self.sm_url = self.options.sm
 +            self.sm_url = self.options.sm
          elif hasattr(config, "SFI_SM"):
 -           self.sm_url = config.SFI_SM
 +            self.sm_url = config.SFI_SM
          else:
 -           self.logger.error("You need to set e.g. SFI_SM='http://your.slicemanager.url:12347/' in {}".format(config_file))
 -           errors += 1 
 +            self.logger.error(
 +                "You need to set e.g. SFI_SM='http://your.slicemanager.url:12347/' in {}".format(config_file))
 +            errors += 1
  
          # Set Registry URL
          if (self.options.registry is not None):
 -           self.reg_url = self.options.registry
 +            self.reg_url = self.options.registry
          elif hasattr(config, "SFI_REGISTRY"):
 -           self.reg_url = config.SFI_REGISTRY
 +            self.reg_url = config.SFI_REGISTRY
          else:
 -           self.logger.error("You need to set e.g. SFI_REGISTRY='http://your.registry.url:12345/' in {}".format(config_file))
 -           errors += 1 
 +            self.logger.error(
 +                "You need to set e.g. SFI_REGISTRY='http://your.registry.url:12345/' in {}".format(config_file))
 +            errors += 1
  
          # Set user HRN
          if (self.options.user is not None):
 -           self.user = self.options.user
 +            self.user = self.options.user
          elif hasattr(config, "SFI_USER"):
 -           self.user = config.SFI_USER
 +            self.user = config.SFI_USER
          else:
 -           self.logger.error("You need to set e.g. SFI_USER='plc.princeton.username' in {}".format(config_file))
 -           errors += 1 
 +            self.logger.error(
 +                "You need to set e.g. SFI_USER='plc.princeton.username' in {}".format(config_file))
 +            errors += 1
  
          # Set authority HRN
          if (self.options.auth is not None):
 -           self.authority = self.options.auth
 +            self.authority = self.options.auth
          elif hasattr(config, "SFI_AUTH"):
 -           self.authority = config.SFI_AUTH
 +            self.authority = config.SFI_AUTH
          else:
 -           self.logger.error("You need to set e.g. SFI_AUTH='plc.princeton' in {}".format(config_file))
 -           errors += 1 
 +            self.logger.error(
 +                "You need to set e.g. SFI_AUTH='plc.princeton' in {}".format(config_file))
 +            errors += 1
  
          self.config_file = config_file
          if errors:
 -           sys.exit(1)
 +            sys.exit(1)
  
      #
      # Get various credential and spec files
      #   - bootstrap authority credential from user credential
      #   - bootstrap slice credential from user credential
      #
 -    
 +
      # init self-signed cert, user credentials and gid
      def bootstrap(self):
          if self.options.verbose:
 -            self.logger.info("Initializing SfaClientBootstrap with {}".format(self.reg_url))
 +            self.logger.info(
 +                "Initializing SfaClientBootstrap with {}".format(self.reg_url))
          client_bootstrap = SfaClientBootstrap(self.user, self.reg_url, self.options.sfi_dir,
 -                                               logger=self.logger)
 +                                              logger=self.logger)
          # if -k is provided, use this to initialize private key
          if self.options.user_private_key:
 -            client_bootstrap.init_private_key_if_missing(self.options.user_private_key)
 +            client_bootstrap.init_private_key_if_missing(
 +                self.options.user_private_key)
          else:
 -            # trigger legacy compat code if needed 
 +            # trigger legacy compat code if needed
              # the name has changed from just <leaf>.pkey to <hrn>.pkey
              if not os.path.isfile(client_bootstrap.private_key_filename()):
                  self.logger.info("private key not found, trying legacy name")
                  try:
                      legacy_private_key = os.path.join(self.options.sfi_dir, "{}.pkey"
 -                                                       .format(Xrn.unescape(get_leaf(self.user))))
 +                                                      .format(Xrn.unescape(get_leaf(self.user))))
                      self.logger.debug("legacy_private_key={}"
                                        .format(legacy_private_key))
 -                    client_bootstrap.init_private_key_if_missing(legacy_private_key)
 +                    client_bootstrap.init_private_key_if_missing(
 +                        legacy_private_key)
                      self.logger.info("Copied private key from legacy location {}"
                                       .format(legacy_private_key))
                  except:
                      self.logger.log_exc("Can't find private key ")
                      sys.exit(1)
 -            
 +
          # make it bootstrap
          client_bootstrap.bootstrap_my_gid()
          # extract what's needed
          self.private_key = client_bootstrap.private_key()
          self.my_credential_string = client_bootstrap.my_credential_string()
          self.my_credential = {'geni_type': 'geni_sfa',
 -                              'geni_version': '3', 
 +                              'geni_version': '3',
                                'geni_value': self.my_credential_string}
          self.my_gid = client_bootstrap.my_gid()
          self.client_bootstrap = client_bootstrap
  
 -
      def my_authority_credential_string(self):
          if not self.authority:
 -            self.logger.critical("no authority specified. Use -a or set SF_AUTH")
 +            self.logger.critical(
 +                "no authority specified. Use -a or set SF_AUTH")
              sys.exit(-1)
          return self.client_bootstrap.authority_credential_string(self.authority)
  
      def slice_credential(self, name):
          return {'geni_type': 'geni_sfa',
                  'geni_version': '3',
 -                'geni_value': self.slice_credential_string(name)}    
 +                'geni_value': self.slice_credential_string(name)}
  
      # xxx should be supported by sfaclientbootstrap as well
      def delegate_cred(self, object_cred, hrn, type='authority'):
          # the gid and hrn of the object we are delegating
          if isinstance(object_cred, str):
 -            object_cred = Credential(string=object_cred) 
 +            object_cred = Credential(string=object_cred)
          object_gid = object_cred.get_gid_object()
          object_hrn = object_gid.get_hrn()
 -    
 +
          if not object_cred.get_privileges().get_all_delegate():
              self.logger.error("Object credential {} does not have delegate bit set"
                                .format(object_hrn))
  
          # the delegating user's gid
          caller_gidfile = self.my_gid()
 -  
 +
          # the gid of the user who will be delegated to
          delegee_gid = self.client_bootstrap.gid(hrn, type)
          delegee_hrn = delegee_gid.get_hrn()
 -        dcred = object_cred.delegate(delegee_gid, self.private_key, caller_gidfile)
 +        dcred = object_cred.delegate(
 +            delegee_gid, self.private_key, caller_gidfile)
          return dcred.save_to_string(save_parents=True)
 -     
 +
      #
      # Management of the servers
 -    # 
 +    #
  
      def registry(self):
          # cache the result
          if not hasattr(self, 'registry_proxy'):
              self.logger.info("Contacting Registry at: {}".format(self.reg_url))
              self.registry_proxy \
 -                =  SfaServerProxy(self.reg_url, self.private_key, self.my_gid, 
 -                                  timeout=self.options.timeout, verbose=self.options.debug)  
 +                = SfaServerProxy(self.reg_url, self.private_key, self.my_gid,
 +                                 timeout=self.options.timeout, verbose=self.options.debug)
          return self.registry_proxy
  
      def sliceapi(self):
          # cache the result
          if not hasattr(self, 'sliceapi_proxy'):
 -            # if the command exposes the --component option, figure it's hostname and connect at CM_PORT
 +            # if the command exposes the --component option, figure it's
 +            # hostname and connect at CM_PORT
              if hasattr(self.command_options, 'component') and self.command_options.component:
                  # resolve the hrn at the registry
                  node_hrn = self.command_options.component
                  records = self.registry().Resolve(node_hrn, self.my_credential_string)
                  records = filter_records('node', records)
                  if not records:
 -                    self.logger.warning("No such component:{}".format(opts.component))
 +                    self.logger.warning(
 +                        "No such component:{}".format(opts.component))
                  record = records[0]
                  cm_url = "http://{}:{}/".format(record['hostname'], CM_PORT)
 -                self.sliceapi_proxy = SfaServerProxy(cm_url, self.private_key, self.my_gid)
 +                self.sliceapi_proxy = SfaServerProxy(
 +                    cm_url, self.private_key, self.my_gid)
              else:
 -                # otherwise use what was provided as --sliceapi, or SFI_SM in the config
 +                # otherwise use what was provided as --sliceapi, or SFI_SM in
 +                # the config
                  if not self.sm_url.startswith('http://') or self.sm_url.startswith('https://'):
                      self.sm_url = 'http://' + self.sm_url
 -                self.logger.info("Contacting Slice Manager at: {}".format(self.sm_url))
 +                self.logger.info(
 +                    "Contacting Slice Manager at: {}".format(self.sm_url))
                  self.sliceapi_proxy \
 -                    = SfaServerProxy(self.sm_url, self.private_key, self.my_gid, 
 -                                     timeout=self.options.timeout, verbose=self.options.debug)  
 +                    = SfaServerProxy(self.sm_url, self.private_key, self.my_gid,
 +                                     timeout=self.options.timeout, verbose=self.options.debug)
          return self.sliceapi_proxy
  
      def get_cached_server_version(self, server):
          # check local cache first
          cache = None
 -        version = None 
 +        version = None
          cache_file = os.path.join(self.options.sfi_dir, 'sfi_cache.dat')
          cache_key = server.url + "-version"
          try:
          if cache:
              version = cache.get(cache_key)
  
 -        if not version: 
 +        if not version:
              result = server.GetVersion()
              version = ReturnValue.get_value(result)
              # cache version for 20 minutes
 -            cache.add(cache_key, version, ttl=60*20)
 +            cache.add(cache_key, version, ttl=60 * 20)
              self.logger.info("Updating cache file {}".format(cache_file))
              cache.save_to_file(cache_file)
  
 -        return version   
 -        
 -    ### resurrect this temporarily so we can support V1 aggregates for a while
 +        return version
 +
 +    # resurrect this temporarily so we can support V1 aggregates for a while
      def server_supports_options_arg(self, server):
          """
          Returns true if server support the optional call_id arg, false otherwise. 
          """
          server_version = self.get_cached_server_version(server)
          result = False
 -        # xxx need to rewrite this 
 +        # xxx need to rewrite this
          if int(server_version.get('geni_api')) >= 2:
              result = True
          return result
  
      def server_supports_call_id_arg(self, server):
          server_version = self.get_cached_server_version(server)
 -        result = False      
 +        result = False
          if 'sfa' in server_version and 'code_tag' in server_version:
              code_tag = server_version['code_tag']
              code_tag_parts = code_tag.split("-")
              rev = code_tag_parts[1]
              if int(major) == 1 and minor == 0 and build >= 22:
                  result = True
 -        return result                 
 +        return result
  
 -    ### ois = options if supported
 -    # to be used in something like serverproxy.Method(arg1, arg2, *self.ois(api_options))
 +    # ois = options if supported
 +    # to be used in something like serverproxy.Method(arg1, arg2,
 +    # *self.ois(api_options))
      def ois(self, server, option_dict):
 -        if self.server_supports_options_arg(server): 
 +        if self.server_supports_options_arg(server):
              return [option_dict]
          elif self.server_supports_call_id_arg(server):
 -            return [ unique_call_id() ]
 -        else: 
 +            return [unique_call_id()]
 +        else:
              return []
  
 -    ### cis = call_id if supported - like ois
 +    # cis = call_id if supported - like ois
      def cis(self, server):
          if self.server_supports_call_id_arg(server):
 -            return [ unique_call_id ]
 +            return [unique_call_id]
          else:
              return []
  
 -    ######################################## miscell utilities
 +    # miscell utilities
      def get_rspec_file(self, rspec):
 -       if (os.path.isabs(rspec)):
 -          file = rspec
 -       else:
 -          file = os.path.join(self.options.sfi_dir, rspec)
 -       if (os.path.isfile(file)):
 -          return file
 -       else:
 -          self.logger.critical("No such rspec file {}".format(rspec))
 -          sys.exit(1)
 -    
 -    def get_record_file(self, record):
 -       if (os.path.isabs(record)):
 -          file = record
 -       else:
 -          file = os.path.join(self.options.sfi_dir, record)
 -       if (os.path.isfile(file)):
 -          return file
 -       else:
 -          self.logger.critical("No such registry record file {}".format(record))
 -          sys.exit(1)
 +        if (os.path.isabs(rspec)):
 +            file = rspec
 +        else:
 +            file = os.path.join(self.options.sfi_dir, rspec)
 +        if (os.path.isfile(file)):
 +            return file
 +        else:
 +            self.logger.critical("No such rspec file {}".format(rspec))
 +            sys.exit(1)
  
 +    def get_record_file(self, record):
 +        if (os.path.isabs(record)):
 +            file = record
 +        else:
 +            file = os.path.join(self.options.sfi_dir, record)
 +        if (os.path.isfile(file)):
 +            return file
 +        else:
 +            self.logger.critical(
 +                "No such registry record file {}".format(record))
 +            sys.exit(1)
  
      # helper function to analyze raw output
 -    # for main : return 0 if everything is fine, something else otherwise (mostly 1 for now)
 +    # for main : return 0 if everything is fine, something else otherwise
 +    # (mostly 1 for now)
      def success(self, raw):
          return_value = ReturnValue(raw)
          output = ReturnValue.get_output(return_value)
          # means everything is fine
 -        if not output: 
 +        if not output:
              return 0
          # something went wrong
          print('ERROR:', output)
              sys.exit(1)
  
          print("# From configuration file {}".format(self.config_file))
 -        flags = [ ('sfi', [ ('registry', 'reg_url'),
 -                            ('auth', 'authority'),
 -                            ('user', 'user'),
 -                            ('sm', 'sm_url'),
 -                        ]),
 -                ]
 +        flags = [('sfi', [('registry', 'reg_url'),
 +                          ('auth', 'authority'),
 +                          ('user', 'user'),
 +                          ('sm', 'sm_url'),
 +                          ]),
 +                 ]
          if options.myslice:
 -            flags.append( ('myslice', ['backend', 'delegate', 'platform', 'username'] ) )
 +            flags.append(
 +                ('myslice', ['backend', 'delegate', 'platform', 'username']))
  
          for (section, tuples) in flags:
              print("[{}]".format(section))
              try:
                  for external_name, internal_name in tuples:
 -                    print("{:<20} = {}".format(external_name, getattr(self, internal_name)))
 +                    print("{:<20} = {}".format(
 +                        external_name, getattr(self, internal_name)))
              except:
                  for external_name, internal_name in tuples:
 -                    varname = "{}_{}".format(section.upper(), external_name.upper())
 +                    varname = "{}_{}".format(
 +                        section.upper(), external_name.upper())
                      value = getattr(self.config_instance, varname)
                      print("{:<20} = {}".format(external_name, value))
          # xxx should analyze result
          if len(args) != 0:
              self.print_help()
              sys.exit(1)
 -        
 +
          if options.version_local:
              version = version_core()
          else:
              result = server.GetVersion()
              version = ReturnValue.get_value(result)
          if self.options.raw:
 -            save_raw_to_file(result, self.options.raw, self.options.rawformat, self.options.rawbanner)
 +            save_raw_to_file(result, self.options.raw,
 +                             self.options.rawformat, self.options.rawbanner)
          else:
              pprinter = PrettyPrinter(indent=4)
              pprinter.pprint(version)
          opts = {}
          if options.recursive:
              opts['recursive'] = options.recursive
 -        
 +
          if options.show_credential:
              show_credentials(self.my_credential_string)
          try:
              save_records_to_file(options.file, list, options.fileformat)
          # xxx should analyze result
          return 0
 -    
 +
      @declare_command("name", "")
      def show(self, options, args):
          """
          resolve_options = {}
          if not options.no_details:
              resolve_options['details'] = True
 -        record_dicts = self.registry().Resolve(hrn, self.my_credential_string, resolve_options)
 +        record_dicts = self.registry().Resolve(
 +            hrn, self.my_credential_string, resolve_options)
          record_dicts = filter_records(options.type, record_dicts)
          if not record_dicts:
              self.logger.error("No record of type {}".format(options.type))
              def project(record):
                  projected = {}
                  for key in options.keys:
 -                    try: projected[key] = record[key]
 -                    except: pass
 +                    try:
 +                        projected[key] = record[key]
 +                    except:
 +                        pass
                  return projected
 -            record_dicts = [ project(record) for record in record_dicts ]
 -        records = [ Record(dict=record_dict) for record_dict in record_dicts ]
 +            record_dicts = [project(record) for record in record_dicts]
 +        records = [Record(dict=record_dict) for record_dict in record_dicts]
          for record in records:
 -            if (options.format == "text"):      record.dump(sort=True)  
 -            else:                               print(record.save_as_xml())
 +            if (options.format == "text"):
 +                record.dump(sort=True)
 +            else:
 +                print(record.save_as_xml())
          if options.file:
 -            save_records_to_file(options.file, record_dicts, options.fileformat)
 +            save_records_to_file(
 +                options.file, record_dicts, options.fileformat)
          # xxx should analyze result
          return 0
 -    
 -    # this historically was named 'add', it is now 'register' with an alias for legacy
 +
 +    # this historically was named 'add', it is now 'register' with an alias
 +    # for legacy
      @declare_command("[xml-filename]", "", ['add'])
      def register(self, options, args):
          """
              try:
                  record_filepath = args[0]
                  rec_file = self.get_record_file(record_filepath)
 -                record_dict.update(load_record_from_file(rec_file).record_to_dict())
 +                record_dict.update(load_record_from_file(
 +                    rec_file).record_to_dict())
              except:
                  print("Cannot load record file {}".format(record_filepath))
                  sys.exit(1)
          if options:
              record_dict.update(load_record_from_opts(options).record_to_dict())
          # we should have a type by now
 -        if 'type' not in record_dict :
 +        if 'type' not in record_dict:
              self.print_help()
              sys.exit(1)
          # this is still planetlab dependent.. as plc will whine without that
              if not 'first_name' in record_dict:
                  record_dict['first_name'] = record_dict['hrn']
              if 'last_name' not in record_dict:
 -                record_dict['last_name'] = record_dict['hrn'] 
 +                record_dict['last_name'] = record_dict['hrn']
          register = self.registry().Register(record_dict, auth_cred)
          # xxx looks like the result here is not ReturnValue-compatible
 -        #return self.success (register)
 +        # return self.success (register)
          # xxx should analyze result
          return 0
 -    
 +
      @declare_command("[xml-filename]", "")
      def update(self, options, args):
          """
          if len(args) == 1:
              record_filepath = args[0]
              rec_file = self.get_record_file(record_filepath)
 -            record_dict.update(load_record_from_file(rec_file).record_to_dict())
 +            record_dict.update(load_record_from_file(
 +                rec_file).record_to_dict())
          if options:
              record_dict.update(load_record_from_opts(options).record_to_dict())
          # at the very least we need 'type' here
              try:
                  cred = self.slice_credential_string(record_dict['hrn'])
              except ServerException as e:
 -               # XXX smbaker -- once we have better error return codes, update this
 -               # to do something better than a string compare
 -               if "Permission error" in e.args[0]:
 -                   cred = self.my_authority_credential_string()
 -               else:
 -                   raise
 +                # XXX smbaker -- once we have better error return codes, update this
 +                # to do something better than a string compare
 +                if "Permission error" in e.args[0]:
 +                    cred = self.my_authority_credential_string()
 +                else:
 +                    raise
          elif record_dict['type'] in ['authority']:
              cred = self.my_authority_credential_string()
          elif record_dict['type'] in ['node']:
              cred = self.my_authority_credential_string()
          else:
 -            raise Exception("unknown record type {}".format(record_dict['type']))
 +            raise Exception(
 +                "unknown record type {}".format(record_dict['type']))
          if options.show_credential:
              show_credentials(cred)
          update = self.registry().Update(record_dict, cred)
          # xxx looks like the result here is not ReturnValue-compatible
 -        #return self.success(update)
 +        # return self.success(update)
          # xxx should analyze result
          return 0
 -  
 +
      @declare_command("hrn", "")
      def remove(self, options, args):
          """
              sys.exit(1)
  
          hrn = args[0]
 -        type = options.type 
 +        type = options.type
          if type in ['all']:
              type = '*'
          if options.show_credential:
              show_credentials(auth_cred)
          remove = self.registry().Remove(hrn, auth_cred, type)
          # xxx looks like the result here is not ReturnValue-compatible
 -        #return self.success (remove)
 +        # return self.success (remove)
          # xxx should analyze result
          return 0
 -    
 +
      # ==================================================================
      # Slice-related commands
      # ==================================================================
  
          server = self.sliceapi()
          # set creds
-         creds = [self.my_credential]
+         creds = [self.my_credential_string]
          if options.delegate:
 -            creds.append(self.delegate_cred(cred, get_authority(self.authority)))
 +            creds.append(self.delegate_cred(
 +                cred, get_authority(self.authority)))
          if options.show_credential:
              show_credentials(creds)
  
          # been a required argument since v1 API
          api_options = {}
          # always send call_id to v2 servers
 -        api_options ['call_id'] = unique_call_id()
 +        api_options['call_id'] = unique_call_id()
          # ask for cached value if available
 -        api_options ['cached'] = True
 +        api_options['cached'] = True
          if options.info:
              api_options['info'] = options.info
          if options.list_leases:
              else:
                  api_options['cached'] = True
          version_manager = VersionManager()
 -        api_options['geni_rspec_version'] = version_manager.get_version(options.rspec_version).to_dict()
 +        api_options['geni_rspec_version'] = version_manager.get_version(
 +            options.rspec_version).to_dict()
  
          list_resources = server.ListResources(creds, api_options)
          value = ReturnValue.get_value(list_resources)
          if self.options.raw:
 -            save_raw_to_file(list_resources, self.options.raw, self.options.rawformat, self.options.rawbanner)
 +            save_raw_to_file(list_resources, self.options.raw,
 +                             self.options.rawformat, self.options.rawbanner)
          if options.file is not None:
              save_rspec_to_file(value, options.file)
          if (self.options.raw is None) and (options.file is None):
          # set creds
          creds = [self.slice_credential(args[0])]
          if options.delegate:
 -            creds.append(self.delegate_cred(cred, get_authority(self.authority)))
 +            creds.append(self.delegate_cred(
 +                cred, get_authority(self.authority)))
          if options.show_credential:
              show_credentials(creds)
  
                         'info': options.info,
                         'list_leases': options.list_leases,
                         'geni_rspec_version': {'type': 'geni', 'version': '3'},
 -                      }
 +                       }
          if options.info:
              api_options['info'] = options.info
  
              server_version = self.get_cached_server_version(server)
              if 'sfa' in server_version:
                  # just request the version the client wants
 -                api_options['geni_rspec_version'] = version_manager.get_version(options.rspec_version).to_dict()
 +                api_options['geni_rspec_version'] = version_manager.get_version(
 +                    options.rspec_version).to_dict()
              else:
 -                api_options['geni_rspec_version'] = {'type': 'geni', 'version': '3'}
 +                api_options['geni_rspec_version'] = {
 +                    'type': 'geni', 'version': '3'}
          urn = Xrn(args[0], type='slice').get_urn()
 -        remove_none_fields(api_options) 
 +        remove_none_fields(api_options)
          describe = server.Describe([urn], creds, api_options)
          value = ReturnValue.get_value(describe)
          if self.options.raw:
 -            save_raw_to_file(describe, self.options.raw, self.options.rawformat, self.options.rawbanner)
 +            save_raw_to_file(describe, self.options.raw,
 +                             self.options.rawformat, self.options.rawbanner)
          if options.file is not None:
              save_rspec_to_file(value['geni_rspec'], options.file)
          if (self.options.raw is None) and (options.file is None):
          server = self.sliceapi()
          # slice urn
          slice_hrn = args[0]
 -        slice_urn = hrn_to_urn(slice_hrn, 'slice') 
 +        slice_urn = hrn_to_urn(slice_hrn, 'slice')
  
          if len(args) > 1:
              # we have sliver urns
          # creds
          slice_cred = self.slice_credential(slice_hrn)
          creds = [slice_cred]
 -        
 +
          # options and call_id when supported
          api_options = {}
 -        api_options ['call_id'] = unique_call_id()
 +        api_options['call_id'] = unique_call_id()
          if options.show_credential:
              show_credentials(creds)
 -        delete = server.Delete(sliver_urns, creds, *self.ois(server, api_options ) )
 +        delete = server.Delete(sliver_urns, creds, *
 +                               self.ois(server, api_options))
          value = ReturnValue.get_value(delete)
          if self.options.raw:
 -            save_raw_to_file(delete, self.options.raw, self.options.rawformat, self.options.rawbanner)
 +            save_raw_to_file(delete, self.options.raw,
 +                             self.options.rawformat, self.options.rawbanner)
          else:
              print(value)
          return self.success(delete)
              # delegate our cred to the slice manager
              # do not delegate cred to slicemgr...not working at the moment
              pass
 -            #if server_version.get('hrn'):
 +            # if server_version.get('hrn'):
              #    delegated_cred = self.delegate_cred(slice_cred, server_version['hrn'])
 -            #elif server_version.get('urn'):
 +            # elif server_version.get('urn'):
              #    delegated_cred = self.delegate_cred(slice_cred, urn_to_hrn(server_version['urn']))
  
          if options.show_credential:
  
          # rspec
          api_options = {}
 -        api_options ['call_id'] = unique_call_id()
 +        api_options['call_id'] = unique_call_id()
          # users
          sfa_users = []
          geni_users = []
 -        slice_records = self.registry().Resolve(slice_urn, [self.my_credential_string])
 +        slice_records = self.registry().Resolve(
 +            slice_urn, [self.my_credential_string])
          remove_none_fields(slice_records[0])
          if slice_records and 'reg-researchers' in slice_records[0] and slice_records[0]['reg-researchers'] != []:
              slice_record = slice_records[0]
              user_hrns = slice_record['reg-researchers']
              user_urns = [hrn_to_urn(hrn, 'user') for hrn in user_hrns]
 -            user_records = self.registry().Resolve(user_urns, [self.my_credential_string])
 +            user_records = self.registry().Resolve(
 +                user_urns, [self.my_credential_string])
              sfa_users = sfa_users_arg(user_records, slice_record)
              geni_users = pg_users_arg(user_records)
  
  
          with open(rspec_file) as rspec:
              rspec_xml = rspec.read()
 -            allocate = server.Allocate(slice_urn, creds, rspec_xml, api_options)
 +            allocate = server.Allocate(
 +                slice_urn, creds, rspec_xml, api_options)
          value = ReturnValue.get_value(allocate)
          if self.options.raw:
 -            save_raw_to_file(allocate, self.options.raw, self.options.rawformat, self.options.rawbanner)
 +            save_raw_to_file(allocate, self.options.raw,
 +                             self.options.rawformat, self.options.rawbanner)
          if options.file is not None:
              save_rspec_to_file(value['geni_rspec'], options.file)
          if (self.options.raw is None) and (options.file is None):
              # delegate our cred to the slice manager
              # do not delegate cred to slicemgr...not working at the moment
              pass
 -            #if server_version.get('hrn'):
 +            # if server_version.get('hrn'):
              #    delegated_cred = self.delegate_cred(slice_cred, server_version['hrn'])
 -            #elif server_version.get('urn'):
 +            # elif server_version.get('urn'):
              #    delegated_cred = self.delegate_cred(slice_cred, urn_to_hrn(server_version['urn']))
  
          if options.show_credential:
              show_credentials(creds)
  
          api_options = {}
 -        api_options ['call_id'] = unique_call_id()
 +        api_options['call_id'] = unique_call_id()
  
          # set the requtested rspec version
          version_manager = VersionManager()
          #    keys: [<ssh key A>, <ssh key B>]
          #  }]
          users = []
 -        slice_records = self.registry().Resolve(slice_urn, [self.my_credential_string])
 +        slice_records = self.registry().Resolve(
 +            slice_urn, [self.my_credential_string])
          if slice_records and 'reg-researchers' in slice_records[0] and slice_records[0]['reg-researchers'] != []:
              slice_record = slice_records[0]
              user_hrns = slice_record['reg-researchers']
              user_urns = [hrn_to_urn(hrn, 'user') for hrn in user_hrns]
 -            user_records = self.registry().Resolve(user_urns, [self.my_credential_string])
 +            user_records = self.registry().Resolve(
 +                user_urns, [self.my_credential_string])
              users = pg_users_arg(user_records)
 -        
 +
          api_options['geni_users'] = users
          provision = server.Provision(sliver_urns, creds, api_options)
          value = ReturnValue.get_value(provision)
          if self.options.raw:
 -            save_raw_to_file(provision, self.options.raw, self.options.rawformat, self.options.rawbanner)
 +            save_raw_to_file(provision, self.options.raw,
 +                             self.options.rawformat, self.options.rawbanner)
          if options.file is not None:
              save_rspec_to_file(value['geni_rspec'], options.file)
          if (self.options.raw is None) and (options.file is None):
          server = self.sliceapi()
          # slice urn
          slice_hrn = args[0]
 -        slice_urn = hrn_to_urn(slice_hrn, 'slice') 
 +        slice_urn = hrn_to_urn(slice_hrn, 'slice')
  
 -        # creds 
 +        # creds
          slice_cred = self.slice_credential(slice_hrn)
          creds = [slice_cred]
  
          api_options['call_id'] = unique_call_id()
          if options.show_credential:
              show_credentials(creds)
 -        status = server.Status([slice_urn], creds, *self.ois(server, api_options))
 +        status = server.Status([slice_urn], creds, *
 +                               self.ois(server, api_options))
          value = ReturnValue.get_value(status)
          if self.options.raw:
 -            save_raw_to_file(status, self.options.raw, self.options.rawformat, self.options.rawbanner)
 +            save_raw_to_file(status, self.options.raw,
 +                             self.options.rawformat, self.options.rawbanner)
          else:
              print(value)
          return self.success(status)
          slice_cred = self.slice_credential(args[0])
          creds = [slice_cred]
          if options.delegate:
 -            delegated_cred = self.delegate_cred(slice_cred, get_authority(self.authority))
 +            delegated_cred = self.delegate_cred(
 +                slice_cred, get_authority(self.authority))
              creds.append(delegated_cred)
 -        
 -        perform_action = server.PerformOperationalAction(sliver_urns, creds, action , api_options)
 +
 +        perform_action = server.PerformOperationalAction(
 +            sliver_urns, creds, action, api_options)
          value = ReturnValue.get_value(perform_action)
          if self.options.raw:
 -            save_raw_to_file(perform_action, self.options.raw, self.options.rawformat, self.options.rawbanner)
 +            save_raw_to_file(perform_action, self.options.raw,
 +                             self.options.rawformat, self.options.rawbanner)
          else:
              print(value)
          return self.success(perform_action)
                                  "sfi renew onelab.ple.heartbeat 2015-04-31T14:00:00Z",
                                  "sfi renew onelab.ple.heartbeat +5d",
                                  "sfi renew onelab.ple.heartbeat +3w",
 -                                "sfi renew onelab.ple.heartbeat +2m",]))
 +                                "sfi renew onelab.ple.heartbeat +2m", ]))
      def renew(self, options, args):
          """
          renew slice(Renew)
              api_options['geni_extend_alap'] = True
          if options.show_credential:
              show_credentials(creds)
 -        renew =  server.Renew(sliver_urns, creds, input_time, *self.ois(server, api_options))
 +        renew = server.Renew(sliver_urns, creds, input_time,
 +                             *self.ois(server, api_options))
          value = ReturnValue.get_value(renew)
          if self.options.raw:
 -            save_raw_to_file(renew, self.options.raw, self.options.rawformat, self.options.rawbanner)
 +            save_raw_to_file(renew, self.options.raw,
 +                             self.options.rawformat, self.options.rawbanner)
          else:
              print(value)
          return self.success(renew)
          server = self.sliceapi()
          # slice urn
          slice_hrn = args[0]
 -        slice_urn = hrn_to_urn(slice_hrn, 'slice') 
 +        slice_urn = hrn_to_urn(slice_hrn, 'slice')
          # creds
          slice_cred = self.slice_credential(slice_hrn)
          creds = [slice_cred]
          shutdown = server.Shutdown(slice_urn, creds)
          value = ReturnValue.get_value(shutdown)
          if self.options.raw:
 -            save_raw_to_file(shutdown, self.options.raw, self.options.rawformat, self.options.rawbanner)
 +            save_raw_to_file(shutdown, self.options.raw,
 +                             self.options.rawformat, self.options.rawbanner)
          else:
              print(value)
          return self.success(shutdown)
              sys.exit(1)
  
          target_hrn = args[0]
 -        my_gid_string = open(self.client_bootstrap.my_gid()).read() 
 +        my_gid_string = open(self.client_bootstrap.my_gid()).read()
          gid = self.registry().CreateGid(self.my_credential_string, target_hrn, my_gid_string)
          if options.file:
              filename = options.file
          else:
 -            filename = os.sep.join([self.options.sfi_dir, '{}.gid'.format(target_hrn)])
 +            filename = os.sep.join(
 +                [self.options.sfi_dir, '{}.gid'.format(target_hrn)])
          self.logger.info("writing {} gid to {}".format(target_hrn, filename))
          GID(string=gid).save_to_file(filename)
          # xxx should analyze result
          return 0
 -         
 +
      ####################
      @declare_command("to_hrn", """$ sfi delegate -u -p -s ple.inria.heartbeat -s ple.inria.omftest ple.upmc.slicebrowser
  
          for slice_hrn in options.delegate_slices:
              message = "{}.slice".format(slice_hrn)
              original = self.slice_credential_string(slice_hrn)
 -            tuples.append( (message, original,) )
 +            tuples.append((message, original,))
          if options.delegate_pi:
              my_authority = self.authority
              message = "{}.pi".format(my_authority)
              original = self.my_authority_credential_string()
 -            tuples.append( (message, original,) )
 +            tuples.append((message, original,))
          for auth_hrn in options.delegate_auths:
              message = "{}.auth".format(auth_hrn)
              original = self.authority_credential_string(auth_hrn)
 -            tuples.append( (message, original, ) )
 +            tuples.append((message, original, ))
          # if nothing was specified at all at this point, let's assume -u
          if not tuples:
              options.delegate_user = True
          if options.delegate_user:
              message = "{}.user".format(self.user)
              original = self.my_credential_string
 -            tuples.append( (message, original, ) )
 +            tuples.append((message, original, ))
  
          # default type for beneficial is user unless -A
          to_type = 'authority' if options.delegate_to_authority else 'user'
          # let's now handle all this
          # it's all in the filenaming scheme
          for (message, original) in tuples:
 -            delegated_string = self.client_bootstrap.delegate_credential_string(original, to_hrn, to_type)
 +            delegated_string = self.client_bootstrap.delegate_credential_string(
 +                original, to_hrn, to_type)
              delegated_credential = Credential(string=delegated_string)
              filename = os.path.join(self.options.sfi_dir,
                                      "{}_for_{}.{}.cred".format(message, to_hrn, to_type))
              delegated_credential.save_to_file(filename, save_parents=True)
              self.logger.info("delegated credential for {} to {} and wrote to {}"
                               .format(message, to_hrn, filename))
 -    
 +
      ####################
      @declare_command("", """$ less +/myslice sfi_config
  [myslice]
@@@ -1785,8 -1684,9 +1785,8 @@@ $ sfi m -b http://mymanifold.foo.com:70
    is synonym to sfi myslice as no other command starts with an 'm'
    and uses a custom backend for this one call
  """
 -) # declare_command
 +                     )  # declare_command
      def myslice(self, options, args):
 -
          """ This helper is for refreshing your credentials at myslice; it will
      * compute all the slices that you currently have credentials on
      * refresh all your credentials (you as a user and pi, your slices)
              self.print_help()
              sys.exit(1)
          # enable info by default
 -        self.logger.setLevelFromOptVerbose(self.options.verbose+1)
 -        ### the rough sketch goes like this
 +        self.logger.setLevelFromOptVerbose(self.options.verbose + 1)
 +        # the rough sketch goes like this
          # (0) produce a p12 file
          self.client_bootstrap.my_pkcs12()
  
          # (a) rain check for sufficient config in sfi_config
          myslice_dict = {}
 -        myslice_keys = [ 'backend', 'delegate', 'platform', 'username']
 +        myslice_keys = ['backend', 'delegate', 'platform', 'username']
          for key in myslice_keys:
              value = None
              # oct 2013 - I'm finding myself juggling with config files
              sys.exit(1)
          my_record = my_records[0]
          my_auths_all = my_record['reg-pi-authorities']
 -        self.logger.info("Found {} authorities that we are PI for".format(len(my_auths_all)))
 +        self.logger.info(
 +            "Found {} authorities that we are PI for".format(len(my_auths_all)))
          self.logger.debug("They are {}".format(my_auths_all))
 -        
 +
          my_auths = my_auths_all
          if options.delegate_auths:
 -            my_auths = list(set(my_auths_all).intersection(set(options.delegate_auths)))
 -            self.logger.debug("Restricted to user-provided auths {}".format(my_auths))
 +            my_auths = list(set(my_auths_all).intersection(
 +                set(options.delegate_auths)))
 +            self.logger.debug(
 +                "Restricted to user-provided auths {}".format(my_auths))
  
          # (c) get the set of slices that we are in
          my_slices_all = my_record['reg-slices']
 -        self.logger.info("Found {} slices that we are member of".format(len(my_slices_all)))
 +        self.logger.info(
 +            "Found {} slices that we are member of".format(len(my_slices_all)))
          self.logger.debug("They are: {}".format(my_slices_all))
 - 
 +
          my_slices = my_slices_all
          # if user provided slices, deal only with these - if they are found
          if options.delegate_slices:
 -            my_slices = list(set(my_slices_all).intersection(set(options.delegate_slices)))
 -            self.logger.debug("Restricted to user-provided slices: {}".format(my_slices))
 +            my_slices = list(set(my_slices_all).intersection(
 +                set(options.delegate_slices)))
 +            self.logger.debug(
 +                "Restricted to user-provided slices: {}".format(my_slices))
  
          # (d) make sure we have *valid* credentials for all these
          hrn_credentials = []
 -        hrn_credentials.append( (self.user, 'user', self.my_credential_string,) )
 +        hrn_credentials.append((self.user, 'user', self.my_credential_string,))
          for auth_hrn in my_auths:
 -            hrn_credentials.append( (auth_hrn, 'auth', self.authority_credential_string(auth_hrn),) )
 +            hrn_credentials.append(
 +                (auth_hrn, 'auth', self.authority_credential_string(auth_hrn),))
          for slice_hrn in my_slices:
              try:
 -                hrn_credentials.append( (slice_hrn, 'slice', self.slice_credential_string(slice_hrn),) )
 +                hrn_credentials.append(
 +                    (slice_hrn, 'slice', self.slice_credential_string(slice_hrn),))
              except:
                  print("WARNING: could not get slice credential for slice {}"
                        .format(slice_hrn))
  
          # (e) check for the delegated version of these
 -        # xxx todo add an option -a/-A? like for 'sfi delegate' for when we ever 
 +        # xxx todo add an option -a/-A? like for 'sfi delegate' for when we ever
          # switch to myslice using an authority instead of a user
          delegatee_type = 'user'
          delegatee_hrn = myslice_dict['delegate']
          hrn_delegated_credentials = []
          for (hrn, htype, credential) in hrn_credentials:
 -            delegated_credential = self.client_bootstrap.delegate_credential_string(credential, delegatee_hrn, delegatee_type)
 +            delegated_credential = self.client_bootstrap.delegate_credential_string(
 +                credential, delegatee_hrn, delegatee_type)
              # save these so user can monitor what she's uploaded
 -            filename = os.path.join( self.options.sfi_dir,
 -                                      "{}.{}_for_{}.{}.cred"\
 -                                      .format(hrn, htype, delegatee_hrn, delegatee_type))
 +            filename = os.path.join(self.options.sfi_dir,
 +                                    "{}.{}_for_{}.{}.cred"
 +                                    .format(hrn, htype, delegatee_hrn, delegatee_type))
              with open(filename, 'w') as f:
                  f.write(delegated_credential)
              self.logger.debug("(Over)wrote {}".format(filename))
 -            hrn_delegated_credentials.append((hrn, htype, delegated_credential, filename, ))
 +            hrn_delegated_credentials.append(
 +                (hrn, htype, delegated_credential, filename, ))
  
          # (f) and finally upload them to manifold server
          # xxx todo add an option so the password can be set on the command line
          # (but *NOT* in the config file) so other apps can leverage this
 -        self.logger.info("Uploading on backend at {}".format(myslice_dict['backend']))
 +        self.logger.info("Uploading on backend at {}".format(
 +            myslice_dict['backend']))
          uploader = ManifoldUploader(logger=self.logger,
 -                                     url=myslice_dict['backend'],
 -                                     platform=myslice_dict['platform'],
 -                                     username=myslice_dict['username'],
 -                                     password=options.password)
 +                                    url=myslice_dict['backend'],
 +                                    platform=myslice_dict['platform'],
 +                                    username=myslice_dict['username'],
 +                                    password=options.password)
          uploader.prompt_all()
          (count_all, count_success) = (0, 0)
          for (hrn, htype, delegated_credential, filename) in hrn_delegated_credentials:
      def trusted(self, options, args):
          """
          return the trusted certs at this interface (get_trusted_certs)
 -        """ 
 +        """
          if options.registry_interface:
              server = self.registry()
          else:
          # at first sight a list here means it's fine,
          # and a dict suggests an error (no support for introspection?)
          if isinstance(results, list):
 -            results = [ name for name in results if 'system.' not in name ]
 +            results = [name for name in results if 'system.' not in name]
              results.sort()
              print("== methods supported at {}".format(server.url))
              if 'Discover' in results:
diff --combined sfa/trust/certificate.py
  # The above copyright notice and this permission notice shall be
  # included in all copies or substantial portions of the Work.
  #
 -# THE WORK IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS 
 -# OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
 -# MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND 
 -# NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT 
 -# HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, 
 -# WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, 
 -# OUT OF OR IN CONNECTION WITH THE WORK OR THE USE OR OTHER DEALINGS 
 +# THE WORK IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
 +# OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
 +# MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
 +# NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
 +# HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
 +# WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
 +# OUT OF OR IN CONNECTION WITH THE WORK OR THE USE OR OTHER DEALINGS
  # IN THE WORK.
  #----------------------------------------------------------------------
  
@@@ -67,7 -67,6 +67,7 @@@ glo_passphrase_callback = Non
  #
  # The callback should return a string containing the passphrase.
  
 +
  def set_passphrase_callback(callback_func):
      global glo_passphrase_callback
  
  ##
  # Sets a fixed passphrase.
  
 +
  def set_passphrase(passphrase):
 -    set_passphrase_callback( lambda k,s,x: passphrase )
 +    set_passphrase_callback(lambda k, s, x: passphrase)
  
  ##
  # Check to see if a passphrase works for a particular private key string.
  # Intended to be used by passphrase callbacks for input validation.
  
 +
  def test_passphrase(string, passphrase):
      try:
 -        OpenSSL.crypto.load_privatekey(OpenSSL.crypto.FILETYPE_PEM, string, (lambda x: passphrase))
 +        OpenSSL.crypto.load_privatekey(
 +            OpenSSL.crypto.FILETYPE_PEM, string, (lambda x: passphrase))
          return True
      except:
          return False
  
 +
  def convert_public_key(key):
      keyconvert_path = "/usr/bin/keyconvert.py"
      if not os.path.isfile(keyconvert_path):
 -        raise IOError("Could not find keyconvert in {}".format(keyconvert_path))
 +        raise IOError(
 +            "Could not find keyconvert in {}".format(keyconvert_path))
  
      # we can only convert rsa keys
      if "ssh-dss" in key:
      # that it can be expected to see why it failed.
      # TODO: for production, cleanup the temporary files
      if not os.path.exists(ssl_fn):
 -        raise Exception("keyconvert: generated certificate not found. keyconvert may have failed.")
 +        raise Exception(
 +            "keyconvert: generated certificate not found. keyconvert may have failed.")
  
      k = Keypair()
      try:
  # A Keypair object may represent both a public and private key pair, or it
  # may represent only a public key (this usage is consistent with OpenSSL).
  
 +
  class Keypair:
      key = None       # public/private keypair
      m2key = None     # public key (m2crypto format)
              self.load_from_file(filename)
  
      ##
 -    # Create a RSA public/private key pair and store it inside the keypair object
 +    # Create a RSA public/private key pair and store it inside the keypair
 +    # object
  
      def create(self):
          self.key = OpenSSL.crypto.PKey()
          self.filename = filename
  
      ##
 -    # Load the private key from a file. Implicity the private key includes the public key.
 +    # Load the private key from a file. Implicity the private key includes the
 +    # public key.
  
      def load_from_file(self, filename):
          self.filename = filename
          self.load_from_string(buffer)
  
      ##
 -    # Load the private key from a string. Implicitly the private key includes the public key.
 +    # Load the private key from a string. Implicitly the private key includes
 +    # the public key.
  
      def load_from_string(self, string):
          import M2Crypto
              self.m2key = M2Crypto.EVP.load_key_string(
                  string, functools.partial(glo_passphrase_callback, self, string))
          else:
 -            self.key = OpenSSL.crypto.load_privatekey(OpenSSL.crypto.FILETYPE_PEM, string)
 +            self.key = OpenSSL.crypto.load_privatekey(
 +                OpenSSL.crypto.FILETYPE_PEM, string)
              self.m2key = M2Crypto.EVP.load_key_string(string)
  
      ##
  
          # create an m2 x509 cert
          m2name = M2Crypto.X509.X509_Name()
 -        m2name.add_entry_by_txt(field="CN", type=0x1001, entry="junk", len=-1, loc=-1, set=0)
 +        m2name.add_entry_by_txt(field="CN", type=0x1001,
 +                                entry="junk", len=-1, loc=-1, set=0)
          m2x509 = M2Crypto.X509.X509()
          m2x509.set_pubkey(self.m2key)
          m2x509.set_serial_number(0)
  
          # convert the m2 x509 cert to a pyopenssl x509
          m2pem = m2x509.as_pem()
 -        pyx509 = OpenSSL.crypto.load_certificate(OpenSSL.crypto.FILETYPE_PEM, m2pem)
 +        pyx509 = OpenSSL.crypto.load_certificate(
 +            OpenSSL.crypto.FILETYPE_PEM, m2pem)
  
          # get the pyopenssl pkey from the pyopenssl x509
          self.key = pyx509.get_pubkey()
  
      # only informative
      def get_filename(self):
 -        return getattr(self,'filename',None)
 +        return getattr(self, 'filename', None)
  
      def dump(self, *args, **kwargs):
          print(self.dump_string(*args, **kwargs))
  
      def dump_string(self):
 -        result =  ""
 +        result = ""
          result += "KEYPAIR: pubkey={:>40}...".format(self.get_pubkey_string())
          filename = self.get_filename()
 -        if filename: result += "Filename {}\n".format(filename)
 +        if filename:
 +            result += "Filename {}\n".format(filename)
          return result
  
  ##
  # When saving a certificate to a file or a string, the caller can choose
  # whether to save the parent certificates as well.
  
 +
  class Certificate:
      digest = "sha256"
  
  #    issuerKey = None
  #    issuerSubject = None
  #    parent = None
 -    isCA = None # will be a boolean once set
 +    isCA = None  # will be a boolean once set
  
      separator = "-----parent-----"
  
          self.x509 = OpenSSL.crypto.X509()
          # FIXME: Use different serial #s
          self.x509.set_serial_number(3)
 -        self.x509.gmtime_adj_notBefore(0) # 0 means now
 -        self.x509.gmtime_adj_notAfter(lifeDays*60*60*24) # five years is default
 -        self.x509.set_version(2) # x509v3 so it can have extensions
 -
 +        self.x509.gmtime_adj_notBefore(0)  # 0 means now
 +        self.x509.gmtime_adj_notAfter(
 +            lifeDays * 60 * 60 * 24)  # five years is default
 +        self.x509.set_version(2)  # x509v3 so it can have extensions
  
      ##
      # Given a pyOpenSSL X509 object, store that object inside of this
  
      def load_from_string(self, string):
          # if it is a chain of multiple certs, then split off the first one and
 -        # load it (support for the ---parent--- tag as well as normal chained certs)
 +        # load it (support for the ---parent--- tag as well as normal chained
 +        # certs)
  
          if string is None or string.strip() == "":
              logger.warn("Empty string in load_from_string")
              return
  
          string = string.strip()
 -        
 +
          # If it's not in proper PEM format, wrap it
          if string.count('-----BEGIN CERTIFICATE') == 0:
              string = '-----BEGIN CERTIFICATE-----\n{}\n-----END CERTIFICATE-----'\
          # such as the text of the certificate, skip the text
          beg = string.find('-----BEGIN CERTIFICATE')
          if beg > 0:
 -            # skipping over non cert beginning                                                                                                              
 +            # skipping over non cert beginning
              string = string[beg:]
  
          parts = []
  
          if string.count('-----BEGIN CERTIFICATE-----') > 1 and \
 -               string.count(Certificate.separator) == 0:
 -            parts = string.split('-----END CERTIFICATE-----',1)
 +                string.count(Certificate.separator) == 0:
 +            parts = string.split('-----END CERTIFICATE-----', 1)
              parts[0] += '-----END CERTIFICATE-----'
          else:
              parts = string.split(Certificate.separator, 1)
  
 -        self.x509 = OpenSSL.crypto.load_certificate(OpenSSL.crypto.FILETYPE_PEM, parts[0])
 +        self.x509 = OpenSSL.crypto.load_certificate(
 +            OpenSSL.crypto.FILETYPE_PEM, parts[0])
  
          if self.x509 is None:
 -            logger.warn("Loaded from string but cert is None: {}".format(string))
 +            logger.warn(
 +                "Loaded from string but cert is None: {}".format(string))
  
          # if there are more certs, then create a parent and let the parent load
          # itself from the remainder of the string
          if self.x509 is None:
              logger.warn("None cert in certificate.save_to_string")
              return ""
 -        string = OpenSSL.crypto.dump_certificate(OpenSSL.crypto.FILETYPE_PEM, self.x509)
 +        string = OpenSSL.crypto.dump_certificate(
 +            OpenSSL.crypto.FILETYPE_PEM, self.x509)
          if PY3 and isinstance(string, bytes):
              string = string.decode()
          if save_parents and self.parent:
      # let's try to make this a little more usable as is makes logs hairy
      # FIXME: Consider adding 'urn:publicid' and 'uuid' back for GENI?
      pretty_fields = ['email']
 +
      def filter_chunk(self, chunk):
          for field in self.pretty_fields:
              if field in chunk:
 -                return " "+chunk
 +                return " " + chunk
  
      def pretty_cert(self):
          message = "[Cert."
          x = self.x509.get_subject()
          ou = getattr(x, "OU")
 -        if ou: message += " OU: {}".format(ou)
 +        if ou:
 +            message += " OU: {}".format(ou)
          cn = getattr(x, "CN")
 -        if cn: message += " CN: {}".format(cn)
 +        if cn:
 +            message += " CN: {}".format(cn)
          data = self.get_data(field='subjectAltName')
          if data:
              message += " SubjectAltName:"
              counter = 0
              filtered = [self.filter_chunk(chunk) for chunk in data.split()]
 -            message += " ".join( [f for f in filtered if f])
 +            message += " ".join([f for f in filtered if f])
              omitted = len([f for f in filtered if not f])
              if omitted:
                  message += "..+{} omitted".format(omitted)
          message += "]"
          return message
  
+     def pretty_chain(self):
+         message = "{}".format(self.x509.get_subject())
+         parent = self.parent
+         while parent:
+             message += " -> {}".format(parent.x509.get_subject())
+             parent = parent.parent
+         return message
+     def pretty_name(self):
+         return self.get_filename() or self.pretty_chain()
      ##
      # Get the public key of the certificate.
      #
          else:
              self.add_extension('basicConstraints', 1, 'CA:FALSE')
  
 -
 -
      ##
      # Add an X509 extension to the certificate. Add_extension can only be called
      # once for a particular extension name, due to limitations in the underlying
      # @param value string containing value of the extension
  
      def add_extension(self, name, critical, value):
-         import M2Crypto
          oldExtVal = None
          try:
              oldExtVal = self.get_extension(name)
          return self.data[field]
  
      ##
 -    # Sign the certificate using the issuer private key and issuer subject previous set with set_issuer().
 +    # Sign the certificate using the issuer private key and issuer subject
 +    # previous set with set_issuer().
  
      def sign(self):
          logger.debug('certificate.sign')
          m2x509 = M2Crypto.X509.load_cert_string(self.save_to_string())
          m2pubkey = pubkey.get_m2_pubkey()
          # verify it
-         # verify returns -1 or 0 on failure depending on how serious the
-         # error conditions are
-         return m2x509.verify(m2pubkey) == 1
+         # https://www.openssl.org/docs/man1.1.0/crypto/X509_verify.html
+         # verify returns
+         # 1  if it checks out
+         # 0  if if does not
+         # -1 if it could not be checked 'for some reason'
+         m2result = m2x509.verify(m2pubkey)
+         result = m2result == 1
+         if debug_verify_chain:
+             logger.debug("Certificate.verify: <- {} (m2={}) ({} x {})"
+                          .format(result, m2result, self.pretty_cert(), m2pubkey))
+         return result
  
          # XXX alternatively, if openssl has been patched, do the much simpler:
          # try:
      # @param cert certificate object
  
      def is_signed_by_cert(self, cert):
+         logger.debug("Certificate.is_signed_by_cert -> invoking verify")
          k = cert.get_pubkey()
          result = self.verify(k)
          return result
      # @param Trusted_certs is a list of certificates that are trusted.
      #
  
 -    def verify_chain(self, trusted_certs = None):
 +    def verify_chain(self, trusted_certs=None):
          # Verify a chain of certificates. Each certificate must be signed by
          # the public key contained in it's parent. The chain is recursed
          # until a certificate is found that is signed by a trusted root.
  
+         logger.debug("Certificate.verify_chain {}".format(self.pretty_name()))
          # verify expiration time
          if self.x509.has_expired():
              if debug_verify_chain:
              raise CertExpired(self.pretty_cert(), "client cert")
  
          # if this cert is signed by a trusted_cert, then we are set
-         for trusted_cert in trusted_certs:
+         for i, trusted_cert in enumerate(trusted_certs, 1):
+             logger.debug("Certificate.verify_chain - trying trusted #{} : {}"
+                          .format(i, trusted_cert.pretty_name()))
              if self.is_signed_by_cert(trusted_cert):
                  # verify expiration of trusted_cert ?
                  if not trusted_cert.x509.has_expired():
                      if debug_verify_chain:
                          logger.debug("verify_chain: YES. Cert {} signed by trusted cert {}"
-                                      .format(self.pretty_cert(), trusted_cert.pretty_cert()))
+                                      .format(self.pretty_name(), trusted_cert.pretty_name()))
                      return trusted_cert
                  else:
                      if debug_verify_chain:
                          logger.debug("verify_chain: NO. Cert {} is signed by trusted_cert {}, "
                                       "but that signer is expired..."
 -                                     .format(self.pretty_name(),trusted_cert.pretty_name()))
 +                                     .format(self.pretty_cert(), trusted_cert.pretty_cert()))
                      raise CertExpired("{} signer trusted_cert {}"
-                                       .format(self.pretty_cert(), trusted_cert.pretty_cert()))
+                                       .format(self.pretty_name(), trusted_cert.pretty_name()))
+             else:
+                 logger.debug("verify_chain: not a direct descendant of a trusted root".
+                              format(self.pretty_name(), trusted_cert))
  
          # if there is no parent, then no way to verify the chain
          if not self.parent:
              if debug_verify_chain:
                  logger.debug("verify_chain: NO. {} has no parent "
                               "and issuer {} is not in {} trusted roots"
-                              .format(self.pretty_cert(), self.get_issuer(), len(trusted_certs)))
+                              .format(self.pretty_name(), self.get_issuer(), len(trusted_certs)))
              raise CertMissingParent("{}: Issuer {} is not one of the {} trusted roots, "
                                      "and cert has no parent."
-                                     .format(self.pretty_cert(), self.get_issuer(), len(trusted_certs)))
+                                     .format(self.pretty_name(), self.get_issuer(), len(trusted_certs)))
  
          # if it wasn't signed by the parent...
          if not self.is_signed_by_cert(self.parent):
              if debug_verify_chain:
-                 logger.debug("verify_chain: NO. {} is not signed by parent {}, but by {}"
-                              .format(self.pretty_cert(),
-                                      self.parent.pretty_cert(),
-                                      self.get_issuer()))
+                 logger.debug("verify_chain: NO. {} is not signed by parent {}"
+                              .format(self.pretty_name(),
+                                      self.parent.pretty_name()))
+                 self.save_to_file("/tmp/xxx-capture.pem", save_parents=True)
              raise CertNotSignedByParent("{}: Parent {}, issuer {}"
-                                         .format(self.pretty_cert(),
-                                                 self.parent.pretty_cert(),
+                                         .format(self.pretty_name(),
+                                                 self.parent.pretty_name(),
                                                  self.get_issuer()))
  
          # Confirm that the parent is a CA. Only CAs can be trusted as
          # extension and hope there are no other basicConstraints
          if not self.parent.isCA and not (self.parent.get_extension('basicConstraints') == 'CA:TRUE'):
              logger.warn("verify_chain: cert {}'s parent {} is not a CA"
-                         .format(self.pretty_cert(), self.parent.pretty_cert()))
+                         .format(self.pretty_name(), self.parent.pretty_name()))
              raise CertNotSignedByParent("{}: Parent {} not a CA"
-                                         .format(self.pretty_cert(), self.parent.pretty_cert()))
+                                         .format(self.pretty_name(), self.parent.pretty_name()))
  
          # if the parent isn't verified...
          if debug_verify_chain:
              logger.debug("verify_chain: .. {}, -> verifying parent {}"
-                          .format(self.pretty_cert(), self.parent.pretty_cert()))
+                          .format(self.pretty_name(),self.parent.pretty_name()))
          self.parent.verify_chain(trusted_certs)
  
          return
  
 -    ### more introspection
 +    # more introspection
      def get_extensions(self):
          import M2Crypto
          # pyOpenSSL does not have a way to get extensions
          logger.debug("X509 had {} extensions".format(nb_extensions))
          for i in range(nb_extensions):
              ext = m2x509.get_ext_at(i)
 -            triples.append( (ext.get_name(), ext.get_value(), ext.get_critical(),) )
 +            triples.append(
 +                (ext.get_name(), ext.get_value(), ext.get_critical(),))
          return triples
  
      def get_data_names(self):
      def get_all_datas(self):
          triples = self.get_extensions()
          for name in self.get_data_names():
 -            triples.append( (name,self.get_data(name),'data',) )
 +            triples.append((name, self.get_data(name), 'data',))
          return triples
  
      # only informative
      def get_filename(self):
 -        return getattr(self,'filename',None)
 +        return getattr(self, 'filename', None)
  
      def dump(self, *args, **kwargs):
          print(self.dump_string(*args, **kwargs))
diff --combined sfa/trust/credential.py
  # The above copyright notice and this permission notice shall be
  # included in all copies or substantial portions of the Work.
  #
 -# THE WORK IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS 
 -# OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
 -# MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND 
 -# NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT 
 -# HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, 
 -# WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, 
 -# OUT OF OR IN CONNECTION WITH THE WORK OR THE USE OR OTHER DEALINGS 
 +# THE WORK IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
 +# OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
 +# MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
 +# NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
 +# HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
 +# WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
 +# OUT OF OR IN CONNECTION WITH THE WORK OR THE USE OR OTHER DEALINGS
  # IN THE WORK.
  #----------------------------------------------------------------------
  ##
@@@ -28,8 -28,7 +28,8 @@@
  
  from __future__ import print_function
  
 -import os, os.path
 +import os
 +import os.path
  import subprocess
  import datetime
  from tempfile import mkstemp
@@@ -53,7 -52,7 +53,7 @@@ from sfa.trust.rights import Right, Rig
  from sfa.trust.gid import GID
  from sfa.util.xrn import urn_to_hrn, hrn_authfor_hrn
  
 -# 31 days, in seconds 
 +# 31 days, in seconds
  DEFAULT_CREDENTIAL_LIFETIME = 86400 * 31
  
  
@@@ -63,7 -62,7 +63,7 @@@
  # . add namespaces to signed-credential element?
  
  signature_format = \
 -'''
 +    '''
  <Signature xml:id="Sig_{refid}" xmlns="http://www.w3.org/2000/09/xmldsig#">
    <SignedInfo>
      <CanonicalizationMethod Algorithm="http://www.w3.org/TR/2001/REC-xml-c14n-20010315"/>
  ##
  # Convert a string into a bool
  # used to convert an xsd:boolean to a Python boolean
 +
 +
  def str2bool(str):
 -    if str.lower() in ['true','1']:
 +    if str.lower() in ('true', '1'):
          return True
      return False
  
  
  def getTextNode(element, subele):
      sub = element.getElementsByTagName(subele)[0]
 -    if len(sub.childNodes) > 0:            
 +    if len(sub.childNodes) > 0:
          return sub.childNodes[0].nodeValue
      else:
          return None
 -        
 +
  ##
  # Utility function to set the text of an XML element
  # It creates the element, adds the text to it,
  # and then appends it to the parent.
  
 +
  def append_sub(doc, parent, element, text):
      ele = doc.createElement(element)
      ele.appendChild(doc.createTextNode(text))
  # for a signed-credential
  #
  
 +
  class Signature(object):
 -   
 +
      def __init__(self, string=None):
          self.refid = None
          self.issuer_gid = None
              self.xml = string
              self.decode()
  
 -
      def get_refid(self):
          if not self.refid:
              self.decode()
      def get_issuer_gid(self):
          if not self.gid:
              self.decode()
 -        return self.gid        
 +        return self.gid
  
      def set_issuer_gid(self, gid):
          self.gid = gid
  
      def decode(self):
 -        # Helper function to pull characters off the front of a string if present
 +        # Helper function to pull characters off the front of a string if
 +        # present
          def remove_prefix(text, prefix):
              if text and prefix and text.startswith(prefix):
                  return text[len(prefix):]
              logger.log_exc("Failed to parse credential, {}".format(self.xml))
              raise
          sig = doc.getElementsByTagName("Signature")[0]
 -        ## This code until the end of function rewritten by Aaron Helsinger
 +        # This code until the end of function rewritten by Aaron Helsinger
          ref_id = remove_prefix(sig.getAttribute("xml:id").strip(), "Sig_")
 -        # The xml:id tag is optional, and could be in a 
 +        # The xml:id tag is optional, and could be in a
          # Reference xml:id or Reference UID sub element instead
          if not ref_id or ref_id == '':
              reference = sig.getElementsByTagName('Reference')[0]
 -            ref_id = remove_prefix(reference.getAttribute('xml:id').strip(), "Sig_")
 +            ref_id = remove_prefix(
 +                reference.getAttribute('xml:id').strip(), "Sig_")
              if not ref_id or ref_id == '':
 -                ref_id = remove_prefix(reference.getAttribute('URI').strip(), "#")
 +                ref_id = remove_prefix(
 +                    reference.getAttribute('URI').strip(), "#")
          self.set_refid(ref_id)
          keyinfos = sig.getElementsByTagName("X509Data")
          gids = None
                  if len(cert.childNodes) > 0:
                      szgid = cert.childNodes[0].nodeValue
                      szgid = szgid.strip()
 -                    szgid = "-----BEGIN CERTIFICATE-----\n{}\n-----END CERTIFICATE-----".format(szgid)
 +                    szgid = "-----BEGIN CERTIFICATE-----\n{}\n-----END CERTIFICATE-----".format(
 +                        szgid)
                      if gids is None:
                          gids = szgid
                      else:
                          gids += "\n" + szgid
          if gids is None:
 -            raise CredentialNotVerifiable("Malformed XML: No certificate found in signature")
 +            raise CredentialNotVerifiable(
 +                "Malformed XML: No certificate found in signature")
          self.set_issuer_gid(GID(string=gids))
 -        
 +
      def encode(self):
          self.xml = signature_format.format(refid=self.get_refid())
  
  # A credential provides a caller gid with privileges to an object gid.
  # A signed credential is signed by the object's authority.
  #
 -# Credentials are encoded in one of two ways. 
 +# Credentials are encoded in one of two ways.
  # The legacy style (now unsupported) places it in the subjectAltName of an X509 certificate.
  # The new credentials are placed in signed XML.
  #
  # WARNING:
  # In general, a signed credential obtained externally should
  # not be changed else the signature is no longer valid.  So, once
 -# you have loaded an existing signed credential, do not call encode() or sign() on it.
 +# you have loaded an existing signed credential, do not call encode() or
 +# sign() on it.
 +
  
  def filter_creds_by_caller(creds, caller_hrn_list):
 -        """
 -        Returns a list of creds who's gid caller matches the
 -        specified caller hrn
 -        """
 -        if not isinstance(creds, list): creds = [creds]
 -        if not isinstance(caller_hrn_list, list): 
 -            caller_hrn_list = [caller_hrn_list]
 -        caller_creds = []
 -        for cred in creds:
 -            try:
 -                tmp_cred = Credential(string=cred)
 -                if tmp_cred.type != Credential.SFA_CREDENTIAL_TYPE:
 -                    continue
 -                if tmp_cred.get_gid_caller().get_hrn() in caller_hrn_list:
 -                    caller_creds.append(cred)
 -            except: pass
 -        return caller_creds
 +    """
 +    Returns a list of creds who's gid caller matches the
 +    specified caller hrn
 +    """
 +    if not isinstance(creds, list):
 +        creds = [creds]
 +    if not isinstance(caller_hrn_list, list):
 +        caller_hrn_list = [caller_hrn_list]
 +    caller_creds = []
 +    for cred in creds:
 +        try:
 +            tmp_cred = Credential(string=cred)
 +            if tmp_cred.type != Credential.SFA_CREDENTIAL_TYPE:
 +                continue
 +            if tmp_cred.get_gid_caller().get_hrn() in caller_hrn_list:
 +                caller_creds.append(cred)
 +        except:
 +            pass
 +    return caller_creds
 +
  
  class Credential(object):
  
                  self.version = cred['geni_version']
  
          if string or filename:
 -            if string:                
 +            if string:
                  str = string
              elif filename:
                  with open(filename) as infile:
                      str = infile.read()
 -                
 +
              # if this is a legacy credential, write error and bail out
              if isinstance(str, StringType) and str.strip().startswith("-----"):
 -                logger.error("Legacy credentials not supported any more - giving up with {}...".format(str[:10]))
 +                logger.error(
 +                    "Legacy credentials not supported any more - giving up with {}...".format(str[:10]))
                  return
              else:
                  self.xml = str
          if not getattr(Credential, 'xmlsec1_path', None):
              # Find a xmlsec1 binary path
              Credential.xmlsec1_path = ''
 -            paths = ['/usr/bin', '/usr/local/bin', '/bin', '/opt/bin', '/opt/local/bin']
 -            try:     paths += os.getenv('PATH').split(':')
 -            except:  pass
 +            paths = ['/usr/bin', '/usr/local/bin',
 +                     '/bin', '/opt/bin', '/opt/local/bin']
 +            try:
 +                paths += os.getenv('PATH').split(':')
 +            except:
 +                pass
              for path in paths:
                  xmlsec1 = os.path.join(path, 'xmlsec1')
                  if os.path.isfile(xmlsec1):
                      Credential.xmlsec1_path = xmlsec1
                      break
          if not Credential.xmlsec1_path:
 -            logger.error("Could not locate required binary 'xmlsec1' - SFA will be unable to sign stuff !!")
 +            logger.error(
 +                "Could not locate required binary 'xmlsec1' - SFA will be unable to sign stuff !!")
          return Credential.xmlsec1_path
  
      def get_subject(self):
      def set_signature(self, sig):
          self.signature = sig
  
 -        
      ##
      # Need the issuer's private key and name
      # @param key Keypair object containing the private key of the issuer
          self.issuer_privkey = privkey
          self.issuer_gid = gid
  
 -
      ##
      # Set this credential's parent
      def set_parent(self, cred):
          if not self.gidObject:
              self.decode()
          return self.gidObject
 -            
 +
      ##
      # Expiration: an absolute UTC time of expiration (as either an int or string or datetime)
 -    # 
 +    #
      def set_expiration(self, expiration):
          expiration_datetime = utcparse(expiration)
          if expiration_datetime is not None:
              self.expiration = expiration_datetime
          else:
 -            logger.error("unexpected input {} in Credential.set_expiration".format(expiration))
 +            logger.error(
 +                "unexpected input {} in Credential.set_expiration".format(expiration))
  
      ##
      # get the lifetime of the credential (always in datetime format)
      def get_expiration(self):
          if not self.expiration:
              self.decode()
 -        # at this point self.expiration is normalized as a datetime - DON'T call utcparse again
 +        # at this point self.expiration is normalized as a datetime - DON'T
 +        # call utcparse again
          return self.expiration
  
      ##
  
      def set_privileges(self, privs):
          if isinstance(privs, str):
 -            self.privileges = Rights(string = privs)
 +            self.privileges = Rights(string=privs)
          else:
 -            self.privileges = privs        
 +            self.privileges = privs
  
      ##
      # return the privileges as a Rights object
  
      def can_perform(self, op_name):
          rights = self.get_privileges()
 -        
 +
          if not rights:
              return False
  
          return rights.can_perform(op_name)
  
 -
      ##
 -    # Encode the attributes of the credential into an XML string    
 -    # This should be done immediately before signing the credential.    
 +    # Encode the attributes of the credential into an XML string
 +    # This should be done immediately before signing the credential.
      # WARNING:
      # In general, a signed credential obtained externally should
      # not be changed else the signature is no longer valid.  So, once
 -    # you have loaded an existing signed credential, do not call encode() or sign() on it.
 +    # you have loaded an existing signed credential, do not call encode() or
 +    # sign() on it.
  
      def encode(self):
          # Create the XML document
          # Note that delegation of credentials between the 2 only really works
          # cause those schemas are identical.
          # Also note these PG schemas talk about PG tickets and CM policies.
 -        signed_cred.setAttribute("xmlns:xsi", "http://www.w3.org/2001/XMLSchema-instance")
 -        # FIXME: See v2 schema at www.geni.net/resources/credential/2/credential.xsd
 -        signed_cred.setAttribute("xsi:noNamespaceSchemaLocation", "http://www.planet-lab.org/resources/sfa/credential.xsd")
 -        signed_cred.setAttribute("xsi:schemaLocation", "http://www.planet-lab.org/resources/sfa/ext/policy/1 http://www.planet-lab.org/resources/sfa/ext/policy/1/policy.xsd")
 +        signed_cred.setAttribute(
 +            "xmlns:xsi", "http://www.w3.org/2001/XMLSchema-instance")
 +        # FIXME: See v2 schema at
 +        # www.geni.net/resources/credential/2/credential.xsd
 +        signed_cred.setAttribute("xsi:noNamespaceSchemaLocation",
 +                                 "http://www.planet-lab.org/resources/sfa/credential.xsd")
 +        signed_cred.setAttribute(
 +            "xsi:schemaLocation", "http://www.planet-lab.org/resources/sfa/ext/policy/1 http://www.planet-lab.org/resources/sfa/ext/policy/1/policy.xsd")
  
          # PG says for those last 2:
          #        signed_cred.setAttribute("xsi:noNamespaceSchemaLocation", "http://www.protogeni.net/resources/credential/credential.xsd")
          #        signed_cred.setAttribute("xsi:schemaLocation", "http://www.protogeni.net/resources/credential/ext/policy/1 http://www.protogeni.net/resources/credential/ext/policy/1/policy.xsd")
  
 -        doc.appendChild(signed_cred)  
 -        
 -        # Fill in the <credential> bit        
 +        doc.appendChild(signed_cred)
 +
 +        # Fill in the <credential> bit
          cred = doc.createElement("credential")
          cred.setAttribute("xml:id", self.get_refid())
          signed_cred.appendChild(cred)
          append_sub(doc, cred, "target_urn", self.gidObject.get_urn())
          append_sub(doc, cred, "uuid", "")
          if not self.expiration:
 -            logger.debug("Creating credential valid for {} s".format(DEFAULT_CREDENTIAL_LIFETIME))
 -            self.set_expiration(datetime.datetime.utcnow() + datetime.timedelta(seconds=DEFAULT_CREDENTIAL_LIFETIME))
 +            logger.debug("Creating credential valid for {} s".format(
 +                DEFAULT_CREDENTIAL_LIFETIME))
 +            self.set_expiration(datetime.datetime.utcnow(
 +            ) + datetime.timedelta(seconds=DEFAULT_CREDENTIAL_LIFETIME))
          self.expiration = self.expiration.replace(microsecond=0)
          if self.expiration.tzinfo is not None and self.expiration.tzinfo.utcoffset(self.expiration) is not None:
              # TZ aware. Make sure it is UTC - by Aaron Helsinger
              self.expiration = self.expiration.astimezone(tz.tzutc())
 -        append_sub(doc, cred, "expires", self.expiration.strftime(SFATIME_FORMAT))
 +        append_sub(doc, cred, "expires",
 +                   self.expiration.strftime(SFATIME_FORMAT))
          privileges = doc.createElement("privileges")
          cred.appendChild(privileges)
  
              for right in rights.rights:
                  priv = doc.createElement("privilege")
                  append_sub(doc, priv, "name", right.kind)
 -                append_sub(doc, priv, "can_delegate", str(right.delegate).lower())
 +                append_sub(doc, priv, "can_delegate",
 +                           str(right.delegate).lower())
                  privileges.appendChild(priv)
  
          # Add the parent credential if it exists
                      attr = parentRoot.attributes.item(attrIx)
                      # returns the old attribute of same name that was
                      # on the credential
 -                    # Below throws InUse exception if we forgot to clone the attribute first
 -                    oldAttr = signed_cred.setAttributeNode(attr.cloneNode(True))
 +                    # Below throws InUse exception if we forgot to clone the
 +                    # attribute first
 +                    oldAttr = signed_cred.setAttributeNode(
 +                        attr.cloneNode(True))
                      if oldAttr and oldAttr.value != attr.value:
                          msg = "Delegating cred from owner {} to {} over {}:\n"
                          "- Replaced attribute {} value '{}' with '{}'"\
                          logger.warn(msg)
                          #raise CredentialNotVerifiable("Can't encode new valid delegated credential: {}".format(msg))
  
 -            p_cred = doc.importNode(sdoc.getElementsByTagName("credential")[0], True)
 +            p_cred = doc.importNode(
 +                sdoc.getElementsByTagName("credential")[0], True)
              p = doc.createElement("parent")
              p.appendChild(p_cred)
              cred.appendChild(p)
          if self.parent:
              for cur_cred in self.get_credential_list()[1:]:
                  sdoc = parseString(cur_cred.get_signature().get_xml())
 -                ele = doc.importNode(sdoc.getElementsByTagName("Signature")[0], True)
 +                ele = doc.importNode(
 +                    sdoc.getElementsByTagName("Signature")[0], True)
                  signatures.appendChild(ele)
 -                
 +
          # Get the finished product
          self.xml = doc.toxml("utf-8")
  
 -
 -    def save_to_random_tmp_file(self):       
 +    def save_to_random_tmp_file(self):
          fp, filename = mkstemp(suffix='cred', text=True)
          fp = os.fdopen(fp, "w")
          self.save_to_file(filename, save_parents=True, filep=fp)
          return filename
 -    
 +
      def save_to_file(self, filename, save_parents=True, filep=None):
          if not self.xml:
              self.encode()
          if filep:
 -            f = filep 
 +            f = filep
          else:
              f = open(filename, "w")
          if PY3 and isinstance(self.xml, bytes):
      # Figure out what refids exist, and update this credential's id
      # so that it doesn't clobber the others.  Returns the refids of
      # the parents.
 -    
 +
      def updateRefID(self):
          if not self.parent:
              self.set_refid('ref0')
              return []
 -        
 +
          refs = []
  
          next_cred = self.parent
              else:
                  next_cred = None
  
 -        
          # Find a unique refid for this credential
          rid = self.get_refid()
          while rid in refs:
      # WARNING:
      # In general, a signed credential obtained externally should
      # not be changed else the signature is no longer valid.  So, once
 -    # you have loaded an existing signed credential, do not call encode() or sign() on it.
 +    # you have loaded an existing signed credential, do not call encode() or
 +    # sign() on it.
  
      def sign(self):
          if not self.issuer_privkey:
          # Create the signature template to be signed
          signature = Signature()
          signature.set_refid(self.get_refid())
 -        sdoc = parseString(signature.get_xml())        
 -        sig_ele = doc.importNode(sdoc.getElementsByTagName("Signature")[0], True)
 +        sdoc = parseString(signature.get_xml())
 +        sig_ele = doc.importNode(
 +            sdoc.getElementsByTagName("Signature")[0], True)
          sigs.appendChild(sig_ele)
  
          self.xml = doc.toxml("utf-8")
  
 -
          # Split the issuer GID into multiple certificates if it's a chain
          chain = GID(filename=self.issuer_gid)
          gid_files = []
              else:
                  chain = None
  
 -
          # Call out to xmlsec1 to sign it
          ref = 'Sig_{}'.format(self.get_refid())
          filename = self.save_to_random_tmp_file()
          self.xml = signed
  
          # Update signatures
 -        self.decode()       
 -
 +        self.decode()
  
      ##
      # Retrieve the attributes of the credential from the XML.
                  sigs = signatures[0].getElementsByTagName("Signature")
          else:
              creds = doc.getElementsByTagName("credential")
 -        
 +
          if creds is None or len(creds) == 0:
              # malformed cred file
 -            raise CredentialNotVerifiable("Malformed XML: No credential tag found")
 +            raise CredentialNotVerifiable(
 +                "Malformed XML: No credential tag found")
  
          # Just take the first cred if there are more than one
          cred = creds[0]
          self.gidCaller = GID(string=getTextNode(cred, "owner_gid"))
          self.gidObject = GID(string=getTextNode(cred, "target_gid"))
  
 -
 -        ## This code until the end of function rewritten by Aaron Helsinger
 +        # This code until the end of function rewritten by Aaron Helsinger
          # Process privileges
          rlist = Rights()
          priv_nodes = cred.getElementsByTagName("privileges")
                  if kind == '*':
                      # Convert * into the default privileges for the credential's type
                      # Each inherits the delegatability from the * above
 -                    _ , type = urn_to_hrn(self.gidObject.get_urn())
 +                    _, type = urn_to_hrn(self.gidObject.get_urn())
                      rl = determine_rights(type, self.gidObject.get_urn())
                      for r in rl.rights:
                          r.delegate = deleg
                      rlist.add(Right(kind.strip(), deleg))
          self.set_privileges(rlist)
  
 -
          # Is there a parent?
          parent = cred.getElementsByTagName("parent")
          if len(parent) > 0:
              parent_doc = parent[0].getElementsByTagName("credential")[0]
              parent_xml = parent_doc.toxml("utf-8")
              if parent_xml is None or parent_xml.strip() == "":
 -                raise CredentialNotVerifiable("Malformed XML: Had parent tag but it is empty")
 +                raise CredentialNotVerifiable(
 +                    "Malformed XML: Had parent tag but it is empty")
              self.parent = Credential(string=parent_xml)
              self.updateRefID()
  
              for cur_cred in self.get_credential_list():
                  if cur_cred.get_refid() == Sig.get_refid():
                      cur_cred.set_signature(Sig)
 -                                    
 -            
 +
      ##
      # Verify
 -    #   trusted_certs: A list of trusted GID filenames (not GID objects!) 
 +    #   trusted_certs: A list of trusted GID filenames (not GID objects!)
      #                  Chaining is not supported within the GIDs by xmlsec1.
      #
      #   trusted_certs_required: Should usually be true. Set False means an
      #                 empty list of trusted_certs would still let this method pass.
      #                 It just skips xmlsec1 verification et al. Only used by some utils
 -    #    
 +    #
      # Verify that:
      # . All of the signatures are valid and that the issuers trace back
      #   to trusted roots (performed by xmlsec1)
                      trusted_cert_objects.append(GID(filename=f))
                      ok_trusted_certs.append(f)
                  except Exception as exc:
 -                    logger.error("Failed to load trusted cert from {}: {}".format(f, exc))
 +                    logger.error(
 +                        "Failed to load trusted cert from {}: {}".format(f, exc))
              trusted_certs = ok_trusted_certs
  
          # make sure it is not expired
          if self.get_expiration() < datetime.datetime.utcnow():
 -            raise CredentialNotVerifiable("Credential {} expired at {}" \
 +            raise CredentialNotVerifiable("Credential {} expired at {}"
                                            .format(self.pretty_cred(),
                                                    self.expiration.strftime(SFATIME_FORMAT)))
  
          # If caller explicitly passed in None that means skip cert chain validation.
          # - Strange and not typical
          if trusted_certs is not None:
-             # Verify the gids of this cred and of its parents
+             # Verify the caller and object gids of this cred and of its parents
              for cur_cred in self.get_credential_list():
-                 cur_cred.get_gid_object().verify_chain(trusted_cert_objects)
-                 cur_cred.get_gid_caller().verify_chain(trusted_cert_objects)
+                 # check both the caller and the subject 
+                 for gid in cur_cred.get_gid_object(), cur_cred.get_gid_caller():
+                     logger.debug("Credential.verify: verifying chain {}"
+                                  .format(gid.pretty_cert()))
+                     logger.debug("Credential.verify: against trusted {}"
+                                  .format(" ".join(trusted_certs)))
+                     gid.verify_chain(trusted_cert_objects)
+                         
          refs = []
          refs.append("Sig_{}".format(self.get_refid()))
  
              # turns out, with fedora21, there is extra input before this 'OK' thing
              # looks like we're better off just using the exit code - that's what it is made for
              #cert_args = " ".join(['--trusted-pem {}'.format(x) for x in trusted_certs])
 -            #command = '{} --verify --node-id "{}" {} {} 2>&1'.\
 +            # command = '{} --verify --node-id "{}" {} {} 2>&1'.\
              #          format(self.xmlsec_path, ref, cert_args, filename)
              xmlsec1 = self.get_xmlsec1_path()
              if not xmlsec1:
                  raise Exception("Could not locate required 'xmlsec1' program")
 -            command = [ xmlsec1, '--verify', '--node-id', ref ]
 +            command = [xmlsec1, '--verify', '--node-id', ref]
              for trusted in trusted_certs:
 -                command += ["--trusted-pem", trusted ]
 -            command += [ filename ]
 +                command += ["--trusted-pem", trusted]
 +            command += [filename]
              logger.debug("Running " + " ".join(command))
              try:
 -                verified = subprocess.check_output(command, stderr=subprocess.STDOUT)
 +                verified = subprocess.check_output(
 +                    command, stderr=subprocess.STDOUT)
                  logger.debug("xmlsec command returned {}".format(verified))
                  if "OK\n" not in verified:
 -                    logger.warning("WARNING: xmlsec1 seemed to return fine but without a OK in its output")
 +                    logger.warning(
 +                        "WARNING: xmlsec1 seemed to return fine but without a OK in its output")
              except subprocess.CalledProcessError as e:
                  verified = e.output
                  # xmlsec errors have a msg= which is the interesting bit.
                      mstart = mstart + 4
                      mend = verified.find('\\', mstart)
                      msg = verified[mstart:mend]
 -                logger.warning("Credential.verify - failed - xmlsec1 returned {}".format(verified.strip()))
 -                raise CredentialNotVerifiable("xmlsec1 error verifying cred {} using Signature ID {}: {}"\
 +                logger.warning(
 +                    "Credential.verify - failed - xmlsec1 returned {}".format(verified.strip()))
 +                raise CredentialNotVerifiable("xmlsec1 error verifying cred {} using Signature ID {}: {}"
                                                .format(self.pretty_cred(), ref, msg))
          os.remove(filename)
  
          return True
  
      ##
 -    # Creates a list of the credential and its parents, with the root 
 +    # Creates a list of the credential and its parents, with the root
      # (original delegated credential) as the last item in the list
 -    def get_credential_list(self):    
 +    def get_credential_list(self):
          cur_cred = self
          list = []
          while cur_cred:
              else:
                  cur_cred = None
          return list
 -    
 +
      ##
      # Make sure the credential's target gid (a) was signed by or (b)
      # is the same as the entity that signed the original credential,
          if root_cred.get_signature() is None:
              # malformed
              raise CredentialNotVerifiable("Could not verify credential owned by {} for object {}. "
 -                                          "Cred has no signature" \
 +                                          "Cred has no signature"
                                            .format(self.gidCaller.get_urn(), self.gidObject.get_urn()))
  
          root_cred_signer = root_cred.get_signature().get_issuer_gid()
          # If not, remove this.
          #root_target_gid_str = root_target_gid.save_to_string()
          #root_cred_signer_str = root_cred_signer.save_to_string()
 -        #if root_target_gid_str == root_cred_signer_str:
 +        # if root_target_gid_str == root_cred_signer_str:
          #    # cred signer is target, return success
          #    return
  
      # . The privileges must have "can_delegate" set for each delegated privilege
      # . The target gid must be the same between child and parents
      # . The expiry time on the child must be no later than the parent
 -    # . The signer of the child must be the owner of the parent        
 +    # . The signer of the child must be the owner of the parent
      def verify_parent(self, parent_cred):
          # make sure the rights given to the child are a subset of the
          # parents rights (and check delegate bits)
              message = (
                  "Parent cred {} (ref {}) rights {} "
                  " not superset of delegated cred {} (ref {}) rights {}"
 -                .format(parent_cred.pretty_cred(),parent_cred.get_refid(),
 +                .format(parent_cred.pretty_cred(), parent_cred.get_refid(),
                          parent_cred.get_privileges().pretty_rights(),
                          self.pretty_cred(), self.get_refid(),
                          self.get_privileges().pretty_rights()))
              logger.error(message)
 -            logger.error("parent details {}".format(parent_cred.get_privileges().save_to_string()))
 -            logger.error("self details {}".format(self.get_privileges().save_to_string()))
 +            logger.error("parent details {}".format(
 +                parent_cred.get_privileges().save_to_string()))
 +            logger.error("self details {}".format(
 +                self.get_privileges().save_to_string()))
              raise ChildRightsNotSubsetOfParent(message)
  
          # make sure my target gid is the same as the parent's
                  "Delegated cred {}: Target gid not equal between parent and child. Parent {}"
                  .format(self.pretty_cred(), parent_cred.pretty_cred()))
              logger.error(message)
 -            logger.error("parent details {}".format(parent_cred.save_to_string()))
 +            logger.error("parent details {}".format(
 +                parent_cred.save_to_string()))
              logger.error("self details {}".format(self.save_to_string()))
              raise CredentialNotVerifiable(message)
  
              message = "Delegated credential {} not signed by parent {}'s caller"\
                  .format(self.pretty_cred(), parent_cred.pretty_cred())
              logger.error(message)
 -            logger.error("compare1 parent {}".format(parent_cred.get_gid_caller().pretty_cert()))
 -            logger.error("compare1 parent details {}".format(parent_cred.get_gid_caller().save_to_string()))
 -            logger.error("compare2 self {}".format(self.get_signature().get_issuer_gid().pretty_crert()))
 -            logger.error("compare2 self details {}".format(self.get_signature().get_issuer_gid().save_to_string()))
 +            logger.error("compare1 parent {}".format(
 +                parent_cred.get_gid_caller().pretty_cert()))
 +            logger.error("compare1 parent details {}".format(
 +                parent_cred.get_gid_caller().save_to_string()))
 +            logger.error("compare2 self {}".format(
 +                self.get_signature().get_issuer_gid().pretty_crert()))
 +            logger.error("compare2 self details {}".format(
 +                self.get_signature().get_issuer_gid().save_to_string()))
              raise CredentialNotVerifiable(message)
 -                
 +
          # Recurse
          if parent_cred.parent:
              parent_cred.verify_parent(parent_cred.parent)
  
 -
      def delegate(self, delegee_gidfile, caller_keyfile, caller_gidfile):
          """
          Return a delegated copy of this credential, delegated to the 
          """
          # get the gid of the object we are delegating
          object_gid = self.get_gid_object()
 -        object_hrn = object_gid.get_hrn()        
 - 
 +        object_hrn = object_gid.get_hrn()
 +
          # the hrn of the user who will be delegated to
          delegee_gid = GID(filename=delegee_gidfile)
          delegee_hrn = delegee_gid.get_hrn()
 -  
 +
          #user_key = Keypair(filename=keyfile)
          #user_hrn = self.get_gid_caller().get_hrn()
          subject_string = "{} delegated to {}".format(object_hrn, delegee_hrn)
  
      # only informative
      def get_filename(self):
 -        return getattr(self,'filename',None)
 +        return getattr(self, 'filename', None)
  
      def actual_caller_hrn(self):
          """
          """
  
          caller_hrn, caller_type = urn_to_hrn(self.get_gid_caller().get_urn())
 -        issuer_hrn, issuer_type = urn_to_hrn(self.get_signature().get_issuer_gid().get_urn())
 +        issuer_hrn, issuer_type = urn_to_hrn(
 +            self.get_signature().get_issuer_gid().get_urn())
          subject_hrn = self.get_gid_object().get_hrn()
          # if the caller is a user and the issuer is not
          # it's probably the former
          # this seems to be a 'regular' credential
          elif caller_hrn.startswith(issuer_hrn):
              actual_caller_hrn = caller_hrn
 -        # else this looks like a delegated credential, and the real caller is the issuer
 +        # else this looks like a delegated credential, and the real caller is
 +        # the issuer
          else:
              actual_caller_hrn = issuer_hrn
          logger.info("actual_caller_hrn: caller_hrn={}, issuer_hrn={}, returning {}"
  
      # SFA code ignores show_xml and disables printing the cred xml
      def dump_string(self, dump_parents=False, show_xml=False):
 -        result=""
 +        result = ""
          result += "CREDENTIAL {}\n".format(self.pretty_subject())
 -        filename=self.get_filename()
 -        if filename: result += "Filename {}\n".format(filename)
 +        filename = self.get_filename()
 +        if filename:
 +            result += "Filename {}\n".format(filename)
          privileges = self.get_privileges()
          if privileges:
              result += "      privs: {}\n".format(privileges.save_to_string())
              result += self.get_signature().get_issuer_gid().dump_string(8, dump_parents)
  
          if self.expiration:
 -            result += "  expiration: " + self.expiration.strftime(SFATIME_FORMAT) + "\n"
 +            result += "  expiration: " + \
 +                self.expiration.strftime(SFATIME_FORMAT) + "\n"
  
          gidObject = self.get_gid_object()
          if gidObject:
diff --combined sfa/trust/gid.py
  # The above copyright notice and this permission notice shall be
  # included in all copies or substantial portions of the Work.
  #
 -# THE WORK IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS 
 -# OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
 -# MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND 
 -# NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT 
 -# HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, 
 -# WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, 
 -# OUT OF OR IN CONNECTION WITH THE WORK OR THE USE OR OTHER DEALINGS 
 +# THE WORK IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
 +# OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
 +# MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
 +# NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
 +# HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
 +# WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
 +# OUT OF OR IN CONNECTION WITH THE WORK OR THE USE OR OTHER DEALINGS
  # IN THE WORK.
  #----------------------------------------------------------------------
  ##
@@@ -39,7 -39,6 +39,7 @@@ from sfa.util.py23 import xmlrpc_clien
  ##
  # Create a new uuid. Returns the UUID as a string.
  
 +
  def create_uuid():
      return str(uuid.uuid4().int)
  
@@@ -55,7 -54,7 +55,7 @@@
  #
  # URN is a human readable identifier of form:
  #   "urn:publicid:IDN+toplevelauthority[:sub-auth.]*[\res. type]\ +object name"
 -#   For  example, urn:publicid:IDN+planetlab:us:arizona+user+bakers      
 +#   For  example, urn:publicid:IDN+planetlab:us:arizona+user+bakers
  #
  # PUBLIC_KEY is the public key of the principal identified by the UUID/HRN.
  # It is a Keypair object as defined in the cert.py module.
@@@ -84,7 -83,7 +84,7 @@@ class GID(Certificate)
          self.uuid = None
          self.hrn = None
          self.urn = None
 -        self.email = None # for adding to the SubjectAltName             
 +        self.email = None  # for adding to the SubjectAltName
          Certificate.__init__(self, lifeDays, create, subject, string, filename)
  
          if subject:
      def set_urn(self, urn):
          self.urn = urn
          self.hrn, type = urn_to_hrn(urn)
 - 
 +
      def get_urn(self):
          if not self.urn:
              self.decode()
 -        return self.urn            
 +        return self.urn
  
      # Will be stuffed into subjectAltName
      def set_email(self, email):
              self.decode()
          _, t = urn_to_hrn(self.urn)
          return t
 -    
 +
      ##
      # Encode the GID fields and package them into the subject-alt-name field
      # of the X509 certificate. This must be called prior to signing the
              urn = self.urn
          else:
              urn = hrn_to_urn(self.hrn, None)
 -            
 +
          str = "URI:" + urn
  
          if self.uuid:
              str += ", " + "URI:" + uuid.UUID(int=self.uuid).urn
 -        
 +
          if self.email:
              str += ", " + "email:" + self.email
  
          self.set_data(str, 'subjectAltName')
  
 -
      ##
      # Decode the subject-alt-name field of the X509 certificate into the
      # fields of the GID. This is automatically called by the various get_*()
                          # FIXME: Ensure there isn't cruft in that address...
                          # EG look for email:copy,....
                          dict['email'] = val[6:]
 -                    
 +
          self.uuid = dict.get("uuid", None)
          self.urn = dict.get("urn", None)
          self.hrn = dict.get("hrn", None)
      # @param dump_parents If true, also dump the parents of the GID
  
      def dump(self, *args, **kwargs):
 -        print(self.dump_string(*args,**kwargs))
 +        print(self.dump_string(*args, **kwargs))
  
      def dump_string(self, indent=0, dump_parents=False):
 -        result=" "*(indent-2) + "GID\n"
 -        result += " "*indent + "hrn:" + str(self.get_hrn()) +"\n"
 -        result += " "*indent + "urn:" + str(self.get_urn()) +"\n"
 -        result += " "*indent + "uuid:" + str(self.get_uuid()) + "\n"
 +        result = " " * (indent - 2) + "GID\n"
 +        result += " " * indent + "hrn:" + str(self.get_hrn()) + "\n"
 +        result += " " * indent + "urn:" + str(self.get_urn()) + "\n"
 +        result += " " * indent + "uuid:" + str(self.get_uuid()) + "\n"
          if self.get_email() is not None:
 -            result += " "*indent + "email:" + str(self.get_email()) + "\n"
 -        filename=self.get_filename()
 -        if filename: result += "Filename %s\n"%filename
 +            result += " " * indent + "email:" + str(self.get_email()) + "\n"
 +        filename = self.get_filename()
 +        if filename:
 +            result += "Filename %s\n" % filename
  
          if self.parent and dump_parents:
 -            result += " "*indent + "parent:\n"
 -            result += self.parent.dump_string(indent+4, dump_parents)
 +            result += " " * indent + "parent:\n"
 +            result += self.parent.dump_string(indent + 4, dump_parents)
          return result
  
      ##
      # for a principal that is not a member of that authority. For example,
      # planetlab.us.arizona cannot sign a GID for planetlab.us.princeton.foo.
  
 -    def verify_chain(self, trusted_certs = None):
 +    def verify_chain(self, trusted_certs=None):
+         logger.debug("GID.verify_chain with {} trusted certs".format(len(trusted_certs)))
          # do the normal certificate verification stuff
 -        trusted_root = Certificate.verify_chain(self, trusted_certs)        
 -       
 +        trusted_root = Certificate.verify_chain(self, trusted_certs)
 +
          if self.parent:
              # make sure the parent's hrn is a prefix of the child's hrn
              if not hrn_authfor_hrn(self.parent.get_hrn(), self.get_hrn()):
              trusted_gid = GID(string=trusted_root.save_to_string())
              trusted_type = trusted_gid.get_type()
              trusted_hrn = trusted_gid.get_hrn()
 -            #if trusted_type == 'authority':
 +            # if trusted_type == 'authority':
              #    trusted_hrn = trusted_hrn[:trusted_hrn.rindex('.')]
              cur_hrn = self.get_hrn()
              if not hrn_authfor_hrn(trusted_hrn, cur_hrn):