do not depend on types.StringTypes anymore
[sfa.git] / sfa / trust / credential.py
index 109a529..f03e135 100644 (file)
 # Credentials are signed XML files that assign a subject gid privileges to an object gid
 ##
 
 # Credentials are signed XML files that assign a subject gid privileges to an object gid
 ##
 
+from __future__ import print_function
+
 import os, os.path
 import subprocess
 import os, os.path
 import subprocess
-from types import StringTypes
 import datetime
 from StringIO import StringIO
 from tempfile import mkstemp
 from xml.dom.minidom import Document, parseString
 
 import datetime
 from StringIO import StringIO
 from tempfile import mkstemp
 from xml.dom.minidom import Document, parseString
 
+from sfa.util.py23 import StringType
+
 HAVELXML = False
 try:
     from lxml import etree
 HAVELXML = False
 try:
     from lxml import etree
@@ -51,7 +54,7 @@ from sfa.trust.gid import GID
 from sfa.util.xrn import urn_to_hrn, hrn_authfor_hrn
 
 # 31 days, in seconds 
 from sfa.util.xrn import urn_to_hrn, hrn_authfor_hrn
 
 # 31 days, in seconds 
-DEFAULT_CREDENTIAL_LIFETIME = 86400 * 28
+DEFAULT_CREDENTIAL_LIFETIME = 86400 * 31
 
 
 # TODO:
 
 
 # TODO:
@@ -181,17 +184,44 @@ class Signature(object):
         self.gid = gid
 
     def decode(self):
         self.gid = gid
 
     def decode(self):
+        # 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):]
+            return text
+
         try:
             doc = parseString(self.xml)
         try:
             doc = parseString(self.xml)
-        except ExpatError,e:
+        except ExpatError as e:
             logger.log_exc ("Failed to parse credential, %s"%self.xml)
             raise
         sig = doc.getElementsByTagName("Signature")[0]
             logger.log_exc ("Failed to parse credential, %s"%self.xml)
             raise
         sig = doc.getElementsByTagName("Signature")[0]
-        self.set_refid(sig.getAttribute("xml:id").strip("Sig_"))
-        keyinfo = sig.getElementsByTagName("X509Data")[0]
-        szgid = getTextNode(keyinfo, "X509Certificate")
-        szgid = "-----BEGIN CERTIFICATE-----\n%s\n-----END CERTIFICATE-----" % szgid
-        self.set_issuer_gid(GID(string=szgid))        
+        ## 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 
+        # 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_")
+            if not ref_id or ref_id == '':
+                ref_id = remove_prefix(reference.getAttribute('URI').strip(), "#")
+        self.set_refid(ref_id)
+        keyinfos = sig.getElementsByTagName("X509Data")
+        gids = None
+        for keyinfo in keyinfos:
+            certs = keyinfo.getElementsByTagName("X509Certificate")
+            for cert in certs:
+                if len(cert.childNodes) > 0:
+                    szgid = cert.childNodes[0].nodeValue
+                    szgid = szgid.strip()
+                    szgid = "-----BEGIN CERTIFICATE-----\n%s\n-----END CERTIFICATE-----" % szgid
+                    if gids is None:
+                        gids = szgid
+                    else:
+                        gids += "\n" + szgid
+        if gids is None:
+            raise CredentialNotVerifiable("Malformed XML: No certificate found in signature")
+        self.set_issuer_gid(GID(string=gids))
         
     def encode(self):
         self.xml = signature_template % (self.get_refid(), self.get_refid())
         
     def encode(self):
         self.xml = signature_template % (self.get_refid(), self.get_refid())
@@ -221,6 +251,8 @@ def filter_creds_by_caller(creds, caller_hrn_list):
         for cred in creds:
             try:
                 tmp_cred = Credential(string=cred)
         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
                 if tmp_cred.get_gid_caller().get_hrn() in caller_hrn_list:
                     caller_creds.append(cred)
             except: pass
@@ -228,6 +260,8 @@ def filter_creds_by_caller(creds, caller_hrn_list):
 
 class Credential(object):
 
 
 class Credential(object):
 
+    SFA_CREDENTIAL_TYPE = "geni_sfa"
+
     ##
     # Create a Credential object
     #
     ##
     # Create a Credential object
     #
