405437 Implement section 13.8.4 Uncovered HTTP methods

3 files changed, 182 insertions, 24 deletions

diff --git a/jetty-security/src/main/java/org/eclipse/jetty/security/ConstraintAware.java b/jetty-security/src/main/java/org/eclipse/jetty/security/ConstraintAware.java
index bec6764904..d5c74a6682 100644
--- a/jetty-security/src/main/java/org/eclipse/jetty/security/ConstraintAware.java
+++ b/jetty-security/src/main/java/org/eclipse/jetty/security/ConstraintAware.java
@@ -51,4 +51,20 @@ public interface ConstraintAware
      * @param role
      */
     void addRole(String role);
+    
+    /**
+     * See Servlet Spec 31, sec 13.8.4, pg 145
+     * When true, requests with http methods not explicitly covered either by inclusion or omissions
+     * in constraints, will have access denied.
+     * @param deny
+     */
+    void setDenyUncoveredHttpMethods(boolean deny);
+    
+    boolean isDenyUncoveredHttpMethods();
+    
+    /**
+     * See Servlet Spec 31, sec 13.8.4, pg 145
+     * Container must check if there are urls with uncovered http methods
+     */
+    boolean checkPathsWithUncoveredHttpMethods();
 }
diff --git a/jetty-security/src/main/java/org/eclipse/jetty/security/ConstraintSecurityHandler.java b/jetty-security/src/main/java/org/eclipse/jetty/security/ConstraintSecurityHandler.java
index f7982a1349..8377d459ec 100644
--- a/jetty-security/src/main/java/org/eclipse/jetty/security/ConstraintSecurityHandler.java
+++ b/jetty-security/src/main/java/org/eclipse/jetty/security/ConstraintSecurityHandler.java
@@ -45,6 +45,8 @@ import org.eclipse.jetty.server.HttpConfiguration;
 import org.eclipse.jetty.server.Request;
 import org.eclipse.jetty.server.Response;
 import org.eclipse.jetty.server.UserIdentity;
+import org.eclipse.jetty.util.log.Log;
+import org.eclipse.jetty.util.log.Logger;
 import org.eclipse.jetty.util.security.Constraint;
 
 /* ------------------------------------------------------------ */
@@ -58,11 +60,14 @@ import org.eclipse.jetty.util.security.Constraint;
  */
 public class ConstraintSecurityHandler extends SecurityHandler implements ConstraintAware
 {
+    private static final Logger LOG = Log.getLogger(SecurityHandler.class); //use same as SecurityHandler
+    
     private static final String OMISSION_SUFFIX = ".omission";
     private static final String ALL_METHODS = "*";
     private final List<ConstraintMapping> _constraintMappings= new CopyOnWriteArrayList<>();
     private final Set<String> _roles = new CopyOnWriteArraySet<>();
     private final PathMap<Map<String, RoleInfo>> _constraintMap = new PathMap<>();
+    private boolean _denyUncoveredMethods = false;
 
 
     /* ------------------------------------------------------------ */
@@ -436,9 +441,13 @@ public class ConstraintSecurityHandler extends SecurityHandler implements Constr
                 processConstraintMapping(mapping);
             }
         }
+        
+        //Servlet Spec 3.1 pg 147 sec 13.8.4.2 log paths for which there are uncovered http methods
+        checkPathsWithUncoveredHttpMethods();        
+       
         super.doStart();
     }
-    
+
     
     /* ------------------------------------------------------------ */
     @Override
@@ -520,34 +529,32 @@ public class ConstraintSecurityHandler extends SecurityHandler implements Constr
 
     /* ------------------------------------------------------------ */
     /** Constraints that name method omissions are dealt with differently.
-     * We create an entry in the mappings with key "method.omission". This entry
+     * We create an entry in the mappings with key "&lt;method&gt;.omission". This entry
      * is only ever combined with other omissions for the same method to produce a
      * consolidated RoleInfo. Then, when we wish to find the relevant constraints for
      *  a given Request (in prepareConstraintInfo()), we consult 3 types of entries in 
      * the mappings: an entry that names the method of the Request specifically, an
      * entry that names constraints that apply to all methods, entries of the form
-     * method.omission, where the method of the Request is not named in the omission.
+     * &lt;method&gt;.omission, where the method of the Request is not named in the omission.
      * @param mapping
      * @param mappings
      */
     protected void processConstraintMappingWithMethodOmissions (ConstraintMapping mapping, Map<String, RoleInfo> mappings)
     {
         String[] omissions = mapping.getMethodOmissions();
-
-        for (String omission:omissions)
+        
+        StringBuilder sb = new StringBuilder();
+        for (int i=0; i<omissions.length; i++)
         {
-            //for each method omission, see if there is already a RoleInfo for it in mappings
-            RoleInfo ri = mappings.get(omission+OMISSION_SUFFIX);
-            if (ri == null)
-            {
-                //if not, make one
-                ri = new RoleInfo();
-                mappings.put(omission+OMISSION_SUFFIX, ri);
-            }
-
-            //initialize RoleInfo or combine from ConstraintMapping
-            configureRoleInfo(ri, mapping);
+            if (i > 0)
+                sb.append(".");
+            sb.append(omissions[i]);
         }
+        sb.append(OMISSION_SUFFIX);
+       
+        RoleInfo ri = new RoleInfo();
+        mappings.put(sb.toString(), ri);
+        configureRoleInfo(ri, mapping);
     }
     
     
