vyommani commented on code in PR #1266:
URL: https://github.com/apache/ranger/pull/1266#discussion_r4143872754
##########
security-admin/src/test/java/org/apache/ranger/biz/TestServiceMgr.java:
##########
@@ -385,4 +386,138 @@ public void
test13_validateConfig_allowsLegitimateExternalUrls() throws Exceptio
Assertions.assertNotNull(response);
Assertions.assertNotEquals(VXResponse.STATUS_ERROR,
response.getStatusCode());
}
+
+ @ParameterizedTest
+ @MethodSource("ssrfBypassConfigValues")
+ public void test14_lookupResource_rejectsSSRFBypassAttempts(String
configKey, String configValue) throws Exception {
+ ServiceMgr mgr = new ServiceMgr();
+ ServiceDBStore svcDBStore = mock(ServiceDBStore.class);
+ RangerServiceService svcService = mock(RangerServiceService.class);
+ TimedExecutor exec = mock(TimedExecutor.class);
+ setField(mgr, ServiceMgr.class, "svcDBStore", svcDBStore);
+ setField(mgr, ServiceMgr.class, "rangerSvcService", svcService);
+ setField(mgr, ServiceMgr.class, "timedExecutor", exec);
+
+ Map<String, String> bypassConfigs = new HashMap<>();
+ bypassConfigs.put(configKey, configValue);
+
+ RangerService svc = new RangerService();
+ svc.setType("knox");
+ svc.setName("knox-test");
+ svc.setConfigs(bypassConfigs);
+
+ when(svcDBStore.getServiceByName("knox-test")).thenReturn(svc);
+
when(svcService.getConfigsWithDecryptedPassword(any(RangerService.class))).thenReturn(bypassConfigs);
+
+ ResourceLookupContext ctx = new ResourceLookupContext();
+ Exception thrown = Assertions.assertThrows(Exception.class, () ->
mgr.lookupResource("knox-test", ctx, null));
+ Assertions.assertTrue(
+ thrown.getMessage().contains("is not allowed") ||
thrown.getMessage().contains("blocked host detected"),
+ "Unexpected message: " + thrown.getMessage());
+ }
+
+ @Test
+ public void
test15_validateConfig_requireSavedService_rejectsMissingService() throws
Exception {
+ ServiceMgr mgr = new ServiceMgr();
+ ServiceDBStore db = mock(ServiceDBStore.class);
+ RangerServiceService svcService = mock(RangerServiceService.class);
+ setField(mgr, ServiceMgr.class, "svcDBStore", db);
+ setField(mgr, ServiceMgr.class, "rangerSvcService", svcService);
+
+
PropertiesUtil.getPropertiesMap().put(ServiceMgr.PROP_SERVICE_CONFIG_VALIDATE_REQUIRE_SAVED_SERVICE,
"true");
+ try {
+ Map<String, String> cfg = new HashMap<>();
+ cfg.put("knox.url", "https://knox.example.com:8443/gateway");
+
+ RangerService svc = new RangerService();
+ svc.setName("knox-prod");
+ svc.setType("knox");
+ svc.setConfigs(cfg);
+
+
when(svcService.getConfigsWithDecryptedPassword(any(RangerService.class))).thenReturn(cfg);
+ when(db.getServiceByName("knox-prod")).thenReturn(null);
+
+ Exception thrown = Assertions.assertThrows(Exception.class, () ->
mgr.validateConfig(svc, null));
+ Assertions.assertTrue(thrown.getMessage().contains("requires a
saved service"));
+ } finally {
+
PropertiesUtil.getPropertiesMap().remove(ServiceMgr.PROP_SERVICE_CONFIG_VALIDATE_REQUIRE_SAVED_SERVICE);
+ }
+ }
+
+ @Test
+ public void
test16_validateConfig_requireSavedService_rejectsEndpointMismatch() throws
Exception {
+ ServiceMgr mgr = new ServiceMgr();
+ ServiceDBStore db = mock(ServiceDBStore.class);
+ RangerServiceService svcService = mock(RangerServiceService.class);
+ setField(mgr, ServiceMgr.class, "svcDBStore", db);
+ setField(mgr, ServiceMgr.class, "rangerSvcService", svcService);
+
+
PropertiesUtil.getPropertiesMap().put(ServiceMgr.PROP_SERVICE_CONFIG_VALIDATE_REQUIRE_SAVED_SERVICE,
"true");
+ try {
+ Map<String, String> savedCfg = new HashMap<>();
+ savedCfg.put("knox.url", "https://knox.example.com:8443/gateway");
+
+ Map<String, String> requestCfg = new HashMap<>();
+ requestCfg.put("knox.url",
"https://attacker.example.com:8443/gateway");
+
+ RangerService saved = new RangerService();
+ saved.setName("knox-prod");
+ saved.setType("knox");
+ saved.setConfigs(savedCfg);
+
+ RangerService request = new RangerService();
+ request.setName("knox-prod");
+ request.setType("knox");
+ request.setConfigs(requestCfg);
+
+ when(db.getServiceByName("knox-prod")).thenReturn(saved);
+
when(svcService.getConfigsWithDecryptedPassword(saved)).thenReturn(savedCfg);
+
when(svcService.getConfigsWithDecryptedPassword(request)).thenReturn(requestCfg);
+
+ Exception thrown = Assertions.assertThrows(Exception.class, () ->
mgr.validateConfig(request, null));
+ Assertions.assertTrue(thrown.getMessage().contains("must match the
saved service configuration"));
+ } finally {
+
PropertiesUtil.getPropertiesMap().remove(ServiceMgr.PROP_SERVICE_CONFIG_VALIDATE_REQUIRE_SAVED_SERVICE);
+ }
+ }
+
+ @Test
+ public void
test17_validateConfig_requireSavedService_allowsMatchingEndpoint() throws
Exception {
Review Comment:
This test catches every exception and only checks that the message is not
the saved-service error. validateConfig can fail for another reason and the
test still passes.
Please assert that validateConfig returns a non-error response, and let an
unexpected exception fail the test. Cases for a different port, a
comma-separated second host, and a jdbc: URL whose host extractHost cannot
parse would cover the gaps above.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]