@@ -248,19 +282,18 @@ class Credential(object):
         self.signature = None
         self.xml = None
         self.refid = None
         self.signature = None
         self.xml = None
         self.refid = None
-        self.type = None
+        self.type = Credential.SFA_CREDENTIAL_TYPE
         self.version = None
 
         if cred:
         self.version = None
 
         if cred:
-            if isinstance(cred, StringTypes):
+            if isinstance(cred, StringType):
                 string = cred
                 string = cred
-                self.type = 'geni_sfa'
-                self.version = '1.0'
+                self.type = Credential.SFA_CREDENTIAL_TYPE
+                self.version = '3'
             elif isinstance(cred, dict):
                 string = cred['geni_value']
                 self.type = cred['geni_type']
                 self.version = cred['geni_version']
             elif isinstance(cred, dict):
                 string = cred['geni_value']
                 self.type = cred['geni_type']
                 self.version = cred['geni_version']
-                
 
         if string or filename:
             if string:                
 
         if string or filename:
             if string:                
@@ -269,38 +302,54 @@ class Credential(object):
                 str = file(filename).read()
                 
             # if this is a legacy credential, write error and bail out
                 str = file(filename).read()
                 
             # if this is a legacy credential, write error and bail out
-            if isinstance (str, StringTypes) and str.strip().startswith("-----"):
+            if isinstance (str, StringType) and str.strip().startswith("-----"):
                 logger.error("Legacy credentials not supported any more - giving up with %s..."%str[:10])
                 return
             else:
                 self.xml = str
                 self.decode()
                 logger.error("Legacy credentials not supported any more - giving up with %s..."%str[:10])
                 return
             else:
                 self.xml = str
                 self.decode()
-
-        # Find an xmlsec1 path
-        self.xmlsec_path = ''
-        paths = ['/usr/bin','/usr/local/bin','/bin','/opt/bin','/opt/local/bin']
-        for path in paths:
-            if os.path.isfile(path + '/' + 'xmlsec1'):
-                self.xmlsec_path = path + '/' + 'xmlsec1'
-                break
+        # not strictly necessary but won't hurt either
+        self.get_xmlsec1_path()
+
+    @staticmethod
+    def get_xmlsec1_path():
+        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
+            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 !!")
+        return Credential.xmlsec1_path
 
     def get_subject(self):
 
     def get_subject(self):
+        if not self.gidObject:
+            self.decode()
+        return self.gidObject.get_subject()
+
+    def pretty_subject(self):
         subject = ""
         if not self.gidObject:
             self.decode()
         if self.gidObject:
         subject = ""
         if not self.gidObject:
             self.decode()
         if self.gidObject:
-            subject = self.gidObject.get_printable_subject()
+            subject = self.gidObject.pretty_cert()
         return subject
 
     # sounds like this should be __repr__ instead ??
         return subject
 
     # sounds like this should be __repr__ instead ??
-    def get_summary_tostring(self):
+    def pretty_cred(self):
         if not self.gidObject:
             self.decode()
         if not self.gidObject:
             self.decode()
-        obj = self.gidObject.get_printable_subject()
-        caller = self.gidCaller.get_printable_subject()
+        obj = self.gidObject.pretty_cert()
+        caller = self.gidCaller.pretty_cert()
         exp = self.get_expiration()
         # Summarize the rights too? The issuer?
         exp = self.get_expiration()
         # Summarize the rights too? The issuer?
-        return "[ Grant %s rights on %s until %s ]" % (caller, obj, exp)
+        return "[Cred. for {caller} rights on {obj} until {exp} ]".format(**locals())
 
     def get_signature(self):
         if not self.signature:
 
     def get_signature(self):
         if not self.signature:
@@ -434,12 +483,13 @@ class Credential(object):
         # 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")
         # 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")
 
         # PG says for those last 2:
         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")
+        #        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)  
         
 
         doc.appendChild(signed_cred)  
         
@@ -458,6 +508,9 @@ class Credential(object):
             logger.debug("Creating credential valid for %s s"%DEFAULT_CREDENTIAL_LIFETIME)
             self.set_expiration(datetime.datetime.utcnow() + datetime.timedelta(seconds=DEFAULT_CREDENTIAL_LIFETIME))
         self.expiration = self.expiration.replace(microsecond=0)
             logger.debug("Creating credential valid for %s s"%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))
         privileges = doc.createElement("privileges")
         cred.appendChild(privileges)
         append_sub(doc, cred, "expires", self.expiration.strftime(SFATIME_FORMAT))
         privileges = doc.createElement("privileges")
         cred.appendChild(privileges)
