Eclipse
/
jetty.project
mirror of https://github.com/jetty/jetty.project.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

405437 Implement section 13.8.4 Uncovered HTTP methods

This commit is contained in:
Jan Bartel 2013-04-23 13:50:39 +10:00
parent a275c8fb37
commit 3df03e18d7
9 changed files with 322 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
jetty-annotations/src/main/java/org/eclipse/jetty/annotations/ServletSecurityAnnotationHandler.java
View File

@ -112,6 +112,9 @@ public class ServletSecurityAnnotationHandler extends AbstractIntrospectableAnno
for (ConstraintMapping m:constraintMappings)
securityHandler.addConstraintMapping(m);
//Servlet Spec 3.1 requires paths with uncovered http methods to be reported
securityHandler.checkPathsWithUncoveredHttpMethods();
}

16
jetty-security/src/main/java/org/eclipse/jetty/security/ConstraintAware.java
View File

@ -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();
}

187
jetty-security/src/main/java/org/eclipse/jetty/security/ConstraintSecurityHandler.java
View File

@ -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;
}
}

3
jetty-security/src/main/java/org/eclipse/jetty/security/RoleInfo.java
View File

@ -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:"")+"}";
}
}

73
jetty-security/src/test/java/org/eclipse/jetty/security/ConstraintTest.java
View File

@ -19,7 +19,9 @@
package org.eclipse.jetty.security;
import static org.hamcrest.Matchers.startsWith;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import static org.junit.matchers.JUnitMatchers.containsString;
@ -27,6 +29,7 @@ import static org.junit.matchers.JUnitMatchers.containsString;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
@ -211,6 +214,76 @@ public class ConstraintTest
assertTrue (mappings.get(2).getConstraint().getAuthenticate());
assertFalse(mappings.get(3).getConstraint().getAuthenticate());
}
@Test
public void testUncoveredHttpMethodDetection() throws Exception
{
//Test no methods named
Constraint constraint1 = new Constraint();
constraint1.setAuthenticate(true);
constraint1.setName("** constraint");
constraint1.setRoles(new String[]{Constraint.ANY_AUTH,"user"}); //No methods named, no uncovered methods
ConstraintMapping mapping1 = new ConstraintMapping();
mapping1.setPathSpec("/starstar/*");
mapping1.setConstraint(constraint1);
_security.setConstraintMappings(Collections.singletonList(mapping1));
_security.setAuthenticator(new BasicAuthenticator());
_server.start();
Set<String> uncoveredPaths = _security.getPathsWithUncoveredHttpMethods();
assertTrue(uncoveredPaths.isEmpty()); //no uncovered methods
//Test only an explicitly named method, no omissions to cover other methods
Constraint constraint2 = new Constraint();
constraint2.setAuthenticate(true);
constraint2.setName("user constraint");
constraint2.setRoles(new String[]{"user"});
ConstraintMapping mapping2 = new ConstraintMapping();
mapping2.setPathSpec("/user/*");
mapping2.setMethod("GET");
mapping2.setConstraint(constraint2);
_security.addConstraintMapping(mapping2);
uncoveredPaths = _security.getPathsWithUncoveredHttpMethods();
assertNotNull(uncoveredPaths);
assertEquals(1, uncoveredPaths.size());
assertTrue(uncoveredPaths.contains("/user/*"));
//Test an explicitly named method with a http-method-omission to cover all other methods
Constraint constraint2a = new Constraint();
constraint2a.setAuthenticate(true);
constraint2a.setName("forbid constraint");
ConstraintMapping mapping2a = new ConstraintMapping();
mapping2a.setPathSpec("/user/*");
mapping2a.setMethodOmissions(new String[]{"GET"});
mapping2a.setConstraint(constraint2a);
_security.addConstraintMapping(mapping2a);
uncoveredPaths = _security.getPathsWithUncoveredHttpMethods();
assertNotNull(uncoveredPaths);
assertEquals(0, uncoveredPaths.size());
//Test a http-method-omission only
Constraint constraint3 = new Constraint();
constraint3.setAuthenticate(true);
constraint3.setName("omit constraint");
ConstraintMapping mapping3 = new ConstraintMapping();
mapping3.setPathSpec("/omit/*");
mapping3.setMethodOmissions(new String[]{"GET", "POST"});
mapping3.setConstraint(constraint3);
_security.addConstraintMapping(mapping3);
uncoveredPaths = _security.getPathsWithUncoveredHttpMethods();
assertNotNull(uncoveredPaths);
assertTrue(uncoveredPaths.contains("/omit/*"));
_security.setDenyUncoveredHttpMethods(true);
uncoveredPaths = _security.getPathsWithUncoveredHttpMethods();
assertNotNull(uncoveredPaths);
assertEquals(0, uncoveredPaths.size());
}
@Test
public void testBasic() throws Exception

40
jetty-security/src/test/java/org/eclipse/jetty/security/SpecExampleConstraintTest.java
View File

