git://git.onelab.eu / sfa.git / blobdiff
? search:
summary | shortlog | log | commit | commitdiff | tree
raw | inline | side by side
prettified certificate, credential and speaksfor_util
[sfa.git] / sfa / trust / credential.py
diff --git a/sfa/trust/credential.py b/sfa/trust/credential.py
index 1f924e0..ee3f732 100644 (file)
--- a/sfa/trust/credential.py
+++ b/sfa/trust/credential.py
@@ -26,14 +26,17 @@
 # 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
 import datetime
-from StringIO import StringIO
 from tempfile import mkstemp
 from xml.dom.minidom import Document, parseString
 
 from tempfile import mkstemp
 from xml.dom.minidom import Document, parseString
 
+from sfa.util.py23 import StringType
+from sfa.util.py23 import StringIO
+
 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:
@@ -59,13 +62,13 @@ DEFAULT_CREDENTIAL_LIFETIME = 86400 * 28
 # . Need to add support for other types of credentials, e.g. tickets
 # . add namespaces to signed-credential element?
 
 # . Need to add support for other types of credentials, e.g. tickets
 # . add namespaces to signed-credential element?
 
-signature_template = \
+signature_format = \
 '''
 '''
-<Signature xml:id="Sig_%s" xmlns="http://www.w3.org/2000/09/xmldsig#">
+<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"/>
     <SignatureMethod Algorithm="http://www.w3.org/2000/09/xmldsig#rsa-sha1"/>
   <SignedInfo>
     <CanonicalizationMethod Algorithm="http://www.w3.org/TR/2001/REC-xml-c14n-20010315"/>
     <SignatureMethod Algorithm="http://www.w3.org/2000/09/xmldsig#rsa-sha1"/>
-    <Reference URI="#%s">
+    <Reference URI="#{refid}">
       <Transforms>
         <Transform Algorithm="http://www.w3.org/2000/09/xmldsig#enveloped-signature" />
       </Transforms>
       <Transforms>
         <Transform Algorithm="http://www.w3.org/2000/09/xmldsig#enveloped-signature" />
       </Transforms>
@@ -85,35 +88,6 @@ signature_template = \
 </Signature>
 '''
 
 </Signature>
 '''
 
-# PG formats the template (whitespace) slightly differently.
-# Note that they don't include the xmlns in the template, but add it later.
-# Otherwise the two are equivalent.
-#signature_template_as_in_pg = \
-#'''
-#<Signature xml:id="Sig_%s" >
-# <SignedInfo>
-#  <CanonicalizationMethod      Algorithm="http://www.w3.org/TR/2001/REC-xml-c14n-20010315"/>
-#  <SignatureMethod      Algorithm="http://www.w3.org/2000/09/xmldsig#rsa-sha1"/>
-#  <Reference URI="#%s">
-#    <Transforms>
-#      <Transform         Algorithm="http://www.w3.org/2000/09/xmldsig#enveloped-signature" />
-#    </Transforms>
-#    <DigestMethod        Algorithm="http://www.w3.org/2000/09/xmldsig#sha1"/>
-#    <DigestValue></DigestValue>
-#    </Reference>
-# </SignedInfo>
-# <SignatureValue />
-# <KeyInfo>
-#  <X509Data >
-#   <X509SubjectName/>
-#   <X509IssuerSerial/>
-#   <X509Certificate/>
-#  </X509Data>
-#  <KeyValue />
-# </KeyInfo>
-#</Signature>
-#'''
-
 ##
 # Convert a string into a bool
 # used to convert an xsd:boolean to a Python boolean
 ##
 # Convert a string into a bool
 # used to convert an xsd:boolean to a Python boolean
@@ -181,20 +155,47 @@ 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:
-            logger.log_exc ("Failed to parse credential, %s"%self.xml)
+        except ExpatError as e:
+            logger.log_exc("Failed to parse credential, {}".format(self.xml))
             raise
         sig = doc.getElementsByTagName("Signature")[0]
             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{}\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")
+        self.set_issuer_gid(GID(string=gids))
         
     def encode(self):
         
     def encode(self):
-        self.xml = signature_template % (self.get_refid(), self.get_refid())
+        self.xml = signature_format.format(refid=self.get_refid())
 
 ##
 # A credential provides a caller gid with privileges to an object gid.
 
 ##
 # A credential provides a caller gid with privileges to an object gid.
@@ -221,6 +222,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 +231,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,41 +253,57 @@ 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:                
                 str = string
             elif filename:
 
         if string or filename:
             if string:                
                 str = string
             elif filename:
-                str = file(filename).read()
+                with open(filename) as infile:
+                    str = infile.read()
                 
             # if this is a legacy credential, write error and bail out
                 
             # if this is a legacy credential, write error and bail out