@@ -536,7 +589,7 @@ class Credential(object):
                 signatures.appendChild(ele)
                 
         # Get the finished product
                 signatures.appendChild(ele)
                 
         # Get the finished product
-        self.xml = doc.toxml()
+        self.xml = doc.toxml("utf-8")
 
 
     def save_to_random_tmp_file(self):       
 
 
     def save_to_random_tmp_file(self):       
@@ -615,7 +668,11 @@ class Credential(object):
     # you have loaded an existing signed credential, do not call encode() or sign() on it.
 
     def sign(self):
     # you have loaded an existing signed credential, do not call encode() or sign() on it.
 
     def sign(self):
-        if not self.issuer_privkey or not self.issuer_gid:
+        if not self.issuer_privkey:
+            logger.warn("Cannot sign credential (no private key)")
+            return
+        if not self.issuer_gid:
+            logger.warn("Cannot sign credential (no issuer gid)")
             return
         doc = parseString(self.get_xml())
         sigs = doc.getElementsByTagName("signatures")[0]
             return
         doc = parseString(self.get_xml())
         sigs = doc.getElementsByTagName("signatures")[0]
@@ -627,7 +684,7 @@ class Credential(object):
         sig_ele = doc.importNode(sdoc.getElementsByTagName("Signature")[0], True)
         sigs.appendChild(sig_ele)
 
         sig_ele = doc.importNode(sdoc.getElementsByTagName("Signature")[0], True)
         sigs.appendChild(sig_ele)
 
-        self.xml = doc.toxml()
+        self.xml = doc.toxml("utf-8")
 
 
         # Split the issuer GID into multiple certificates if it's a chain
 
 
         # Split the issuer GID into multiple certificates if it's a chain
@@ -644,8 +701,13 @@ class Credential(object):
         # Call out to xmlsec1 to sign it
         ref = 'Sig_%s' % self.get_refid()
         filename = self.save_to_random_tmp_file()
         # Call out to xmlsec1 to sign it
         ref = 'Sig_%s' % self.get_refid()
         filename = self.save_to_random_tmp_file()
-        signed = os.popen('%s --sign --node-id "%s" --privkey-pem %s,%s %s' \
-                 % (self.xmlsec_path, ref, self.issuer_privkey, ",".join(gid_files), filename)).read()
+        xmlsec1 = self.get_xmlsec1_path()
+        if not xmlsec1:
+            raise Exception("Could not locate required 'xmlsec1' program")
+        command = '%s --sign --node-id "%s" --privkey-pem %s,%s %s' \
+            % (xmlsec1, ref, self.issuer_privkey, ",".join(gid_files), filename)
+#        print 'command',command
+        signed = os.popen(command).read()
         os.remove(filename)
 
         for gid_file in gid_files:
         os.remove(filename)
 
         for gid_file in gid_files:
@@ -656,7 +718,7 @@ class Credential(object):
         # Update signatures
         self.decode()       
 
         # Update signatures
         self.decode()       
 
-        
+
     ##
     # Retrieve the attributes of the credential from the XML.
     # This is automatically called by the various get_* methods of
     ##
     # Retrieve the attributes of the credential from the XML.
     # This is automatically called by the various get_* methods of
@@ -669,7 +731,7 @@ class Credential(object):
         doc = None
         try:
             doc = parseString(self.xml)
         doc = None
         try:
             doc = parseString(self.xml)
-        except ExpatError,e:
+        except ExpatError as e:
             raise CredentialNotVerifiable("Malformed credential")
         doc = parseString(self.xml)
         sigs = []
             raise CredentialNotVerifiable("Malformed credential")
         doc = parseString(self.xml)
         sigs = []
@@ -694,25 +756,28 @@ class Credential(object):
         self.set_refid(cred.getAttribute("xml:id"))
         self.set_expiration(utcparse(getTextNode(cred, "expires")))
         self.gidCaller = GID(string=getTextNode(cred, "owner_gid"))
         self.set_refid(cred.getAttribute("xml:id"))
         self.set_expiration(utcparse(getTextNode(cred, "expires")))
         self.gidCaller = GID(string=getTextNode(cred, "owner_gid"))
