Added additional logging/error handling in SHIRE and XMLOriginSiteMapper.

Fixed problem with error page pointing to incorrect jsp.
Extracted signature verification to a method.

git-svn-id: https://subversion.switch.ch/svn/shibboleth/java-idp/trunk@249 ab3bd59b-922f-494d-bb5f-6f0a3c29deca

diff --git a/src/edu/internet2/middleware/shibboleth/shire/ShireServlet.java b/src/edu/internet2/middleware/shibboleth/shire/ShireServlet.java
index 1b0f0c3..120c1be 100755 (executable)
--- a/src/edu/internet2/middleware/shibboleth/shire/ShireServlet.java
+++ b/src/edu/internet2/middleware/shibboleth/shire/ShireServlet.java
@@ -116,7 +116,7 @@ public class ShireServlet extends HttpServlet {
         *    <DD> A pathname to the trusted CA roots to accept</DD>
         *    <DT> keystore-password <I>(required)</I> </DT>
         *    <DD> The root keystore password</DD>
-        *    <DT> registry-alias <I>(optional)</I> </DT>
+        *    <DT> keystore-alias <I>(optional)</I> </DT>
         *    <DD> An alias in the provided keystore for the cert that can verify
         *    the origin site registry signature</DD>
         *    <DT> registry-uri <I>(required)</I> </DT>
@@ -150,6 +150,7 @@ public class ShireServlet extends HttpServlet {
                        KeyStore ks = KeyStore.getInstance("JKS");
                        ks.load(getServletContext().getResourceAsStream(keyStorePath), keyStorePasswd.toCharArray());
 
+                       log.debug("Configured to use keystore-alias (" + keyStoreAlias + ") to verify site file.");
                        if (keyStoreAlias != null) {
                                Certificate cert;
                                cert = ks.getCertificate(keyStoreAlias);
@@ -171,20 +172,20 @@ public class ShireServlet extends HttpServlet {
                        log.info("Completed SHIRE initialization");
 
                } catch (OriginSiteMapperException e) {
-                       log.fatal("Unable load shibboleth site information." + e);
-                       throw new UnavailableException("Unable load shibboleth site information." + e);
+                       log.fatal("Configuration problem: Unable load shibboleth site information." + e);
+                       throw new UnavailableException("Configuration problem: Unable load shibboleth site information." + e);
                } catch (KeyStoreException e) {
-                       log.fatal("Unable supplied keystore." + e);
-                       throw new UnavailableException("Unable load supplied keystore." + e);
+                       log.fatal("Configuration problem: Unable to load supplied keystore." + e);
+                       throw new UnavailableException("Configuration problem: Unable load supplied keystore." + e);
                } catch (NoSuchAlgorithmException e) {
-                       log.fatal("Unable supplied keystore." + e);
-                       throw new UnavailableException("Unable load supplied keystore." + e);
+                       log.fatal("Configuration problem: Unable to load supplied keystore." + e);
+                       throw new UnavailableException("Configuration problem: Unable load supplied keystore." + e);
                } catch (CertificateException e) {
-                       log.fatal("Unable supplied keystore." + e);
-                       throw new UnavailableException("Unable load supplied keystore." + e);
+                       log.fatal("Configuration problem: Unable to load supplied keystore." + e);
+                       throw new UnavailableException("Configuration problem: Unable load supplied keystore." + e);
                } catch (IOException e) {
-                       log.fatal("Unable supplied keystore." + e);
-                       throw new UnavailableException("Unable load supplied keystore." + e);
+                       log.fatal("Configuration problem: Unable to loadsupplied keystore." + e);
+                       throw new UnavailableException("Configuration problem: Unable load supplied keystore." + e);
                }
 
        }
@@ -442,7 +443,7 @@ public class ShireServlet extends HttpServlet {
                log.debug("Displaying error page.");
                req.setAttribute("errorText", se.toString());
                req.setAttribute("requestURL", req.getRequestURI().toString());
-               RequestDispatcher rd = req.getRequestDispatcher("/wayferror.jsp");
+               RequestDispatcher rd = req.getRequestDispatcher("/shireerror.jsp");
 
                try {
                        rd.forward(req, res);

diff --git a/src/edu/internet2/middleware/shibboleth/shire/XMLOriginSiteMapper.java b/src/edu/internet2/middleware/shibboleth/shire/XMLOriginSiteMapper.java
index 8e2c22a..e8ccd41 100755 (executable)
--- a/src/edu/internet2/middleware/shibboleth/shire/XMLOriginSiteMapper.java
+++ b/src/edu/internet2/middleware/shibboleth/shire/XMLOriginSiteMapper.java
@@ -60,6 +60,7 @@ import java.util.Vector;
 import javax.xml.parsers.DocumentBuilder;
 import javax.xml.parsers.ParserConfigurationException;
 
+import org.apache.log4j.Logger;
 import org.apache.xml.security.c14n.Canonicalizer;
 import org.apache.xml.security.exceptions.XMLSecurityException;
 import org.apache.xml.security.keys.KeyInfo;
@@ -89,6 +90,7 @@ public class XMLOriginSiteMapper implements OriginSiteMapper {
        private HashMap originSites = null;
        private HashMap hsKeys = null;
        private KeyStore ks = null;
+       private static Logger log = Logger.getLogger(XMLOriginSiteMapper.class.getName());
 
        /**
         *  Constructor for the XMLOriginSiteMapper object
@@ -113,9 +115,12 @@ public class XMLOriginSiteMapper implements OriginSiteMapper {
                        builder = org.opensaml.XML.parserPool.get();
                        Document doc;
                        doc = builder.parse(registryURI);
+                       log.info("Located site file (" +registryURI +").");
                        Element e = doc.getDocumentElement();
-                       if (!XML.SHIB_NS.equals(e.getNamespaceURI()) || !"Sites".equals(e.getLocalName()))
-                               throw new OriginSiteMapperException("XMLOriginSiteMapper() requires shib:Sites as root element");
+                       if (!XML.SHIB_NS.equals(e.getNamespaceURI()) || !"Sites".equals(e.getLocalName())) {
+                               log.error("Construction requires a valid site file: (shib:Sites as root element)");
+                               throw new OriginSiteMapperException("Construction requires a valid site file: (shib:Sites as root element)");
+                       }
 
                        // Loop over the OriginSite elements.
                        NodeList nlist = e.getElementsByTagNameNS(XML.SHIB_NS, "OriginSite");
@@ -168,51 +173,21 @@ public class XMLOriginSiteMapper implements OriginSiteMapper {
                                }
                        }
 
-                       if (verifyKey == null)
-                               return;
-
-                       Node n = e.getLastChild();
-                       while (n != null && n.getNodeType() != Node.ELEMENT_NODE)
-                               n = n.getPreviousSibling();
-
-                       if (n != null
-                               && org.opensaml.XML.XMLSIG_NS.equals(n.getNamespaceURI())
-                               && "Signature".equals(n.getLocalName())) {
-                               try {
-                                       XMLSignature sig = new XMLSignature((Element) n, null);
-                                       if (sig.checkSignatureValue(verifyKey)) {
-                                               // Now we verify that what is signed is what we expect.
-                                               SignedInfo sinfo = sig.getSignedInfo();
-                                               if (sinfo.getLength() == 1
-                                                       && (sinfo
-                                                               .getCanonicalizationMethodURI()
-                                                               .equals(Canonicalizer.ALGO_ID_C14N_WITH_COMMENTS)
-                                                               || sinfo.getCanonicalizationMethodURI().equals(
-                                                                       Canonicalizer.ALGO_ID_C14N_OMIT_COMMENTS)))
-                                                       //                                sinfo.getCanonicalizationMethodURI().equals(Canonicalizer.ALGO_ID_C14N_EXCL_WITH_COMMENTS) ||
-                                                       //                         sinfo.getCanonicalizationMethodURI().equals(Canonicalizer.ALGO_ID_C14N_EXCL_OMIT_COMMENTS))
-                                                       {
-                                                       Reference ref = sinfo.item(0);
-                                                       if (ref.getURI() == null || ref.getURI().equals("")) {
-                                                               Transforms trans = ref.getTransforms();
-                                                               if (trans.getLength() == 1
-                                                                       && trans.item(0).getURI().equals(Transforms.TRANSFORM_ENVELOPED_SIGNATURE))
-                                                                       return;
-                                                       }
-                                               }
-                                       }
-                               } catch (Exception sigE) {
-                                       throw new OriginSiteMapperException(
-                                               "Unable to verify signature on registry file: Site file not signed correctly with specified key:"
-                                                       + sigE);
-                               }
+                       if (verifyKey != null) {
+                               log.info("Initialized with a key: attempting to verify document signature.");
+                               validateSignature(verifyKey, e);
+                       } else {
+                               log.info("Initialized without key: skipping signature verification.");
                        }
-                       throw new OriginSiteMapperException("Unable to verify signature on registry file: no signature found.");
+
                } catch (SAXException e) {
+                       log.error("Problem parsing site configuration" + e);
                        throw new OriginSiteMapperException("Problem parsing site configuration" + e);
                } catch (IOException e) {
+                       log.error("Problem accessing site configuration" + e);
                        throw new OriginSiteMapperException("Problem accessing site configuration" + e);
                } catch (ParserConfigurationException pce) {
+                       log.error("Parser configuration error" + pce);
                        throw new OriginSiteMapperException("Parser configuration error" + pce);
                } finally {
                        if (builder != null)
@@ -220,6 +195,63 @@ public class XMLOriginSiteMapper implements OriginSiteMapper {
                }
        }
 
+       private void validateSignature(Key verifyKey, Element e) throws OriginSiteMapperException {
+
+               Node n = e.getLastChild();
+               while (n != null && n.getNodeType() != Node.ELEMENT_NODE)
+                       n = n.getPreviousSibling();
+
+               if (n != null
+                       && org.opensaml.XML.XMLSIG_NS.equals(n.getNamespaceURI())
+                       && "Signature".equals(n.getLocalName())) {
+                               log.info("Located signature in document... verifying.");
+                       try {
+                               XMLSignature sig = new XMLSignature((Element) n, null);
+                               if (sig.checkSignatureValue(verifyKey)) {
+                                       // Now we verify that what is signed is what we expect.
+                                       SignedInfo sinfo = sig.getSignedInfo();
+                                       if (sinfo.getLength() == 1
+                                               && (sinfo
+                                                       .getCanonicalizationMethodURI()
+                                                       .equals(Canonicalizer.ALGO_ID_C14N_WITH_COMMENTS)
+                                                       || sinfo.getCanonicalizationMethodURI().equals(
+                                                               Canonicalizer.ALGO_ID_C14N_OMIT_COMMENTS))) {
+                                               Reference ref = sinfo.item(0);
+                                               if (ref.getURI() == null || ref.getURI().equals("")) {
+                                                       Transforms trans = ref.getTransforms();
+                                                       if (trans.getLength() == 1
+                                                               && trans.item(0).getURI().equals(Transforms.TRANSFORM_ENVELOPED_SIGNATURE))
+                                                               log.info("Signature verification successful.");
+                                                               return;
+                                               }
+                                               log.error(
+                                                       "Unable to verify signature on registry file: Unsupported dsig reference or transform data submitted with signature.");
+                                               throw new OriginSiteMapperException("Unable to verify signature on registry file: Unsupported dsig reference or transform data submitted with signature.");
+                                       } else {
+                                               log.error(
+                                                       "Unable to verify signature on registry file: Unsupported canonicalization method.");
+                                               throw new OriginSiteMapperException("Unable to verify signature on registry file: Unsupported canonicalization method.");
+                                       }
+                               } else {
+                                       log.error(
+                                               "Unable to verify signature on registry file: signature cannot be verified with the specified key.");
+                                       throw new OriginSiteMapperException("Unable to verify signature on registry file: signature cannot be verified with the specified key.");
+                               }
+                       } catch (Exception sigE) {
+                               log.error(
+                                       "Unable to verify signature on registry file: An error occured while attempting to verify the signature:"
+                                               + sigE);
+                               throw new OriginSiteMapperException(
+                                       "Unable to verify signature on registry file: An error occured while attempting to verify the signature:"
+                                               + sigE);
+                       }
+               } else {
+                       log.error("Unable to verify signature on registry file: no signature found in document.");
+                       throw new OriginSiteMapperException("Unable to verify signature on registry file: no signature found in document.");
+               }
+
+       }
+
        /**
         *  Provides an iterator over the trusted Handle Services for the specified
         *  origin site

diff --git a/webApplication/shireerror.jsp b/webApplication/shireerror.jsp
index 2dc6b38..7bef00c 100755 (executable)
--- a/webApplication/shireerror.jsp
+++ b/webApplication/shireerror.jsp
@@ -16,13 +16,13 @@
 \r
 <body>\r
 <div class="head">\r
-<img src="images/internet2.gif" />" alt="Logo" />\r
+<img src="images/internet2.gif"" alt="Logo" />\r
 <h1>Shibboleth SHIRE Failure</h1>\r
 </div>\r
 \r
 <p>The inter-institutional access system experienced a technical failure.</p>\r
 \r
-<p>Please email <a href="mailto:<bean:write name="test@test.edu" />"><bean:write name="test@test.edu" /></a> and include the following error message:</p>\r
+<p>Please email <a href="mailto:test@test.edu"">test@test.edu"</a> and include the following error message:</p>\r
 \r
 <p class="error">SHIRE failure at (<bean:write name="requestURL" />)</p>\r
 \r