Skip to content

Commit 8c46b8c

Browse files
IshikaDawdaIshikaDawda
committed
Merge branch 'refs/heads/main' into feature/sa-dashboard-metric
# Conflicts: # newrelic-security-agent/src/main/java/com/newrelic/agent/security/intcodeagent/websocket/WSClient.java
2 parents 26ba104 + 44a4a17 commit 8c46b8c

File tree

24 files changed

+624
-305
lines changed

24 files changed

+624
-305
lines changed

‎gradle.properties

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# The agent version.
22
agentVersion=1.4.0
3-
jsonVersion=1.2.4
3+
jsonVersion=1.2.5
44
# Updated exposed NR APM API version.
55
nrAPIVersion=8.12.0
66

‎instrumentation-security/servlet-2.4/src/main/java/javax/servlet/http/HttpServletResponse_Instrumentation.java

+8-5
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
import com.newrelic.api.agent.security.schema.SecurityMetaData;
1010
import com.newrelic.api.agent.security.schema.StringUtils;
1111
import com.newrelic.api.agent.security.schema.exceptions.NewRelicSecurityException;
12-
import com.newrelic.api.agent.security.schema.operation.SecureCookieOperation;
1312
import com.newrelic.api.agent.security.schema.operation.SecureCookieOperationSet;
1413
import com.newrelic.api.agent.security.utils.logging.LogLevel;
1514
import com.newrelic.api.agent.weaver.MatchType;
@@ -56,12 +55,16 @@ private AbstractOperation preprocessSecurityHook(Cookie cookie, String className
5655
sameSiteStrict = StringUtils.containsIgnoreCase(cookie.getValue(), "SameSite=Strict");
5756
}
5857

59-
SecureCookieOperation operation = new SecureCookieOperation(Boolean.toString(isSecure ), isSecure, isHttpOnly, sameSiteStrict, cookie.getValue(), className, methodName);
60-
operation.setLowSeverityHook(true);
61-
NewRelicSecurity.getAgent().registerOperation(operation);
58+
SecureCookieOperationSet operations = NewRelicSecurity.getAgent().getSecurityMetaData().getCustomAttribute("SECURE_COOKIE_OPERATION", SecureCookieOperationSet.class);
59+
if(operations == null){
60+
operations = new SecureCookieOperationSet(className, methodName);;
61+
operations.setLowSeverityHook(true);
62+
NewRelicSecurity.getAgent().getSecurityMetaData().addCustomAttribute("SECURE_COOKIE_OPERATION", operations);
63+
}
64+
operations.addOperation(cookie.getName(), cookie.getValue(), isSecure, isHttpOnly, sameSiteStrict);
6265
// NewRelicSecurity.getAgent().registerOperation(operation);
6366