-            if isinstance (str, StringTypes) and str.strip().startswith("-----"):
-                logger.error("Legacy credentials not supported any more - giving up with %s..."%str[:10])
+            if isinstance(str, StringType) and str.strip().startswith("-----"):
+                logger.error("Legacy credentials not supported any more - giving up with {}...".format(str[:10]))
                 return
             else:
                 self.xml = str
                 self.decode()
                 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):
+        if not self.gidObject:
+            self.decode()
+        return self.gidObject.get_subject()
 
     def pretty_subject(self):
         subject = ""
 
     def pretty_subject(self):
         subject = ""
@@ -365,11 +386,11 @@ class Credential(object):
     # Expiration: an absolute UTC time of expiration (as either an int or string or datetime)
     # 
     def set_expiration(self, expiration):
     # Expiration: an absolute UTC time of expiration (as either an int or string or datetime)
     # 
     def set_expiration(self, expiration):
-        expiration_datetime = utcparse (expiration)
+        expiration_datetime = utcparse(expiration)
         if expiration_datetime is not None:
             self.expiration = expiration_datetime
         else:
         if expiration_datetime is not None:
             self.expiration = expiration_datetime
         else:
-            logger.error ("unexpected input %s in Credential.set_expiration"%expiration)
+            logger.error("unexpected input {} in Credential.set_expiration".format(expiration))
 
     ##
     # get the lifetime of the credential (always in datetime format)
 
     ##
     # get the lifetime of the credential (always in datetime format)
@@ -434,12 +455,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)  
         
@@ -455,9 +477,12 @@ class Credential(object):
         append_sub(doc, cred, "target_urn", self.gidObject.get_urn())
         append_sub(doc, cred, "uuid", "")
         if not self.expiration:
         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 s"%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)
             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)
@@ -513,10 +538,12 @@ class Credential(object):
                     # 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:
                     # 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 %s to %s over %s:\n - Replaced attribute %s value '%s' with '%s'" % \
-                              (self.parent.gidCaller.get_urn(), self.gidCaller.get_urn(), self.gidObject.get_urn(), oldAttr.name, oldAttr.value, attr.value)
+                        msg = "Delegating cred from owner {} to {} over {}:\n"
+                        "- Replaced attribute {} value '{}' with '{}'"\
+                            .format(self.parent.gidCaller.get_urn(), self.gidCaller.get_urn(),
+                                    self.gidObject.get_urn(), oldAttr.name, oldAttr.value, attr.value)
                         logger.warn(msg)
                         logger.warn(msg)
-                        #raise CredentialNotVerifiable("Can't encode new valid delegated credential: %s" % msg)
+                        #raise CredentialNotVerifiable("Can't encode new valid delegated credential: {}".format(msg))
 
             p_cred = doc.importNode(sdoc.getElementsByTagName("credential")[0], True)
             p = doc.createElement("parent")
 
             p_cred = doc.importNode(sdoc.getElementsByTagName("credential")[0], True)
             p = doc.createElement("parent")
@@ -536,7 +563,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):       
@@ -593,7 +620,7 @@ class Credential(object):
         rid = self.get_refid()
         while rid in refs:
             val = int(rid[3:])
         rid = self.get_refid()
         while rid in refs:
             val = int(rid[3:])
-            rid = "ref%d" % (val + 1)
+            rid = "ref{}".format(val + 1)
 
         # Set the new refid
         self.set_refid(rid)
 
         # Set the new refid
         self.set_refid(rid)
@@ -615,7 +642,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 +658,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
@@ -642,10 +673,14 @@ class Credential(object):
 
 
         # Call out to xmlsec1 to sign it
 
 
         # Call out to xmlsec1 to sign it
-        ref = 'Sig_%s' % self.get_refid()
+        ref = 'Sig_{}'.format(self.get_refid())
         filename = self.save_to_random_tmp_file()
         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 = '{} --sign --node-id "{}" --privkey-pem {},{} {}' \
+                  .format(xmlsec1, ref, self.issuer_privkey, ",".join(gid_files), filename)
+        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 +691,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 +704,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 +729,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 +758,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 +816,8 @@ 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.pretty_cred(), error.message, error.line)
+                    message = "{}: {} (line {})".format(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,15 +835,15 @@ 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:
-                    logger.error("Failed to load trusted cert from %s: %r"%( f, exc))
+                except Exception as 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():
             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.pretty_cred(),
-                                           self.expiration.strftime(SFATIME_FORMAT)))
+            raise CredentialNotVerifiable("Credential {} expired at {}" \
+                                          .format(self.pretty_cred(),
+                                                  self.expiration.strftime(SFATIME_FORMAT)))
 
         # Verify the signatures
         filename = self.save_to_random_tmp_file()
 
         # Verify the signatures
         filename = self.save_to_random_tmp_file()