-        self.gidObject = GID(string=getTextNode(cred, "target_gid"))   
+        self.gidObject = GID(string=getTextNode(cred, "target_gid"))
 
 
 
 
+        ## This code until the end of function rewritten by Aaron Helsinger
         # Process privileges
         # Process privileges
-        privs = cred.getElementsByTagName("privileges")[0]
         rlist = Rights()
         rlist = Rights()
-        for priv in privs.getElementsByTagName("privilege"):
-            kind = getTextNode(priv, "name")
-            deleg = str2bool(getTextNode(priv, "can_delegate"))
-            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())
-                rl = determine_rights(type, self.gidObject.get_urn())
-                for r in rl.rights:
-                    r.delegate = deleg
-                    rlist.add(r)
-            else:
-                rlist.add(Right(kind.strip(), deleg))
+        priv_nodes = cred.getElementsByTagName("privileges")
+        if len(priv_nodes) > 0:
+            privs = priv_nodes[0]
+            for priv in privs.getElementsByTagName("privilege"):
+                kind = getTextNode(priv, "name")
+                deleg = str2bool(getTextNode(priv, "can_delegate"))
+                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())
+                    rl = determine_rights(type, self.gidObject.get_urn())
+                    for r in rl.rights:
+                        r.delegate = deleg
+                        rlist.add(r)
+                else:
+                    rlist.add(Right(kind.strip(), deleg))
         self.set_privileges(rlist)
 
 
         self.set_privileges(rlist)
 
 
@@ -720,13 +785,15 @@ class Credential(object):
         parent = cred.getElementsByTagName("parent")
         if len(parent) > 0:
             parent_doc = parent[0].getElementsByTagName("credential")[0]
         parent = cred.getElementsByTagName("parent")
         if len(parent) > 0:
             parent_doc = parent[0].getElementsByTagName("credential")[0]
-            parent_xml = parent_doc.toxml()
+            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")
             self.parent = Credential(string=parent_xml)
             self.updateRefID()
 
         # Assign the signatures to the credentials
         for sig in sigs:
             self.parent = Credential(string=parent_xml)
             self.updateRefID()
 
         # Assign the signatures to the credentials
         for sig in sigs:
-            Sig = Signature(string=sig.toxml())
+            Sig = Signature(string=sig.toxml("utf-8"))
 
             for cur_cred in self.get_credential_list():
                 if cur_cred.get_refid() == Sig.get_refid():
 
             for cur_cred in self.get_credential_list():
                 if cur_cred.get_refid() == Sig.get_refid():
@@ -776,7 +843,7 @@ class Credential(object):
                 xmlschema = etree.XMLSchema(schema_doc)
                 if not xmlschema.validate(tree):
                     error = xmlschema.error_log.last_error
                 xmlschema = etree.XMLSchema(schema_doc)
                 if not xmlschema.validate(tree):
                     error = xmlschema.error_log.last_error
-                    message = "%s: %s (line %s)" % (self.get_summary_tostring(), error.message, error.line)
+                    message = "%s: %s (line %s)" % (self.pretty_cred(), error.message, error.line)
                     raise CredentialNotVerifiable(message)
 
         if trusted_certs_required and trusted_certs is None:
                     raise CredentialNotVerifiable(message)
 
         if trusted_certs_required and trusted_certs is None:
@@ -794,14 +861,14 @@ class Credential(object):
                     # or non PEM files
                     trusted_cert_objects.append(GID(filename=f))
                     ok_trusted_certs.append(f)
                     # or non PEM files
                     trusted_cert_objects.append(GID(filename=f))
                     ok_trusted_certs.append(f)
-                except Exception, exc:
+                except Exception as exc:
                     logger.error("Failed to load trusted cert from %s: %r"%( f, exc))
             trusted_certs = ok_trusted_certs
 
         # make sure it is not expired
         if self.get_expiration() < datetime.datetime.utcnow():
             raise CredentialNotVerifiable("Credential %s expired at %s" % \
                     logger.error("Failed to load trusted cert from %s: %r"%( f, exc))
             trusted_certs = ok_trusted_certs
 
         # make sure it is not expired
         if self.get_expiration() < datetime.datetime.utcnow():
             raise CredentialNotVerifiable("Credential %s expired at %s" % \
-                                          (self.get_summary_tostring(),
+                                          (self.pretty_cred(),
                                            self.expiration.strftime(SFATIME_FORMAT)))
 
         # Verify the signatures
                                            self.expiration.strftime(SFATIME_FORMAT)))
 
         # Verify the signatures