@ -19,6 +19,7 @@
package org.eclipse.jetty.security;
import static org.hamcrest.Matchers.startsWith;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
@ -232,6 +233,45 @@ public class SpecExampleConstraintTest
}
}
@Test
public void testUncoveredHttpMethodDetection() throws Exception
{
_security.setAuthenticator(new BasicAuthenticator());
_server.start();
Set<String> paths = _security.getPathsWithUncoveredHttpMethods();
assertEquals(1, paths.size());
assertEquals("/*", paths.iterator().next());
}
@Test
public void testUncoveredHttpMethodsDenied() throws Exception
{
try
{
_security.setDenyUncoveredHttpMethods(false);
_security.setAuthenticator(new BasicAuthenticator());
_server.start();
//There are uncovered methods for GET/POST at url /*
//without deny-uncovered-http-methods they should be accessible
String response;
response = _connector.getResponses("GET /ctx/index.html HTTP/1.0\r\n\r\n");
assertThat(response,startsWith("HTTP/1.1 200 OK"));
//set deny-uncovered-http-methods true
_security.setDenyUncoveredHttpMethods(true);
//check they cannot be accessed
response = _connector.getResponses("GET /ctx/index.html HTTP/1.0\r\n\r\n");
assertTrue(response.startsWith("HTTP/1.1 403 Forbidden"));
}
finally
{
_security.setDenyUncoveredHttpMethods(false);
}
}
@Test

9
jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java
View File

@ -854,9 +854,12 @@ public class ServletHandler extends ScopedHandler
{
setServletMappings(ArrayUtil.addToArray(getServletMappings(), mapping, ServletMapping.class));
}
public Set<String> setServletSecurity(ServletRegistration.Dynamic registration, ServletSecurityElement servletSecurityElement) {
if (_contextHandler != null) {
/* ------------------------------------------------------------ */
public Set<String> setServletSecurity(ServletRegistration.Dynamic registration, ServletSecurityElement servletSecurityElement)
{
if (_contextHandler != null)
{
return _contextHandler.setServletSecurity(registration, servletSecurityElement);
}
return Collections.emptySet();

16
jetty-webapp/src/main/java/org/eclipse/jetty/webapp/StandardDescriptorProcessor.java
View File

@ -92,6 +92,7 @@ public class StandardDescriptorProcessor extends IterativeDescriptorProcessor
registerVisitor("filter-mapping", this.getClass().getDeclaredMethod("visitFilterMapping", __signature));
registerVisitor("listener", this.getClass().getDeclaredMethod("visitListener", __signature));
registerVisitor("distributable", this.getClass().getDeclaredMethod("visitDistributable", __signature));
registerVisitor("deny-uncovered-http-methods", this.getClass().getDeclaredMethod("visitDenyUncoveredHttpMethods", __signature));
}
catch (Exception e)
{
@ -1886,6 +1887,21 @@ public class StandardDescriptorProcessor extends IterativeDescriptorProcessor
//Servlet Spec 3.0 p.74 distributable only if all fragments are distributable
((WebDescriptor)descriptor).setDistributable(true);
}
/**
* Servlet spec 3.1. When present in web.xml, this means that http methods that are
* not covered by security constraints should have access denied.
*
* See section 13.8.4, pg 145
* @param context
* @param descriptor
* @param node
*/
protected void visitDenyUncoveredHttpMethods(WebAppContext context, Descriptor descriptor, XmlParser.Node node)
{
((ConstraintAware)context.getSecurityHandler()).setDenyUncoveredHttpMethods(true);
}
/**
* @param context

3
jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppContext.java
View File

@ -1325,7 +1325,6 @@ public class WebAppContext extends ServletContextHandler implements WebAppClassL
@Override
public Set<String> setServletSecurity(Dynamic registration, ServletSecurityElement servletSecurityElement)
{
Set<String> unchangedURLMappings = new HashSet<String>();
//From javadoc for ServletSecurityElement:
/*
@ -1360,6 +1359,7 @@ public class WebAppContext extends ServletContextHandler implements WebAppClassL
List<ConstraintMapping> mappings = ConstraintSecurityHandler.createConstraintsWithMappingsForPath(registration.getName(), pathSpec, servletSecurityElement);
for (ConstraintMapping m:mappings)
((ConstraintAware)getSecurityHandler()).addConstraintMapping(m);
((ConstraintAware)getSecurityHandler()).checkPathsWithUncoveredHttpMethods();
getMetaData().setOrigin("constraint.url."+pathSpec, Origin.API);
break;
}
@ -1383,6 +1383,7 @@ public class WebAppContext extends ServletContextHandler implements WebAppClassL
constraintMappings.addAll(freshMappings);
((ConstraintSecurityHandler)getSecurityHandler()).setConstraintMappings(constraintMappings);
((ConstraintAware)getSecurityHandler()).checkPathsWithUncoveredHttpMethods();
break;
}
}