Python3 compatibility for Credential & Certificate in save_to_string & save_to_file...

[sfa.git] / sfa / trust / credential.py

diff --git a/sfa/trust/credential.py b/sfa/trust/credential.py
index a9f2d1d..0aa6162 100644 (file)
--- a/sfa/trust/credential.py
+++ b/sfa/trust/credential.py
@@ -26,13 +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
 ##
 

-import os
-from types import StringTypes
+from __future__ import print_function
+
+import os, os.path
+import subprocess

 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


@@ -44,12 +48,12 @@ from xml.parsers.expat import ExpatError
 
 from sfa.util.faults import CredentialNotVerifiable, ChildRightsNotSubsetOfParent
 from sfa.util.sfalogging import logger
 
 from sfa.util.faults import CredentialNotVerifiable, ChildRightsNotSubsetOfParent
 from sfa.util.sfalogging import logger

-from sfa.util.sfatime import utcparse
+from sfa.util.sfatime import utcparse, SFATIME_FORMAT

 from sfa.trust.rights import Right, Rights, determine_rights
 from sfa.trust.gid import GID
 from sfa.util.xrn import urn_to_hrn, hrn_authfor_hrn
 
 from sfa.trust.rights import Right, Rights, determine_rights
 from sfa.trust.gid import GID
 from sfa.util.xrn import urn_to_hrn, hrn_authfor_hrn
 

-# 2 weeks, in seconds 
+# 31 days, in seconds 

 DEFAULT_CREDENTIAL_LIFETIME = 86400 * 31
 
 
 DEFAULT_CREDENTIAL_LIFETIME = 86400 * 31
 
 


@@ -58,13 +62,13 @@ DEFAULT_CREDENTIAL_LIFETIME = 86400 * 31
 # . 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>


@@ -84,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


@@ -180,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.


@@ -220,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


@@ -227,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
     #


@@ -247,59 +253,75 @@ 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 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):
 
     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:


@@ -364,15 +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):

-        if isinstance(expiration, (int, float)):
-            self.expiration = datetime.datetime.fromtimestamp(expiration)
-        elif isinstance (expiration, datetime.datetime):
-            self.expiration = expiration
-        elif isinstance (expiration, StringTypes):
-            self.expiration = utcparse (expiration)
+        expiration_datetime = utcparse(expiration)
+        if expiration_datetime is not None:
+            self.expiration = expiration_datetime

         else:
         else:

-            logger.error ("unexpected input type in Credential.set_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)


@@ -437,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)  
         


@@ -458,9 +477,13 @@ 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".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)

-        append_sub(doc, cred, "expires", self.expiration.isoformat())
+        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)
 
         privileges = doc.createElement("privileges")
         cred.appendChild(privileges)
 


@@ -515,9 +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")


@@ -537,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):       


@@ -553,12 +579,16 @@ class Credential(object):
             f = filep 
         else:
             f = open(filename, "w")
             f = filep 
         else:
             f = open(filename, "w")

+        if isinstance(self.xml, bytes):
+            self.xml = self.xml.decode()

         f.write(self.xml)
         f.close()
 
     def save_to_string(self, save_parents=True):
         if not self.xml:
             self.encode()
         f.write(self.xml)
         f.close()
 
     def save_to_string(self, save_parents=True):
         if not self.xml:
             self.encode()

+        if isinstance(self.xml, bytes):
+            self.xml = self.xml.decode()

         return self.xml
 
     def get_refid(self):
         return self.xml
 
     def get_refid(self):


@@ -594,7 +624,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)


@@ -616,7 +646,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]


@@ -628,7 +662,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


@@ -643,10 +677,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:


@@ -657,7 +695,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


@@ -670,7 +708,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 = []


@@ -695,25 +733,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)
 
 


@@ -721,13 +762,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():


@@ -777,7 +820,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.get_summary_tostring(), 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:


@@ -795,18 +839,18 @@ 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.get_summary_tostring(), self.expiration.isoformat()))
+            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()

-        if trusted_certs is not None:
-            cert_args = " ".join(['--trusted-pem %s' % x for x in trusted_certs])

 
         # If caller explicitly passed in None that means skip cert chain validation.
         # - Strange and not typical
 
         # If caller explicitly passed in None that means skip cert chain validation.
         # - Strange and not typical


@@ -817,11 +861,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.


@@ -829,11 +873,28 @@ class Credential(object):
             if trusted_certs is None:
                 break
 
             if trusted_certs is None:
                 break
 

-#            print "Doing %s --verify --node-id '%s' %s %s 2>&1" % \
-#                (self.xmlsec_path, ref, cert_args, filename)
-            verified = os.popen('%s --verify --node-id "%s" %s %s 2>&1' \
-                            % (self.xmlsec_path, ref, cert_args, filename)).read()
-            if not verified.strip().startswith("OK"):
+            # Thierry - jan 2015
+            # 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 {}'.format(x) for x in trusted_certs])
+            #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 ]
+            for trusted in trusted_certs:
+                command += ["--trusted-pem", trusted ]
+            command += [ filename ]
+            logger.debug("Running " + " ".join(command))
+            try:
+                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")
+            except subprocess.CalledProcessError as e:
+                verified = e.output

                 # xmlsec errors have a msg= which is the interesting bit.
                 mstart = verified.find("msg=")
                 msg = ""
                 # xmlsec errors have a msg= which is the interesting bit.
                 mstart = verified.find("msg=")
                 msg = ""


@@ -841,7 +902,9 @@ class Credential(object):
                     mstart = mstart + 4
                     mend = verified.find('\\', mstart)
                     msg = verified[mstart:mend]
                     mstart = mstart + 4
                     mend = verified.find('\\', mstart)
                     msg = verified[mstart:mend]

-                raise CredentialNotVerifiable("xmlsec1 error verifying cred %s using Signature ID %s: %s %s" % (self.get_summary_tostring(), ref, msg, verified.strip()))
+                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)
 
         # Verify the parents (delegation)
         os.remove(filename)
 
         # Verify the parents (delegation)


@@ -876,6 +939,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:


@@ -918,13 +987,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()


@@ -939,8 +1009,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:


@@ -953,23 +1026,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:


@@ -991,7 +1087,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)


@@ -1009,13 +1105,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


@@ -1027,43 +1123,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.get_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.isoformat()
+            result += "  expiration: " + self.expiration.strftime(SFATIME_FORMAT) + "\n"

 
         gidObject = self.get_gid_object()
         if gidObject:
 
         gidObject = self.get_gid_object()
         if gidObject:


@@ -1074,4 +1171,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