@@ -835,7 +902,10 @@ class Credential(object):
             #cert_args = " ".join(['--trusted-pem %s' % x for x in trusted_certs])
             #command = '{} --verify --node-id "{}" {} {} 2>&1'.\
             #          format(self.xmlsec_path, ref, cert_args, filename)
             #cert_args = " ".join(['--trusted-pem %s' % x for x in trusted_certs])
             #command = '{} --verify --node-id "{}" {} {} 2>&1'.\
             #          format(self.xmlsec_path, ref, cert_args, filename)
-            command = [ self.xmlsec_path, '--verify', '--node-id', ref ]
+            xmlsec1 = self.get_xmlsec1_path()
+            if not xmlsec1:
+                raise Exception("Could not locate required 'xmlsec1' program")
+            command = [ xmlsec1, '--verify', '--node-id', ref ]
             for trusted in trusted_certs:
                 command += ["--trusted-pem", trusted ]
             command += [ filename ]
             for trusted in trusted_certs:
                 command += ["--trusted-pem", trusted ]
             command += [ filename ]
@@ -856,7 +926,7 @@ class Credential(object):
                     msg = verified[mstart:mend]
                 logger.warning("Credential.verify - failed - xmlsec1 returned {}".format(verified.strip()))
                 raise CredentialNotVerifiable("xmlsec1 error verifying cred %s using Signature ID %s: %s" % \
                     msg = verified[mstart:mend]
                 logger.warning("Credential.verify - failed - xmlsec1 returned {}".format(verified.strip()))
                 raise CredentialNotVerifiable("xmlsec1 error verifying cred %s using Signature ID %s: %s" % \
-                                              (self.get_summary_tostring(), ref, msg))
+                                              (self.pretty_cred(), ref, msg))
         os.remove(filename)
 
         # Verify the parents (delegation)
         os.remove(filename)
 
         # Verify the parents (delegation)
@@ -891,6 +961,10 @@ class Credential(object):
     def verify_issuer(self, trusted_gids):
         root_cred = self.get_credential_list()[-1]
         root_target_gid = root_cred.get_gid_object()
     def verify_issuer(self, trusted_gids):
         root_cred = self.get_credential_list()[-1]
         root_target_gid = root_cred.get_gid_object()
+        if root_cred.get_signature() is None:
+            # malformed
+            raise CredentialNotVerifiable("Could not verify credential owned by %s for object %s. Cred has no signature" % (self.gidCaller.get_urn(), self.gidObject.get_urn()))
+
         root_cred_signer = root_cred.get_signature().get_issuer_gid()
 
         # Case 1:
         root_cred_signer = root_cred.get_signature().get_issuer_gid()
 
         # Case 1:
@@ -933,13 +1007,14 @@ class Credential(object):
         if trusted_gids and len(trusted_gids) > 0:
             root_cred_signer.verify_chain(trusted_gids)
         else:
         if trusted_gids and len(trusted_gids) > 0:
             root_cred_signer.verify_chain(trusted_gids)
         else:
-            logger.debug("No trusted gids. Cannot verify that cred signer is signed by a trusted authority. Skipping that check.")
+            logger.debug("Cannot verify that cred signer is signed by a trusted authority. "
+                         "No trusted gids. Skipping that check.")
 
         # See if the signer is an authority over the domain of the target.
         # There are multiple types of authority - accept them all here
         # Maybe should be (hrn, type) = urn_to_hrn(root_cred_signer.get_urn())
         root_cred_signer_type = root_cred_signer.get_type()
 
         # See if the signer is an authority over the domain of the target.
         # There are multiple types of authority - accept them all here
         # Maybe should be (hrn, type) = urn_to_hrn(root_cred_signer.get_urn())
         root_cred_signer_type = root_cred_signer.get_type()
-        if (root_cred_signer_type.find('authority') == 0):
+        if root_cred_signer_type.find('authority') == 0:
             #logger.debug('Cred signer is an authority')
             # signer is an authority, see if target is in authority's domain
             signerhrn = root_cred_signer.get_hrn()
             #logger.debug('Cred signer is an authority')
             # signer is an authority, see if target is in authority's domain
             signerhrn = root_cred_signer.get_hrn()