64-
return operation;
67+
return operations;
6568
} catch (Throwable e) {
6669
if (e instanceof NewRelicSecurityException) {
6770
NewRelicSecurity.getAgent().log(LogLevel.WARNING, String.format(GenericHelper.SECURITY_EXCEPTION_MESSAGE, HttpServletHelper.SERVLET_2_4, e.getMessage()), e, HttpServletResponse_Instrumentation.class.getName());

‎instrumentation-security/servlet-2.4/src/test/java/com/nr/agent/security/instrumentation/servlet24/HttpSessionTest.java

+120-33
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
import com.newrelic.agent.security.introspec.SecurityIntrospector;
66
import com.newrelic.api.agent.security.schema.AbstractOperation;
77
import com.newrelic.api.agent.security.schema.VulnerabilityCaseType;
8-
import com.newrelic.api.agent.security.schema.operation.SecureCookieOperation;
8+
import com.newrelic.api.agent.security.schema.operation.SecureCookieOperationSet;
99
import com.newrelic.api.agent.security.schema.operation.TrustBoundaryOperation;
1010
import org.junit.Assert;
1111
import org.junit.ClassRule;
@@ -18,6 +18,7 @@
1818
import java.net.HttpURLConnection;
1919
import java.net.URISyntaxException;
2020
import java.net.URL;
21+
import java.util.Iterator;
2122
import java.util.List;
2223

2324
@RunWith(SecurityInstrumentationTestRunner.class)
@@ -29,9 +30,7 @@ public class HttpSessionTest {
2930

3031
@Test
3132
public void testSessionSetAttribute() throws IOException, URISyntaxException {
32-
String method = "GET";
33-
String POST_PARAMS = "hook=readLine";
34-
makeRequest(method, POST_PARAMS, "set");
33+
makeRequest("set");
3534

3635
SecurityIntrospector introspector = SecurityInstrumentationTestRunner.getIntrospector();
3736
List<AbstractOperation> operations = introspector.getOperations();
@@ -61,9 +60,7 @@ else if(i==1){
6160

6261
@Test
6362
public void testSessionPutValue() throws IOException, URISyntaxException {
64-
String method = "GET";
65-
String POST_PARAMS = "hook=readLine";
66-
makeRequest(method, POST_PARAMS, "put");
63+
makeRequest("put");
6764

6865
SecurityIntrospector introspector = SecurityInstrumentationTestRunner.getIntrospector();
6966
List<AbstractOperation> operations = introspector.getOperations();
@@ -84,56 +81,146 @@ public void testSessionPutValue() throws IOException, URISyntaxException {
8481

8582
@Test
8683
public void testAddCookie() throws IOException, URISyntaxException {
87-
String method = "GET";
88-
String POST_PARAMS = "hook=readLine";
89-
makeRequest(method, POST_PARAMS, "cookie");
84+
makeRequest("securecookie");
9085

9186
SecurityIntrospector introspector = SecurityInstrumentationTestRunner.getIntrospector();
9287
List<AbstractOperation> operations = introspector.getOperations();
9388
Assert.assertTrue("No operations detected", operations.size() > 0);
9489
Assert.assertTrue("Unexpected operation count detected", operations.size() == 1 || operations.size() == 2);
95-
SecureCookieOperation targetOperation = null;
96-
for (AbstractOperation operation : operations) {
97-
if (operation instanceof SecureCookieOperation)
98-
targetOperation = (SecureCookieOperation) operation;
99-
};
100-
Assert.assertNotNull("No target operation detected", targetOperation);
101-
Assert.assertEquals("Wrong case-type detected", VulnerabilityCaseType.SECURE_COOKIE, targetOperation.getCaseType());
102-
Assert.assertEquals("Wrong key detected", "false", targetOperation.getValue());
90+
SecureCookieOperationSet targetOperation = null;
91+
targetOperation = verifySecureCookieOp(operations);
92+
93+
Assert.assertEquals(1, targetOperation.getOperations().size());
94+
Iterator<SecureCookieOperationSet.SecureCookieOperation> secureCookieOps = targetOperation.getOperations().iterator();
95+
Assert.assertTrue(secureCookieOps.hasNext());
96+
97+
SecureCookieOperationSet.SecureCookieOperation secureCookieOp = secureCookieOps.next();
98+
verifySecureCookie(secureCookieOp, "key", false, true);
10399
}
104100

105101
@Test
106102
public void testAddCookie1() throws IOException, URISyntaxException {
107-
String method = "GET";
108-
String POST_PARAMS = "hook=readLine";
109-
makeRequest(method, POST_PARAMS, "securecookie");
103+
makeRequest("cookie");
110104

111105
SecurityIntrospector introspector = SecurityInstrumentationTestRunner.getIntrospector();
112106
List<AbstractOperation> operations = introspector.getOperations();
113-
Assert.assertTrue("No operations detected", operations.size() > 0);
114-
Assert.assertTrue("Unexpected operation count detected", operations.size() == 1 || operations.size() == 2);
115-
SecureCookieOperation targetOperation = null;
116-
for (AbstractOperation operation : operations) {
117-
if (operation instanceof SecureCookieOperation)
118-
targetOperation = (SecureCookieOperation) operation;
119-
};
120-
Assert.assertNotNull("No target operation detected", targetOperation);
121-
Assert.assertEquals("Wrong case-type detected", VulnerabilityCaseType.SECURE_COOKIE, targetOperation.getCaseType());
122-
Assert.assertEquals("Wrong key detected", "true", targetOperation.getValue());
107+
108+
SecureCookieOperationSet targetOperation = verifySecureCookieOp(operations);
109+
Assert.assertEquals(1, targetOperation.getOperations().size());
110+
111+
Iterator<SecureCookieOperationSet.SecureCookieOperation> secureCookieOps = targetOperation.getOperations().iterator();
112+
Assert.assertTrue(secureCookieOps.hasNext());
113+
114+
SecureCookieOperationSet.SecureCookieOperation secureCookieOp = secureCookieOps.next();
115+
verifySecureCookie(secureCookieOp, "key", false, false);
116+
}
117+
118+
@Test
119+
public void testAddSecureCookies() throws IOException, URISyntaxException {
120+
makeRequest("secure_cookies");
121+
122+
SecurityIntrospector introspector = SecurityInstrumentationTestRunner.getIntrospector();
123+
List<AbstractOperation> operations = introspector.getOperations();
124+
125+
SecureCookieOperationSet targetOperation = verifySecureCookieOp(operations);
126+
Assert.assertEquals(2, targetOperation.getOperations().size());
127+
128+
for (SecureCookieOperationSet.SecureCookieOperation secureCookieOp : targetOperation.getOperations()) {
129+
if (secureCookieOp.getName().equals("secure-cookie-1")) {
130+
verifySecureCookie(secureCookieOp, "secure-cookie-1", false, true);
131+
} else {
132+
verifySecureCookie(secureCookieOp, "secure-cookie-2", true, true);
133+
}
134+
}
135+
}
136+
137+
@Test
138+
public void testAddInSecureCookies() throws IOException, URISyntaxException {
139+
makeRequest("insecure_cookies");
140+
141+
SecurityIntrospector introspector = SecurityInstrumentationTestRunner.getIntrospector();
142+
List<AbstractOperation> operations = introspector.getOperations();
143+
144+
SecureCookieOperationSet targetOperation = verifySecureCookieOp(operations);
145+
Assert.assertEquals(2, targetOperation.getOperations().size());
146+
147+
for (SecureCookieOperationSet.SecureCookieOperation secureCookieOp : targetOperation.getOperations()) {
148+
if (secureCookieOp.getName().equals("insecure-cookie-1")) {
149+
verifySecureCookie(secureCookieOp, "insecure-cookie-1", false, false);
150+
} else {
151+
verifySecureCookie(secureCookieOp, "insecure-cookie-2", false, false);
152+
}
153+
}
123154
}
124155

125-
private void makeRequest( String Method, final String POST_PARAMS, String path) throws URISyntaxException, IOException{
156+
@Test
157+
public void testAddMultiSecureCookies() throws IOException, URISyntaxException {
158+
makeRequest("cookies");
159+
160+
SecurityIntrospector introspector = SecurityInstrumentationTestRunner.getIntrospector();
161+
List<AbstractOperation> operations = introspector.getOperations();
162+
163+
SecureCookieOperationSet targetOperation = verifySecureCookieOp(operations);
164+
Assert.assertEquals(2, targetOperation.getOperations().size());
165+
166+
for (SecureCookieOperationSet.SecureCookieOperation secureCookieOp : targetOperation.getOperations()) {
167+
if (secureCookieOp.getName().equals("insecure-cookie")) {
168+
verifySecureCookie(secureCookieOp, "insecure-cookie", false, false);
169+
} else {
170+
verifySecureCookie(secureCookieOp, "secure-cookie", false, true);
171+
}
172+
}
173+
}
174+
175+
@Test
176+
public void testSingleCookie() throws IOException, URISyntaxException {
177+
makeRequest("single-cookie");
178+
179+
SecurityIntrospector introspector = SecurityInstrumentationTestRunner.getIntrospector();
180+
List<AbstractOperation> operations = introspector.getOperations();
181+
182+
SecureCookieOperationSet targetOperation = verifySecureCookieOp(operations);
183+
Assert.assertEquals(1, targetOperation.getOperations().size());
184+
185+
Iterator<SecureCookieOperationSet.SecureCookieOperation> secureCookieOps = targetOperation.getOperations().iterator();
126186

187+
Assert.assertTrue(secureCookieOps.hasNext());
188+
SecureCookieOperationSet.SecureCookieOperation secureCookieOp = secureCookieOps.next();
189+
verifySecureCookie(secureCookieOp, "cookie", true, true);
190+
}
191+
192+
private void makeRequest(String path) throws URISyntaxException, IOException{
127193
URL u = server.getEndPoint("session/"+path).toURL();
128194
HttpURLConnection conn = (HttpURLConnection) u.openConnection();
129-
conn.setRequestMethod(Method);
130195
conn.setDoOutput(true);
131196
conn.setRequestProperty("Connection", "Keep-Alive");
132197
conn.setRequestProperty("Cache-Control", "no-cache");
133198
conn.setRequestProperty("Content-Type", "multipart/form-data");
134199

135200
conn.connect();
136201
conn.getResponseCode();
202+
}
203+
204+
private SecureCookieOperationSet verifySecureCookieOp(List<AbstractOperation> operations) {
205+
SecureCookieOperationSet targetOperation = null;
206+
for (AbstractOperation operation : operations) {
207+
if (operation instanceof SecureCookieOperationSet) {
208+
targetOperation = (SecureCookieOperationSet) operation;
209+
}
210+
}
211+
212+
Assert.assertNotNull("No target operation detected", targetOperation);
213+
Assert.assertEquals("Wrong method detected", "addCookie", targetOperation.getMethodName());
214+
Assert.assertEquals("Wrong case-type detected", VulnerabilityCaseType.SECURE_COOKIE, targetOperation.getCaseType());
215+
Assert.assertTrue("isLowSeverityHook should be true", targetOperation.isLowSeverityHook());
216+
return targetOperation;
217+
}
137218

219+
private void verifySecureCookie(SecureCookieOperationSet.SecureCookieOperation secureCookieOp, String key, boolean isHttpOnly, boolean isSecure) {
220+
Assert.assertEquals("Wrong cookie key detected", key, secureCookieOp.getName());
221+
Assert.assertEquals("Wrong cookie value detected", "value", secureCookieOp.getValue());
222+
Assert.assertEquals(String.format("isHttpOnly should be %s", isHttpOnly), isHttpOnly, secureCookieOp.isHttpOnly());
223+
Assert.assertEquals(String.format("isSecure should be %s", isSecure), isSecure, secureCookieOp.isSecure());
224+
Assert.assertTrue(String.format("isSameSiteStrict should be %s", true), secureCookieOp.isSameSiteStrict());
138225
}
139226
}

‎instrumentation-security/servlet-2.4/src/test/java/com/nr/agent/security/instrumentation/servlet24/HttpTestServlet.java

+29-1
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,35 @@ else if(path.equals("/session/put")){
102102
cookie.setSecure(true);
103103
response.addCookie(cookie);
104104
} else if(path.equals("/session/cookie")){
105-
Cookie cookie = new Cookie("key1", "value1");
105+
// add insecure cookie to response
106+
response.addCookie(new Cookie("key", "value"));
107+
} else if (path.equals("/session/secure_cookies")) {
108+
// add multiple secure cookies to response
109+
Cookie cookie = new Cookie("secure-cookie-1", "value");
110+
cookie.setSecure(true);
111+
response.addCookie(cookie);
112+
113+
Cookie cookie1 = new Cookie("secure-cookie-2", "value");
114+
cookie1.setSecure(true);
115+
cookie1.setHttpOnly(true);
116+
cookie1.setComment("__SAME_SITE_STRICT__");
117+
response.addCookie(cookie1);
118+
} else if (path.equals("/session/insecure_cookies")) {
119+
// add multiple insecure cookies to response
120+
response.addCookie(new Cookie("insecure-cookie-1", "value"));
121+
response.addCookie(new Cookie("insecure-cookie-2", "value"));
122+
} else if (path.equals("/session/cookies")){
123+
// add multiple cookies to response
124+
Cookie cookie = new Cookie("secure-cookie", "value");
125+
cookie.setSecure(true);
126+
response.addCookie(cookie);
127+
128+
response.addCookie(new Cookie("insecure-cookie","value"));
129+
} else if (path.equals("/session/single-cookie")){
130+
Cookie cookie = new Cookie("cookie", "value");
131+
cookie.setSecure(true);
132+
cookie.setHttpOnly(true);
133+
cookie.setComment("__SAME_SITE_LAX__");
106134
response.addCookie(cookie);
107135
}
108136
}

‎instrumentation-security/servlet-5.0/src/main/java/jakarta/servlet/http/HttpServletResponse_Instrumentation.java

+8-5
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
import com.newrelic.api.agent.security.schema.SecurityMetaData;
1010
import com.newrelic.api.agent.security.schema.StringUtils;
1111
import com.newrelic.api.agent.security.schema.exceptions.NewRelicSecurityException;
12-
import com.newrelic.api.agent.security.schema.operation.SecureCookieOperation;
1312
import com.newrelic.api.agent.security.schema.operation.SecureCookieOperationSet;
1413
import com.newrelic.api.agent.security.utils.logging.LogLevel;
1514
import com.newrelic.api.agent.weaver.MatchType;
@@ -56,12 +55,16 @@ private AbstractOperation preprocessSecurityHook(Cookie cookie, String className
5655
sameSiteStrict = StringUtils.containsIgnoreCase(cookie.getValue(), "SameSite=Strict");
5756
}
5857

59-
SecureCookieOperation operation = new SecureCookieOperation(Boolean.toString(isSecure ), isSecure, isHttpOnly, sameSiteStrict, cookie.getValue(), className, methodName);
60-
operation.setLowSeverityHook(true);
61-
NewRelicSecurity.getAgent().registerOperation(operation);
58+
SecureCookieOperationSet operations = NewRelicSecurity.getAgent().getSecurityMetaData().getCustomAttribute("SECURE_COOKIE_OPERATION", SecureCookieOperationSet.class);
59+
if(operations == null){
60+
operations = new SecureCookieOperationSet(className, methodName);;
61+
operations.setLowSeverityHook(true);
62+
NewRelicSecurity.getAgent().getSecurityMetaData().addCustomAttribute("SECURE_COOKIE_OPERATION", operations);
63+
}
64+
operations.addOperation(cookie.getName(), cookie.getValue(), isSecure, isHttpOnly, sameSiteStrict);
6265
// NewRelicSecurity.getAgent().registerOperation(operation);
6366

64-
return operation;
67+
return operations;
6568
} catch (Throwable e) {
6669
if (e instanceof NewRelicSecurityException) {
6770
NewRelicSecurity.getAgent().log(LogLevel.WARNING, String.format(GenericHelper.SECURITY_EXCEPTION_MESSAGE, HttpServletHelper.SERVLET_5_0, e.getMessage()), e, HttpServletResponse_Instrumentation.class.getName());

0 commit comments

Comments
 (0)