Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API Endpoint Phase 2: Add Endpoint route in Servlet Framework #270

Open
wants to merge 18 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
530ef1b
API Endpoint Phase 2: Add Endpoint route in Servlet Framework
IshikaDawda Jun 7, 2024
b842390
API Endpoint Phase 2: Add Endpoint route in Servlet Framework
IshikaDawda Jun 7, 2024
0576b23
Add Unit tests for route detection for Servlet Framework
IshikaDawda Jun 7, 2024
b823e8d
Add Endpoint route in Servlet-3.0 Framework
IshikaDawda Jun 7, 2024
5f3301b
Add Unit tests for route detection for Servlet-3.0
IshikaDawda Jun 7, 2024
c40ee48
API Endpoint Phase 2: Add Endpoint route in Servlet Framework
IshikaDawda Jun 13, 2024
5ca6984
Fix for NR-280811
IshikaDawda Jun 14, 2024
a5cf4e8
Merge branch 'refs/heads/main' into feature/api-endpoint/Servlet-NR-2…
IshikaDawda Jul 16, 2024
7960d89
Consider corner cases for servlet route detection
IshikaDawda Jul 16, 2024
d1a2e22
Fix for NR-286896, where incorrect route calculated when empty route …
IshikaDawda Jul 25, 2024
a4bc3ce
Route detection support for servlet framework
IshikaDawda Sep 20, 2024
357624a
Merge branch 'refs/heads/main' into feature/api-endpoint/Servlet-NR-2…
IshikaDawda Sep 20, 2024
d1d5d8d
Merge branch 'refs/heads/main' into feature/api-endpoint/Servlet-NR-2…
IshikaDawda Sep 20, 2024
b69814b
Fix for extension based endpoints present in servlet
IshikaDawda Sep 20, 2024
22e5548
Merge branch 'refs/heads/main' into feature/api-endpoint/Servlet-NR-2…
IshikaDawda Nov 21, 2024
f2b3fb2
NR-338563: Fix for NR-282698 where route not detected for diff. mappi…
IshikaDawda Nov 21, 2024
b6443b7
Fix for NR-266822 where false APIs reported for servlet using applica…
IshikaDawda Nov 22, 2024
df473df
Fix for NR-266822 where false APIs reported for servlet using applica…
IshikaDawda Nov 22, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,21 @@
import com.newrelic.api.agent.security.instrumentation.helpers.*;
import com.newrelic.api.agent.security.schema.AgentMetaData;
import com.newrelic.api.agent.security.schema.ApplicationURLMapping;
import com.newrelic.api.agent.security.schema.Framework;
import com.newrelic.api.agent.security.schema.HttpRequest;
import com.newrelic.api.agent.security.schema.StringUtils;
import com.newrelic.api.agent.security.schema.policy.AgentPolicy;
import com.newrelic.api.agent.security.utils.logging.LogLevel;

import javax.servlet.ServletConfig;
import javax.servlet.ServletContext;
import javax.servlet.ServletRegistration;
import javax.servlet.http.HttpServletRequest;
import java.util.Collection;
import java.util.Enumeration;
import java.util.Iterator;
import java.util.Map;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

public class HttpServletHelper {

Expand Down Expand Up @@ -154,4 +158,31 @@ else if(path.endsWith(".jsp") || path.endsWith(".jspx") || path.endsWith(".JSP")
NewRelicSecurity.getAgent().log(LogLevel.WARNING, String.format(GenericHelper.ERROR_WHILE_GETTING_APP_ENDPOINTS, SERVLET_2_4, e.getMessage()), e, HttpServletHelper.class.getName());
}
}