@@ -816,11 +857,11 @@ class Credential(object):
                 cur_cred.get_gid_caller().verify_chain(trusted_cert_objects)
 
         refs = []
                 cur_cred.get_gid_caller().verify_chain(trusted_cert_objects)
 
         refs = []
-        refs.append("Sig_%s" % self.get_refid())
+        refs.append("Sig_{}".format(self.get_refid()))
 
         parentRefs = self.updateRefID()
         for ref in parentRefs:
 
         parentRefs = self.updateRefID()
         for ref in parentRefs:
-            refs.append("Sig_%s" % ref)
+            refs.append("Sig_{}".format(ref))
 
         for ref in refs:
             # If caller explicitly passed in None that means skip xmlsec1 validation.
 
         for ref in refs:
             # If caller explicitly passed in None that means skip xmlsec1 validation.
@@ -832,10 +873,13 @@ class Credential(object):
             # up to fedora20 we used os.popen and checked that the output begins with OK
             # 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
             # up to fedora20 we used os.popen and checked that the output begins with OK
             # 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 %s' % x for x in trusted_certs])
+            #cert_args = " ".join(['--trusted-pem {}'.format(x) for x in trusted_certs])
             #command = '{} --verify --node-id "{}" {} {} 2>&1'.\
             #          format(self.xmlsec_path, ref, cert_args, filename)
             #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 ]
@@ -855,8 +899,8 @@ class Credential(object):
                     mend = verified.find('\\', mstart)
                     msg = verified[mstart:mend]
                 logger.warning("Credential.verify - failed - xmlsec1 returned {}".format(verified.strip()))
                     mend = verified.find('\\', mstart)
                     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.pretty_cred(), ref, msg))
+                raise CredentialNotVerifiable("xmlsec1 error verifying cred {} using Signature ID {}: {}"\
+                                              .format(self.pretty_cred(), ref, msg))
         os.remove(filename)
 
         # Verify the parents (delegation)
         os.remove(filename)
 
         # Verify the parents (delegation)
@@ -891,6 +935,12 @@ 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 {} for object {}. "
+                                          "Cred has no signature" \
+                                          .format(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:
@@ -972,20 +1022,28 @@ 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 {}) rights {} "
-                .format(parent_cred.get_refid(),
-                        self.parent.get_privileges().save_to_string())
-                + " not superset of delegated cred %s (ref %s) rights {}"
-                .format(self.pretty_cred(), 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(
+            message = (
                 "Delegated cred {}: Target gid not equal between parent and child. Parent {}"
                 .format(self.pretty_cred(), parent_cred.pretty_cred()))
                 "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():
@@ -999,8 +1057,10 @@ class Credential(object):
             message = "Delegated credential {} not signed by parent {}'s caller"\
                 .format(self.pretty_cred(), parent_cred.pretty_cred())
             logger.error(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().save_to_string()))
-            logger.error("compare2 self {}".format(self.get_signature().get_issuer_gid().save_to_string()))
+            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
             raise CredentialNotVerifiable(message)
                 
         # Recurse
@@ -1023,7 +1083,7 @@ class Credential(object):
   
         #user_key = Keypair(filename=keyfile)
         #user_hrn = self.get_gid_caller().get_hrn()
   
         #user_key = Keypair(filename=keyfile)
         #user_hrn = self.get_gid_caller().get_hrn()
-        subject_string = "%s delegated to %s" % (object_hrn, delegee_hrn)
+        subject_string = "{} delegated to {}".format(object_hrn, delegee_hrn)
         dcred = Credential(subject=subject_string)
         dcred.set_gid_caller(delegee_gid)
         dcred.set_gid_object(object_gid)
         dcred = Credential(subject=subject_string)
         dcred.set_gid_caller(delegee_gid)
         dcred.set_gid_object(object_gid)
@@ -1041,13 +1101,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):
+
+    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
         """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
@@ -1059,44 +1119,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={}, issuer_hrn={}, returning {}"
+                    .format(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
     ##
     # 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)
+    def dump(self, *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.pretty_subject()
+        result += "CREDENTIAL {}\n".format(self.pretty_subject())
         filename=self.get_filename()
         filename=self.get_filename()
-        if filename: result += "Filename %s\n"%filename
+        if filename: result += "Filename {}\n".format(filename)
         privileges = self.get_privileges()
         if privileges:
         privileges = self.get_privileges()
         if privileges:
-            result += "      privs: %s\n" % privileges.save_to_string()
+            result += "      privs: {}\n".format(privileges.save_to_string())
         else:
         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:
@@ -1107,4 +1167,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
sfa