@@ -558,7 +565,7 @@ public class ConstraintSecurityHandler extends SecurityHandler implements Constr
      * @param mapping
      */
     protected void configureRoleInfo (RoleInfo ri, ConstraintMapping mapping)
-    {
+    { 
         Constraint constraint = mapping.getConstraint();
         boolean forbidden = constraint.isForbidden();
         ri.setForbidden(forbidden);
@@ -567,7 +574,6 @@ public class ConstraintSecurityHandler extends SecurityHandler implements Constr
         //which we need in order to do combining of omissions in prepareConstraintInfo
         UserDataConstraint userDataConstraint = UserDataConstraint.get(mapping.getConstraint().getDataConstraint());
         ri.setUserDataConstraint(userDataConstraint);
-        
 
         //if forbidden, no point setting up roles
         if (!ri.isForbidden())
@@ -614,8 +620,8 @@ public class ConstraintSecurityHandler extends SecurityHandler implements Constr
      * represents a merged set of user data constraints, roles etc -:
      * <ol>
      * <li>A mapping of an exact method name </li>
-     * <li>A mapping will null key that matches every method name</li>
-     * <li>Mappings with keys of the form "method.omission" that indicates it will match every method name EXCEPT that given</li>
+     * <li>A mapping with key * that matches every method name</li>
+     * <li>Mappings with keys of the form "&lt;method&gt;.&lt;method&gt;.&lt;method&gt;.omission" that indicates it will match every method name EXCEPT those given</li>
      * </ol>
      * 
      * @see org.eclipse.jetty.security.SecurityHandler#prepareConstraintInfo(java.lang.String, org.eclipse.jetty.server.Request)
@@ -644,11 +650,16 @@ public class ConstraintSecurityHandler extends SecurityHandler implements Constr
                 //(ie matches because target method is not omitted, hence considered covered by the constraint)
                 for (Entry<String, RoleInfo> entry: mappings.entrySet())
                 {
-                    if (entry.getKey() != null && entry.getKey().contains(OMISSION_SUFFIX) && !(httpMethod+OMISSION_SUFFIX).equals(entry.getKey()))
+                    if (entry.getKey() != null && entry.getKey().endsWith(OMISSION_SUFFIX) && ! entry.getKey().contains(httpMethod))
                         applicableConstraints.add(entry.getValue());
                 }
                 
-                if (applicableConstraints.size() == 1)
+                if (applicableConstraints.size() == 0 && isDenyUncoveredHttpMethods())
+                {
+                    roleInfo = new RoleInfo();
+                    roleInfo.setForbidden(true);
+                }
+                else if (applicableConstraints.size() == 1)
                     roleInfo = applicableConstraints.get(0);
                 else
                 {
@@ -660,6 +671,7 @@ public class ConstraintSecurityHandler extends SecurityHandler implements Constr
                 }
 
             }
+           
             return roleInfo;
         }
 
@@ -681,7 +693,6 @@ public class ConstraintSecurityHandler extends SecurityHandler implements Constr
 
         HttpConfiguration httpConfig = HttpChannel.getCurrentHttpChannel().getHttpConfiguration();
 
-
         if (dataConstraint == UserDataConstraint.Confidential || dataConstraint == UserDataConstraint.Integral)
         {
             if (request.isSecure())
@@ -782,5 +793,135 @@ public class ConstraintSecurityHandler extends SecurityHandler implements Constr
                 Collections.singleton(_roles),
                 _constraintMap.entrySet());
     }