public static void setRoute(HttpServletRequest request, HttpRequest securityRequest, ServletConfig servletConfig) {
try {
if (URLMappingsHelper.getApplicationURLMappings().isEmpty()){
return;
}
String servletPath = request.getServletPath();
if (URLMappingsHelper.getApplicationURLMappings().contains(new ApplicationURLMapping(URLMappingsHelper.WILDCARD, servletPath))) {
securityRequest.setRoute(servletPath);
} else if (servletConfig != null) {
ServletRegistration registration = servletConfig.getServletContext().getServletRegistration(servletConfig.getServletName());
if (registration != null && registration.getMappings() != null && !registration.getMappings().isEmpty()) {
for (String mapping : registration.getMappings()) {
Pattern pattern = Pattern.compile(StringUtils.replace(mapping, URLMappingsHelper.WILDCARD, ".*"));
Matcher matcher = pattern.matcher(servletPath);
if (matcher.matches()) {
securityRequest.setRoute(mapping);
break;
}
}
}
}
NewRelicSecurity.getAgent().getSecurityMetaData().getMetaData().setFramework(Framework.SERVLET);
} catch (Exception e){
NewRelicSecurity.getAgent().log(LogLevel.WARNING, String.format(GenericHelper.ERROR_WHILE_GETTING_ROUTE_FOR_INCOMING_REQUEST, SERVLET_2_4, e.getMessage()), e, HttpServletHelper.class.getName());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import com.newrelic.api.agent.security.instrumentation.helpers.LowSeverityHelper;
import com.newrelic.api.agent.security.instrumentation.helpers.ServletHelper;
import com.newrelic.api.agent.security.schema.AgentMetaData;
import com.newrelic.api.agent.security.schema.Framework;
import com.newrelic.api.agent.security.schema.HttpRequest;
import com.newrelic.api.agent.security.schema.SecurityMetaData;
import com.newrelic.api.agent.security.schema.exceptions.NewRelicSecurityException;
Expand All @@ -26,6 +27,9 @@ public abstract class FilterChain_Instrumentation {

public void doFilter(ServletRequest request, ServletResponse response) throws IOException, ServletException {
boolean isServletLockAcquired = acquireServletLockIfPossible();
if (NewRelicSecurity.isHookProcessingActive() && request instanceof HttpServletRequest){
HttpServletHelper.setRoute((HttpServletRequest) request, NewRelicSecurity.getAgent().getSecurityMetaData().getRequest(), null);
}
if(isServletLockAcquired) {
preprocessSecurityHook(request, response);
}
Expand Down Expand Up @@ -68,7 +72,6 @@ private void preprocessSecurityHook(ServletRequest request, ServletResponse resp
}

HttpServletHelper.processHttpRequestHeader(httpServletRequest, securityRequest);

securityMetaData.setTracingHeaderValue(HttpServletHelper.getTraceHeader(securityRequest.getHeaders()));

securityRequest.setProtocol(httpServletRequest.getScheme());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import com.newrelic.api.agent.security.instrumentation.helpers.LowSeverityHelper;
import com.newrelic.api.agent.security.instrumentation.helpers.ServletHelper;
import com.newrelic.api.agent.security.schema.AgentMetaData;
import com.newrelic.api.agent.security.schema.Framework;
import com.newrelic.api.agent.security.schema.HttpRequest;
import com.newrelic.api.agent.security.schema.SecurityMetaData;
import com.newrelic.api.agent.security.schema.exceptions.NewRelicSecurityException;
Expand All @@ -28,6 +29,9 @@ public abstract class Filter_Instrumentation {
public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain)
throws IOException, ServletException {
boolean isServletLockAcquired = acquireServletLockIfPossible();
if (NewRelicSecurity.isHookProcessingActive() && request instanceof HttpServletRequest){
HttpServletHelper.setRoute((HttpServletRequest) request, NewRelicSecurity.getAgent().getSecurityMetaData().getRequest(), null);
}
if(isServletLockAcquired) {
preprocessSecurityHook(request, response);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ public abstract class Servlet_Instrumentation {

public void service(ServletRequest_Instrumentation request, ServletResponse_Instrumentation response) {
boolean isServletLockAcquired = acquireServletLockIfPossible();
if (NewRelicSecurity.isHookProcessingActive() && request instanceof HttpServletRequest){
HttpServletHelper.setRoute((HttpServletRequest) request, NewRelicSecurity.getAgent().getSecurityMetaData().getRequest(), getServletConfig());
}
if(isServletLockAcquired) {
preprocessSecurityHook(request, response);
}
Expand Down Expand Up @@ -140,4 +143,6 @@ private void releaseServletLock() {
HttpServletHelper.releaseServletLock();
} catch (Throwable e) {}
}

public abstract ServletConfig getServletConfig();
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,19 @@
import com.newrelic.api.agent.security.instrumentation.helpers.GenericHelper;
import com.newrelic.api.agent.security.instrumentation.helpers.URLMappingsHelper;
import com.newrelic.api.agent.security.schema.ApplicationURLMapping;
import com.newrelic.api.agent.security.schema.Framework;
import com.newrelic.api.agent.security.schema.HttpRequest;
import com.newrelic.api.agent.security.schema.StringUtils;
import com.newrelic.api.agent.security.utils.logging.LogLevel;

import javax.servlet.ServletConfig;
import javax.servlet.ServletContext;
import javax.servlet.ServletRegistration;
import javax.servlet.http.HttpServletRequest;
import java.util.Collection;
import java.util.Map;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

public class HttpServletHelper {
private static final String WILDCARD = "*";
Expand Down Expand Up @@ -48,4 +55,31 @@ else if(path.endsWith(".jsp") || path.endsWith(".jspx") || path.endsWith(".JSP")
NewRelicSecurity.getAgent().log(LogLevel.WARNING, String.format(GenericHelper.ERROR_WHILE_GETTING_APP_ENDPOINTS, SERVLET_3_0, e.getMessage()), e, HttpServletHelper.class.getName());
}
}

public static void setRoute(HttpServletRequest request, HttpRequest securityRequest, ServletConfig servletConfig) {
try {
if (URLMappingsHelper.getApplicationURLMappings().isEmpty()){
return;
}
String servletPath = request.getServletPath();
if (URLMappingsHelper.getApplicationURLMappings().contains(new ApplicationURLMapping(URLMappingsHelper.WILDCARD, servletPath))) {
securityRequest.setRoute(servletPath);
} else if (servletConfig != null) {
ServletRegistration registration = servletConfig.getServletContext().getServletRegistration(servletConfig.getServletName());
if (registration != null && registration.getMappings() != null && !registration.getMappings().isEmpty()) {
for (String mapping : registration.getMappings()) {
Pattern pattern = Pattern.compile(StringUtils.replace(mapping, URLMappingsHelper.WILDCARD, ".*"));
Matcher matcher = pattern.matcher(servletPath);
if (matcher.matches()) {
securityRequest.setRoute(mapping);
break;
}
}
}
}
NewRelicSecurity.getAgent().getSecurityMetaData().getMetaData().setFramework(Framework.SERVLET);
} catch (Exception e){
NewRelicSecurity.getAgent().log(LogLevel.WARNING, String.format(GenericHelper.ERROR_WHILE_GETTING_ROUTE_FOR_INCOMING_REQUEST, SERVLET_3_0, e.getMessage()), e, HttpServletHelper.class.getName());
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package javax.servlet;

import com.newrelic.agent.security.instrumentation.servlet30.HttpServletHelper;
import com.newrelic.api.agent.security.NewRelicSecurity;
import com.newrelic.api.agent.security.schema.SecurityMetaData;
import com.newrelic.api.agent.weaver.MatchType;
import com.newrelic.api.agent.weaver.Weave;
import com.newrelic.api.agent.weaver.Weaver;

import javax.servlet.http.HttpServletRequest;

@Weave(type = MatchType.Interface, originalName = "javax.servlet.Servlet")
public abstract class Servlet_Instrumentation {

public void service(ServletRequest req, ServletResponse res){
if (NewRelicSecurity.isHookProcessingActive() && req instanceof HttpServletRequest){
HttpServletHelper.setRoute((HttpServletRequest) req, NewRelicSecurity.getAgent().getSecurityMetaData().getRequest(), getServletConfig());
}
Weaver.callOriginal();
}

public abstract ServletConfig getServletConfig();

}
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,20 @@

import com.newrelic.agent.security.introspec.InstrumentationTestConfig;
import com.newrelic.agent.security.introspec.SecurityInstrumentationTestRunner;
import com.newrelic.api.agent.Trace;
import com.newrelic.api.agent.security.instrumentation.helpers.URLMappingsHelper;
import com.newrelic.api.agent.security.schema.ApplicationURLMapping;
import org.apache.catalina.servlets.DefaultServlet;
import com.newrelic.api.agent.security.schema.Framework;
import com.newrelic.api.agent.security.schema.SecurityMetaData;
import org.junit.Assert;
import org.junit.ClassRule;
import org.junit.Test;
import org.junit.runner.RunWith;

import java.io.IOException;
import java.net.HttpURLConnection;
import java.net.URISyntaxException;
import java.net.URL;
import java.util.Iterator;

@RunWith(SecurityInstrumentationTestRunner.class)
Expand All @@ -32,4 +38,21 @@ public void testURLMappings() {
ApplicationURLMapping mapping2 = mappings.next();
Assert.assertEquals("URL Mappings", new ApplicationURLMapping(method, "/test", handler), mapping2);
}

@Test
public void testRoute() throws IOException, URISyntaxException {
connect();
SecurityMetaData metaData = SecurityInstrumentationTestRunner.getIntrospector().getSecurityMetaData();
Assert.assertEquals( "Incorrect Route Detected","/test", metaData.getRequest().getRoute());
Assert.assertEquals("Incorrect Framework detected", Framework.SERVLET.name(), metaData.getMetaData().getFramework());
}

@Trace(dispatcher = true)
private void connect() throws IOException, URISyntaxException {
URL u = server.getEndPoint().toURL();
HttpURLConnection conn = (HttpURLConnection) u.openConnection();
conn.setRequestProperty("content-type", "text/plain; charset=utf-8");
conn.connect();
conn.getResponseCode();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import org.apache.catalina.Context;
import org.apache.catalina.LifecycleState;
import org.apache.catalina.connector.Connector;
import org.apache.catalina.servlets.DefaultServlet;
import org.apache.catalina.startup.Tomcat;
import org.apache.catalina.webresources.TomcatURLStreamHandlerFactory;
import org.apache.tomcat.util.http.fileupload.FileUtils;
Expand All @@ -19,6 +18,8 @@
import java.io.File;
import java.io.IOException;
import java.net.ServerSocket;
import java.net.URI;
import java.net.URISyntaxException;
import java.util.Collections;
import java.util.Set;

Expand Down Expand Up @@ -91,6 +92,10 @@ private void stop() {
}
}
}

public URI getEndPoint() throws URISyntaxException {
return new URI("http://localhost:" + port + "/test");
}
}
class MyServlet extends HttpServlet {
@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,17 @@
import com.newrelic.api.agent.security.instrumentation.helpers.*;
import com.newrelic.api.agent.security.schema.AgentMetaData;
import com.newrelic.api.agent.security.schema.ApplicationURLMapping;
import com.newrelic.api.agent.security.schema.Framework;
import com.newrelic.api.agent.security.schema.HttpRequest;
import com.newrelic.api.agent.security.schema.policy.AgentPolicy;
import com.newrelic.api.agent.security.utils.logging.LogLevel;
import jakarta.servlet.ServletContext;
import jakarta.servlet.ServletRegistration;
import jakarta.servlet.http.HttpServletMapping;
import jakarta.servlet.http.HttpServletRequest;

import java.util.Collection;
import java.util.Enumeration;
import java.util.Iterator;
import java.util.Map;

public class HttpServletHelper {
Expand Down Expand Up @@ -154,4 +155,19 @@ else if(path.endsWith(".jsp") || path.endsWith(".jspx") || path.endsWith(".JSP")
NewRelicSecurity.getAgent().log(LogLevel.WARNING, String.format(GenericHelper.ERROR_WHILE_GETTING_APP_ENDPOINTS, SERVLET_5_0, e.getMessage()), e, HttpServletHelper.class.getName());
}
}

public static void setRoute(HttpServletRequest request){
try {
if (!NewRelicSecurity.isHookProcessingActive() || URLMappingsHelper.getApplicationURLMappings().isEmpty() || (!NewRelicSecurity.getAgent().getSecurityMetaData().getMetaData().getFramework().isEmpty() && !NewRelicSecurity.getAgent().getSecurityMetaData().getMetaData().getFramework().equals(Framework.SERVLET.name()))){
return;
}
HttpServletMapping mapping = request.getHttpServletMapping();
if (mapping != null) {
NewRelicSecurity.getAgent().getSecurityMetaData().getRequest().setRoute(mapping.getPattern());
}
NewRelicSecurity.getAgent().getSecurityMetaData().getMetaData().setFramework(Framework.SERVLET);
} catch (Exception e){
NewRelicSecurity.getAgent().log(LogLevel.WARNING, String.format(GenericHelper.ERROR_WHILE_GETTING_ROUTE_FOR_INCOMING_REQUEST, SERVLET_5_0, e.getMessage()), e, HttpServletHelper.class.getName());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ public abstract class FilterChain_Instrumentation {

public void doFilter(ServletRequest request, ServletResponse response) throws IOException, ServletException {
boolean isServletLockAcquired = acquireServletLockIfPossible();
if (request instanceof HttpServletRequest) {
HttpServletHelper.setRoute((HttpServletRequest)request);
}
if(isServletLockAcquired) {
preprocessSecurityHook(request, response);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ public abstract class Filter_Instrumentation {
public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain)
throws IOException, ServletException {
boolean isServletLockAcquired = acquireServletLockIfPossible();
if (request instanceof HttpServletRequest) {
HttpServletHelper.setRoute((HttpServletRequest)request);
}
if(isServletLockAcquired) {
preprocessSecurityHook(request, response);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ public abstract class Servlet_Instrumentation {

public void service(ServletRequest_Instrumentation request, ServletResponse_Instrumentation response) {
boolean isServletLockAcquired = acquireServletLockIfPossible();
if (request instanceof HttpServletRequest) {
HttpServletHelper.setRoute((HttpServletRequest)request);
}
if(isServletLockAcquired) {
preprocessSecurityHook(request, response);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ public void testPost() throws Exception {
AgentMetaData metaData = introspector.getSecurityMetaData().getMetaData();
Assert.assertTrue(metaData.isUserLevelServiceMethodEncountered());
Assert.assertNotNull(metaData.getServiceTrace());
Assert.assertEquals("Incorrect route detected", "/*", introspector.getSecurityMetaData().getRequest().getRoute());

}
@Test
public void testDelete() throws Exception {
Expand All @@ -41,6 +43,8 @@ public void testDelete() throws Exception {
AgentMetaData metaData = introspector.getSecurityMetaData().getMetaData();
Assert.assertTrue(metaData.isUserLevelServiceMethodEncountered());
Assert.assertNotNull(metaData.getServiceTrace());

Assert.assertEquals("Incorrect route detected", "/*", introspector.getSecurityMetaData().getRequest().getRoute());
}
@Test
public void testPUT() throws Exception {
Expand All @@ -50,6 +54,8 @@ public void testPUT() throws Exception {
AgentMetaData metaData = introspector.getSecurityMetaData().getMetaData();
Assert.assertTrue(metaData.isUserLevelServiceMethodEncountered());
Assert.assertNotNull(metaData.getServiceTrace());

Assert.assertEquals("Incorrect route detected", "/*", introspector.getSecurityMetaData().getRequest().getRoute());
}

@Test
Expand All @@ -60,6 +66,8 @@ public void testHEAD() throws Exception {
AgentMetaData metaData = introspector.getSecurityMetaData().getMetaData();
Assert.assertTrue(metaData.isUserLevelServiceMethodEncountered());
Assert.assertNotNull(metaData.getServiceTrace());

Assert.assertEquals("Incorrect route detected", "/*", introspector.getSecurityMetaData().getRequest().getRoute());
}
@Test
public void testGET() throws Exception {
Expand All @@ -69,6 +77,8 @@ public void testGET() throws Exception {
AgentMetaData metaData = introspector.getSecurityMetaData().getMetaData();
Assert.assertTrue(metaData.isUserLevelServiceMethodEncountered());
Assert.assertNotNull(metaData.getServiceTrace());

Assert.assertEquals("Incorrect route detected", "/*", introspector.getSecurityMetaData().getRequest().getRoute());
}

@Trace(dispatcher = true)
Expand Down
Loading
Loading