@@ -954,9 +1029,11 @@ class Credential(object):
 
         # Give up, credential does not pass issuer verification
 
 
         # Give up, credential does not pass issuer verification
 
-        raise CredentialNotVerifiable("Could not verify credential owned by %s for object %s. Cred signer %s not the trusted authority for Cred target %s" % \
-                                      (self.gidCaller.get_urn(), self.gidObject.get_urn(), root_cred_signer.get_hrn(), root_target_gid.get_hrn()))
-
+        raise CredentialNotVerifiable(
+            "Could not verify credential owned by {} for object {}. "
+            "Cred signer {} not the trusted authority for Cred target {}"
+            .format(self.gidCaller.get_hrn(), self.gidObject.get_hrn(),
+                    root_cred_signer.get_hrn(), root_target_gid.get_hrn()))
 
     ##
     # -- For Delegates (credentials with parents) verify that:
 
     ##
     # -- For Delegates (credentials with parents) verify that:
@@ -969,27 +1046,46 @@ class Credential(object):
         # make sure the rights given to the child are a subset of the
         # parents rights (and check delegate bits)
         if not parent_cred.get_privileges().is_superset(self.get_privileges()):
         # make sure the rights given to the child are a subset of the
         # parents rights (and check delegate bits)
         if not parent_cred.get_privileges().is_superset(self.get_privileges()):
-            raise ChildRightsNotSubsetOfParent(("Parent cred ref %s rights " % parent_cred.get_refid()) +
-                self.parent.get_privileges().save_to_string() + (" not superset of delegated cred %s ref %s rights " % \
-                                                                 (self.get_summary_tostring(), self.get_refid())) +
-                self.get_privileges().save_to_string())
+            message = (
+                "Parent cred {} (ref {}) rights {} "
+                " not superset of delegated cred {} (ref {}) rights {}"
+                .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()))
+            raise ChildRightsNotSubsetOfParent(message)
 
         # make sure my target gid is the same as the parent's
         if not parent_cred.get_gid_object().save_to_string() == \
            self.get_gid_object().save_to_string():
 
         # make sure my target gid is the same as the parent's
         if not parent_cred.get_gid_object().save_to_string() == \
            self.get_gid_object().save_to_string():
-            raise CredentialNotVerifiable("Delegated cred %s: Target gid not equal between parent and child. Parent %s" % \
-                                          (self.get_summary_tostring(), parent_cred.get_summary_tostring()))
+            message = (
+                "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("self details {}".format(self.save_to_string()))
+            raise CredentialNotVerifiable(message)
 
         # make sure my expiry time is <= my parent's
         if not parent_cred.get_expiration() >= self.get_expiration():
 
         # make sure my expiry time is <= my parent's
         if not parent_cred.get_expiration() >= self.get_expiration():
-            raise CredentialNotVerifiable("Delegated credential %s expires after parent %s" % \
-                                          (self.get_summary_tostring(), parent_cred.get_summary_tostring()))
+            raise CredentialNotVerifiable(
+                "Delegated credential {} expires after parent {}"
+                .format(self.pretty_cred(), parent_cred.pretty_cred()))
 
         # make sure my signer is the parent's caller
         if not parent_cred.get_gid_caller().save_to_string(False) == \
            self.get_signature().get_issuer_gid().save_to_string(False):
 
         # make sure my signer is the parent's caller
         if not parent_cred.get_gid_caller().save_to_string(False) == \
            self.get_signature().get_issuer_gid().save_to_string(False):
-            raise CredentialNotVerifiable("Delegated credential %s not signed by parent %s's caller" % \
-                                          (self.get_summary_tostring(), parent_cred.get_summary_tostring()))
+            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_cred()))
+            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_cred()))
+            logger.error("compare2 self details {}".format(self.get_signature().get_issuer_gid().save_to_string()))
+            raise CredentialNotVerifiable(message)
                 
         # Recurse
         if parent_cred.parent:
                 
         # Recurse
         if parent_cred.parent:
@@ -1029,13 +1125,13 @@ class Credential(object):
     # only informative
     def get_filename(self):
         return getattr(self,'filename',None)
     # only informative
     def get_filename(self):
         return getattr(self,'filename',None)