+    
+    /* ------------------------------------------------------------ */
+    /** 
+     * @see org.eclipse.jetty.security.ConstraintAware#setDenyUncoveredHttpMethods(boolean)
+     */
+    @Override
+    public void setDenyUncoveredHttpMethods(boolean deny)
+    {
+        _denyUncoveredMethods = deny;
+    }
+    
+    /* ------------------------------------------------------------ */
+    @Override
+    public boolean isDenyUncoveredHttpMethods()
+    {
+        return _denyUncoveredMethods;
+    }
+    
+    
+    /* ------------------------------------------------------------ */
+    /**
+     * Servlet spec 3.1 pg. 147.
+     */
+    @Override
+    public boolean checkPathsWithUncoveredHttpMethods()
+    {
+        Set<String> paths = getPathsWithUncoveredHttpMethods();
+        if (paths != null && !paths.isEmpty())
+        {
+            for (String p:paths)
+                LOG.warn("Path with uncovered http methods: {}",p);
+            return true;
+        }
+        return false; 
+    }
+    
 
+    /* ------------------------------------------------------------ */
+    /**
+     * Servlet spec 3.1 pg. 147.
+     * The container must check all the combined security constraint
+     * information and log any methods that are not protected and the
+     * urls at which they are not protected
+     * 
+     * @return list of paths for which there are uncovered methods
+     */
+    public Set<String> getPathsWithUncoveredHttpMethods ()
+    {
+        //if automatically denying uncovered methods, there are no uncovered methods
+        if (_denyUncoveredMethods)
+            return Collections.emptySet();
+        
+        Set<String> uncoveredPaths = new HashSet<String>();
+        
+        for (String path:_constraintMap.keySet())
+        {
+            Map<String, RoleInfo> methodMappings = _constraintMap.get(path);
+            //Each key is either:
+            // : an exact method name
+            // : * which means that the constraint applies to every method
+            // : a name of the form <method>.<method>.<method>.omission, which means it applies to every method EXCEPT those named
+            if (methodMappings.get(ALL_METHODS) != null)
+                continue; //can't be any uncovered methods for this url path
+          
+            boolean hasOmissions = omissionsExist(path, methodMappings);
+            
+            for (String method:methodMappings.keySet())
+            {
+                if (method.endsWith(OMISSION_SUFFIX))
+                {
+                    Set<String> omittedMethods = getOmittedMethods(method);
+                    for (String m:omittedMethods)
+                    {
+                        if (!methodMappings.containsKey(m))
+                            uncoveredPaths.add(path);
+                    }
+                }
+                else
+                {
+                    //an exact method name
+                    if (!hasOmissions)
+                        //a http-method does not have http-method-omission to cover the other method names
+                        uncoveredPaths.add(path);
+                }
+                
+            }
+        }
+        return uncoveredPaths;
+    }
+    
+    /* ------------------------------------------------------------ */
+    /**
+     * Check if any http method omissions exist in the list of method
+     * to auth info mappings.
+     * 
+     * @param methodNames
+     * @return
+     */
+    protected boolean omissionsExist (String path, Map<String, RoleInfo> methodMappings)
+    {
+        if (methodMappings == null)
+            return false;
+        boolean hasOmissions = false;
+        for (String m:methodMappings.keySet())
+        {
+            if (m.endsWith(OMISSION_SUFFIX))
+                hasOmissions = true;
+        }
+        return hasOmissions;
+    }
+    
+    
+    /* ------------------------------------------------------------ */
+    /**
+     * Given a string of the form &lt;method&gt;.&lt;method&gt;.omission
+     * split out the individual method names.
+     * 
+     * @param omission
+     * @return
+     */
+    protected Set<String> getOmittedMethods (String omission)
+    {
+        if (omission == null || !omission.endsWith(OMISSION_SUFFIX))
+            return Collections.emptySet();
+        
+        String[] strings = omission.split("\\.");
+        Set<String> methods = new HashSet<String>();
+        for (int i=0;i<strings.length-1;i++)
+            methods.add(strings[i]);
+        return methods;
+    }
 }
diff --git a/jetty-security/src/main/java/org/eclipse/jetty/security/RoleInfo.java b/jetty-security/src/main/java/org/eclipse/jetty/security/RoleInfo.java
index 652ce7a3f7..5fcab95780 100644
--- a/jetty-security/src/main/java/org/eclipse/jetty/security/RoleInfo.java
+++ b/jetty-security/src/main/java/org/eclipse/jetty/security/RoleInfo.java
@@ -116,6 +116,7 @@ public class RoleInfo
         if (userDataConstraint == null) throw new NullPointerException("Null UserDataConstraint");
         if (this._userDataConstraint == null)
         {
+           
             this._userDataConstraint = userDataConstraint;
         }
         else
@@ -156,6 +157,6 @@ public class RoleInfo
     @Override
     public String toString()
     {
-        return "{RoleInfo"+(_forbidden?",F":"")+(_checked?",C":"")+(_isAnyRole?",*":_roles)+"}";
+        return "{RoleInfo"+(_forbidden?",F":"")+(_checked?",C":"")+(_isAnyRole?",*":_roles)+(_userDataConstraint!=null?","+_userDataConstraint:"")+"}";
     }
 }