{"id":20563,"date":"2014-01-13T16:00:19","date_gmt":"2014-01-13T14:00:19","guid":{"rendered":"http:\/\/www.javacodegeeks.com\/?p=20563"},"modified":"2014-01-12T21:58:45","modified_gmt":"2014-01-12T19:58:45","slug":"check-your-rest-parameters","status":"publish","type":"post","link":"https:\/\/www.javacodegeeks.com\/2014\/01\/check-your-rest-parameters.html","title":{"rendered":"Check your REST parameters!"},"content":{"rendered":"

I was doing research related to my ongoing \u201cproject student\u201d series and realized that I had made one of the most common \u2013 and most easily remedied \u2013 mistakes. I wasn\u2019t using everything I know about the webapp to push my security perimeter outwards.<\/p>\n

I am thinking specifically about the UUID parameters. I know that every valid externally visible ID will be a UUID. I know the form of the UUIDs. So why don\u2019t I verify that my \u201cuuid\u201d parameters are potentially valid UUIDs before going any further?<\/p>\n

It\u2019s true that the database layer won\u2019t recognize a bad \u201cuuid\u201d value \u2013 but that may not be the intent of the attacker. Perhaps it\u2019s part of a SQL injection attack. Perhaps it\u2019s part of an XSS attack. Perhaps it\u2019s part of an attack on my logs (e.g., by including a really long value that might cause a buffer overflow). Perhaps it\u2019s part of something I\u2019ve never heard of. It doesn\u2019t matter \u2013 I will always<\/strong> be stronger by eliminating known-invalid data as quickly as possible.<\/p>\n

Utility Method<\/h2>\n

The utility method to determine whether a value is a possible UUID uses a simple regex pattern.<\/p>\n

public final class StudentUtil {\r\n    private static final Pattern UUID_PATTERN = Pattern\r\n            .compile(\"^\\\\p{XDigit}{8}+-\\\\p{XDigit}{4}+-\\\\p{XDigit}{4}-\\\\p{XDigit}{4}+-\\\\p{XDigit}{12}$\");\r\n\r\n    \/**\r\n     * Private constructor to prevent instantiation.\r\n     *\/\r\n    private StudentUtil() {\r\n\r\n    }\r\n\r\n    public static boolean isPossibleUuid(String value) {\r\n        return value != null && UUID_PATTERN.matcher(value).matches();\r\n    }\r\n}<\/pre>\n
If we want to be aggressive we could carefully select our UUIDs so they have additional properties that we can check. For instance the corresponding BigInteger could always have a remainder of 3 mod 17. It\u2019s unlikely an attack would know this and we would have warning when somebody is probing our system. An even more sophisticated approach would use a different property for each class of UUID, e.g., a \u2018course\u2019 UUID might be 3 mod 17 while a \u2018student\u2019 UUID is 5 mod 17.<\/p>\n
Unit Test<\/h2>\n
It\u2019s easy to go overboard on our tests but a minimal set would be checking for non-hex digits,
\ntoo many or too few values, an empty string, and a null value.<\/p>\n
public class StudentUtilTest {\r\n\r\n    @Test\r\n    public void testValidUuid() {\r\n        assertTrue(StudentUtil.isPossibleUuid(\"63c7d688-705c-4374-937c-6628952b41e1\"));\r\n    }\r\n\r\n    @Test\r\n    public void testInvalidUuid() {\r\n        assertTrue(!StudentUtil.isPossibleUuid(\"63c7d68x-705c-4374-937c-6628952b41e1\"));\r\n        assertTrue(!StudentUtil.isPossibleUuid(\"63c7d68-8705c-4374-937c-6628952b41e1\"));\r\n        assertTrue(!StudentUtil.isPossibleUuid(\"63c7d688-705c4-374-937c-6628952b41e1\"));\r\n        assertTrue(!StudentUtil.isPossibleUuid(\"63c7d688-705c-43749-37c-6628952b41e1\"));\r\n        assertTrue(!StudentUtil.isPossibleUuid(\"63c7d688-705c-4374-937c6-628952b41e1\"));\r\n        assertTrue(!StudentUtil.isPossibleUuid(\"63c7d688-705c-4374-937c-6628952b41e1a\"));\r\n        assertTrue(!StudentUtil.isPossibleUuid(\"63c7d688-705c-4374-937c-6628952b41e\"));\r\n        assertTrue(!StudentUtil.isPossibleUuid(\"\"));\r\n        assertTrue(!StudentUtil.isPossibleUuid(null));\r\n    }\r\n}<\/pre>\n
REST Server<\/h2>\n
The REST server should check the UUID value for all methods that require one. It\u2019s safe to log the request parameter after we\u2019ve verified it\u2019s a well-formed UUID but still need to be careful about logging unsanitized values in the request.
 
<\/div> <\/div><\/p>\n
@Path(\"\/{courseId}\")\r\n    @GET\r\n    @Produces({ MediaType.APPLICATION_JSON, MediaType.TEXT_XML })\r\n    public Response getCourse(@PathParam(\"courseId\") String id) {\r\n\r\n        Response response = null;\r\n        if (!StudentUtil.isPossibleUuid(id)) {\r\n            response = Response.status(Status.BAD_REQUEST).build();\r\n            LOG.info(\"attempt to use malformed UUID\");\r\n        } else {\r\n            LOG.debug(\"CourseResource: getCourse(\" + id + \")\");\r\n            try {\r\n                Course course = finder.findCourseByUuid(id);\r\n                response = Response.ok(scrubCourse(course)).build();\r\n            } catch (ObjectNotFoundException e) {\r\n                response = Response.status(Status.NOT_FOUND).build();\r\n                LOG.debug(\"course not found: \" + id);\r\n            } catch (Exception e) {\r\n                if (!(e instanceof UnitTestException)) {\r\n                    LOG.info(\"unhandled exception\", e);\r\n                }\r\n                response = Response.status(Status.INTERNAL_SERVER_ERROR).build();\r\n            }\r\n        }\r\n\r\n        return response;\r\n    }<\/pre>\n
An obvious improvement is to move this check (and the exception catchall) into an AOP wrapper to all service methods. This will simplify the code and go a long way towards guaranteeing that the checks are always performed. (I\u2019m not using it in Project Student at the moment since the webservice server layer doesn\u2019t currently have Spring dependencies.)<\/p>\n
You can make a strong opsec argument that the REST methods should return a NOT_FOUND response instead of a BAD_REQUEST response in order to reduce information leakage.<\/p>\n
Webapps<\/h2>\n
The details differ but we should do the same thing with webapps even if they\u2019re just shallow front-ends to REST services. Whenever there\u2019s a UUID, no matter it\u2019s source, it should be checked before it is used.<\/p>\n
Filters<\/h2>\n
There is a school of thought that security should be handled separately from the application \u2013 that the best security is knit in at deployment (via filters and AOP) instead of being baked into the application. Nobody is suggesting that app developers should ignore security considerations, just that checks like I discussed above are clutter that distract the developer and aren\u2019t reliable since it\u2019s easy for a developer to overlook. They would recommend using AOP or a filter instead.<\/p>\n
It\u2019s straightforward to write a filter that does the same work as the code above:<\/p>\n
public class RestParameterFilter implements Filter {\r\n    private static final Logger LOG = Logger.getLogger(RestParameterFilter.class);\r\n    private static final Set<String> validNouns = new HashSet<>();\r\n\r\n    \/**\r\n     * @see javax.servlet.Filter#init(javax.servlet.FilterConfig)\r\n     *\/\r\n    @Override\r\n    public void init(FilterConfig cfg) throws ServletException {\r\n\r\n        \/\/ learn valid nouns\r\n        final String nouns = cfg.getInitParameter(\"valid-nouns\");\r\n        if (nouns != null) {\r\n            for (String noun : nouns.split(\",\")) {\r\n                validNouns.add(noun.trim());\r\n            }\r\n        }\r\n    }\r\n\r\n    \/**\r\n     * @see javax.servlet.Filter#doFilter(javax.servlet.ServletRequest,\r\n     *      javax.servlet.ServletResponse, javax.servlet.FilterChain)\r\n     *\/\r\n    @Override\r\n    public void doFilter(ServletRequest req, ServletResponse resp, FilterChain chain) throws IOException,\r\n            ServletException {\r\n\r\n        HttpServletRequest hreq = (HttpServletRequest) req;\r\n        HttpServletResponse hresp = (HttpServletResponse) resp;\r\n\r\n        \/\/ verify the noun + uuid\r\n        if (!checkPathInfo(hreq, hresp)) {\r\n            return;\r\n        }\r\n\r\n        \/\/ do additional tests, e.g., inspect payload\r\n\r\n        chain.doFilter(req, resp);\r\n    }\r\n\r\n    \/**\r\n     * @see javax.servlet.Filter#destroy()\r\n     *\/\r\n    @Override\r\n    public void destroy() {\r\n    }\r\n\r\n    \/**\r\n     * Check the pathInfo. We know that all paths should have the form\r\n     * \/{noun}\/{uuid}\/...\r\n     * \r\n     * @param req\r\n     * @return\r\n     *\/\r\n    public boolean checkPathInfo(HttpServletRequest req, HttpServletResponse resp) {\r\n        \/\/ this pattern only handles noun and UUID, no additional parameters.\r\n        Pattern pattern = Pattern.compile(\"^\/([\\\\p{Alpha}]+)(\/?([\\\\p{XDigit}-]+)?)?\");\r\n        Matcher matcher = pattern.matcher(req.getPathInfo());\r\n        matcher.find();\r\n\r\n        \/\/ verify this is a valid noun.\r\n        if ((matcher.groupCount() >= 1) && !validNouns.contains(matcher.group(1))) {\r\n            \/\/ LOG.info(\"unrecognized noun\");\r\n            LOG.info(\"unrecognized noun: '\" + matcher.group(1) + \"'\");\r\n            resp.setStatus(HttpServletResponse.SC_BAD_REQUEST);\r\n            return false;\r\n        }\r\n\r\n        \/\/ verify this is a valid verb.\r\n        if ((matcher.groupCount() >= 4) && !StudentUtil.isPossibleUuid(matcher.group(4))) {\r\n            LOG.info(\"invalid UUID\");\r\n            resp.setStatus(HttpServletResponse.SC_BAD_REQUEST);\r\n            return false;\r\n        }\r\n\r\n        return true;\r\n    }\r\n}<\/pre>\n
There\u2019s no reason why we can\u2019t also inspect the payload. For instance we can verify that dates, phone numbers and credit card numbers are properly formed; or that names only include letters (including non-Latin characters like \u00f1), spaces and apostrophes. (Think \u201cAnne-Marie Pe\u00f1a O\u2019Brien\u201d.) It\u2019s important to remember that these checks are not for \u2018valid\u2019 data \u2013 it\u2019s to eliminate clearly \u2018invalid\u2019 data.<\/p>\n
We must add the filter to our web.xml file.<\/p>\n
web.xml<\/strong><\/p>\n
<filter>\r\n    <filter-name>REST parameter filter<\/filter-name>\r\n    <filter-class>com.invariantproperties.sandbox.student.webservice.security.RestParameterFilter<\/filter-class>\r\n     <init-param>\r\n        <param-name>valid-nouns<\/param-name>\r\n        <param-value>classroom,course,instructor,section,student,term,testRun<\/param-value>\r\n    <\/init-param>\r\n<\/filter>\r\n\r\n<filter-mapping>\r\n    <filter-name>REST parameter filter<\/filter-name>\r\n    <servlet-name>REST dispatcher<\/servlet-name>\r\n<\/filter-mapping><\/pre>\n
ModSecurity<\/h2>\n
It\u2019s easy to write a filter for simple elements like phone numbers and names but plaintext fields are another matter. These fields need the maximum flexibility while at the same time we want to minimize the risk of XSS and other attacks.<\/p>\n
A good resource along these lines is ModSecurity. This was originally an Apache module but it has been adopted by Trustwave Spider Labs. It sits on the web server \u2013 not the webapp \u2013 and inspects the data crossing it. A recent port (in summer 2013) allows it to be set up using a servlet filter instead of an external reverse proxy. (It uses JNI to instrument the containing appserver.)<\/p>\n