-    
+
     def actual_caller_hrn (self):
         """a helper method used by some API calls like e.g. Allocate
         to try and find out who really is the original caller
     def actual_caller_hrn (self):
         """a helper method used by some API calls like e.g. Allocate
         to try and find out who really is the original caller
-        
+
         This admittedly is a bit of a hack, please USE IN LAST RESORT
         This admittedly is a bit of a hack, please USE IN LAST RESORT
-        
+
         This code uses a heuristic to identify a delegated credential
 
         A first known restriction if for traffic that gets through a slice manager
         This code uses a heuristic to identify a delegated credential
 
         A first known restriction if for traffic that gets through a slice manager
@@ -1047,43 +1143,44 @@ class Credential(object):
         subject_hrn = self.get_gid_object().get_hrn()
         # if we find that the caller_hrn is an immediate descendant of the issuer, then
         # this seems to be a 'regular' credential
         subject_hrn = self.get_gid_object().get_hrn()
         # if we find that the caller_hrn is an immediate descendant of the issuer, then
         # this seems to be a 'regular' credential
-        if caller_hrn.startswith(issuer_hrn): 
+        if 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:
             actual_caller_hrn=issuer_hrn
             actual_caller_hrn=caller_hrn
         # 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=%s, issuer_hrn=%s, returning %s"%(caller_hrn,issuer_hrn,actual_caller_hrn))
+        logger.info("actual_caller_hrn: caller_hrn=%s, issuer_hrn=%s, returning %s"
+                    %(caller_hrn,issuer_hrn,actual_caller_hrn))
         return actual_caller_hrn
         return actual_caller_hrn
-            
+
     ##
     # Dump the contents of a credential to stdout in human-readable format
     #
     # @param dump_parents If true, also dump the parent certificates
     def dump (self, *args, **kwargs):
     ##
     # Dump the contents of a credential to stdout in human-readable format
     #
     # @param dump_parents If true, also dump the parent certificates
     def dump (self, *args, **kwargs):
-        print self.dump_string(*args, **kwargs)
+        print(self.dump_string(*args, **kwargs))
 
 
-    # show_xml is ignored
-    def dump_string(self, dump_parents=False, show_xml=None):
+    # 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 %s\n" % self.get_subject()
+        result += "CREDENTIAL %s\n" % self.pretty_subject()
         filename=self.get_filename()
         if filename: result += "Filename %s\n"%filename
         privileges = self.get_privileges()
         if privileges:
             result += "      privs: %s\n" % privileges.save_to_string()
         else:
         filename=self.get_filename()
         if filename: result += "Filename %s\n"%filename
         privileges = self.get_privileges()
         if privileges:
             result += "      privs: %s\n" % privileges.save_to_string()
         else:
-            result += "      privs: \n" 
+            result += "      privs: \n"
         gidCaller = self.get_gid_caller()
         if gidCaller:
             result += "  gidCaller:\n"
             result += gidCaller.dump_string(8, dump_parents)
 
         if self.get_signature():
         gidCaller = self.get_gid_caller()
         if gidCaller:
             result += "  gidCaller:\n"
             result += gidCaller.dump_string(8, dump_parents)
 
         if self.get_signature():
-            print "  gidIssuer:"
-            self.get_signature().get_issuer_gid().dump(8, dump_parents)
+            result += "  gidIssuer:\n"
+            result += self.get_signature().get_issuer_gid().dump_string(8, dump_parents)
 
         if self.expiration:
 
         if self.expiration:
-            print "  expiration:", self.expiration.strftime(SFATIME_FORMAT)
+            result += "  expiration: " + self.expiration.strftime(SFATIME_FORMAT) + "\n"
 
         gidObject = self.get_gid_object()
         if gidObject:
 
         gidObject = self.get_gid_object()
         if gidObject:
@@ -1094,4 +1191,16 @@ class Credential(object):
             result += "\nPARENT"
             result += self.parent.dump_string(True)
 
             result += "\nPARENT"
             result += self.parent.dump_string(True)
 
+        if show_xml and HAVELXML:
+            try:
+                tree = etree.parse(StringIO(self.xml))
+                aside = etree.tostring(tree, pretty_print=True)
+                result += "\nXML:\n\n"
+                result += aside
+                result += "\nEnd XML\n"
+            except:
+                import traceback
+                print("exc. Credential.dump_string / XML")
+                traceback.print_exc()
+
         return result
         return result