Add validation for null elements in JSON array (#1157) (#1359)

(cherry picked from commit 960d420)

src/main/java/com/amazon/opendistroforelasticsearch/security/dlic/rest/validation/AbstractConfigurationValidator.java

@@ -188,6 +188,16 @@ public boolean validate() {
             return false;
         }
 
+        //null element in the values of all the possible keys with DataType as ARRAY
+        for (Entry<String, DataType> allowedKey : allowedKeys.entrySet()) {
+            JsonNode value = contentAsNode.get(allowedKey.getKey());
+            if (value != null) {
+                if (hasNullArrayElement(value)) {
+                    this.errorType = ErrorType.NULL_ARRAY_ELEMENT;
+                    return false;
+                }
+            }
+        }
         return valid;
     }
 
@@ -253,6 +263,10 @@ public XContentBuilder errorsAsXContent(RestChannel channel) {
                         builder.field(entry.getKey(), entry.getValue());
                     }
                     break;
+                case NULL_ARRAY_ELEMENT:
+                    builder.field("status", "error");
+                    builder.field("reason", ErrorType.NULL_ARRAY_ELEMENT.getMessage());
+                    break;
                 default:
                     builder.field("status", "error");
                     builder.field("reason", errorType.getMessage());
@@ -281,7 +295,8 @@ public static enum DataType {
     public static enum ErrorType {
         NONE("ok"), INVALID_CONFIGURATION("Invalid configuration"), INVALID_PASSWORD("Invalid password"), WRONG_DATATYPE("Wrong datatype"),
         BODY_NOT_PARSEABLE("Could not parse content of request."), PAYLOAD_NOT_ALLOWED("Request body not allowed for this action."),
-        PAYLOAD_MANDATORY("Request body required for this action."), SECURITY_NOT_INITIALIZED("Security index not initialized");
+        PAYLOAD_MANDATORY("Request body required for this action."), SECURITY_NOT_INITIALIZED("Security index not initialized"),
+        NULL_ARRAY_ELEMENT("`null` is not allowed as json array element");
 
         private String message;
 
@@ -297,4 +312,19 @@ public String getMessage() {
     protected final boolean hasParams() {
         return param != null && param.length > 0;
     }
-}
+
+    private boolean hasNullArrayElement(JsonNode node) {
+        for (JsonNode element: node) {
+            if(element.isNull()) {
+                if (node.isArray()) {
+                    return true;
+                }
+            } else if (element.isContainerNode()) {
+                if (hasNullArrayElement(element)) {
+                    return true;
+                }
+            }
+        }
+        return false;
+    }
+}

src/test/java/com/amazon/opendistroforelasticsearch/security/dlic/rest/api/ActionGroupsApiTest.java

@@ -359,4 +359,15 @@ public void testActionGroupsApiForNonSuperAdmin() throws Exception {
 
     }
 
+    @Test
+    public void checkNullElementsInArray() throws Exception{
+        setup();
+        rh.keystore = "restapi/kirk-keystore.jks";
+        rh.sendAdminCertificate = true;
+
+        String body = FileHelper.loadFile("restapi/actiongroup_null_array_element.json");
+        HttpResponse response = rh.executePutRequest("/_opendistro/_security/api/actiongroups/CRUD_UT", body, new Header[0]);
+        Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode());
+    }
+
 }

src/test/java/com/amazon/opendistroforelasticsearch/security/dlic/rest/api/NodesDnApiTest.java

@@ -19,6 +19,7 @@
 import com.amazon.opendistroforelasticsearch.security.auditlog.impl.AuditMessage;
 import com.amazon.opendistroforelasticsearch.security.auditlog.integration.TestAuditlogImpl;
 import com.amazon.opendistroforelasticsearch.security.support.ConfigConstants;
+import com.amazon.opendistroforelasticsearch.security.test.helper.file.FileHelper;
 import com.amazon.opendistroforelasticsearch.security.test.helper.rest.RestHelper.HttpResponse;
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
@@ -192,4 +193,15 @@ public void testNodesDnApiAuditComplianceLogging() throws Exception {
 
         assertThat(actualCategoryCounts, equalTo(expectedCategoryCounts));
     }
+
+    @Test
+    public void checkNullElementsInArray() throws Exception{
+        setup();
+        rh.keystore = "restapi/kirk-keystore.jks";
+        rh.sendAdminCertificate = true;
+
+        String body = FileHelper.loadFile("restapi/nodesdn_null_array_element.json");
+        HttpResponse response = rh.executePutRequest("_opendistro/_security/api/nodesdn", body, new Header[0]);
+        Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode());
+    }
 }

src/test/java/com/amazon/opendistroforelasticsearch/security/dlic/rest/api/RolesApiTest.java

@@ -502,4 +502,35 @@ public void testRolesApiForNonSuperAdmin() throws Exception {
 
     }
 
+    @Test
+    public void checkNullElementsInArray() throws Exception{
+        setup();
+        rh.keystore = "restapi/kirk-keystore.jks";
+        rh.sendAdminCertificate = true;
+
+        String body = FileHelper.loadFile("restapi/roles_null_array_element_cluster_permissions.json");
+        HttpResponse response = rh.executePutRequest("/_opendistro/_security/api/roles/opendistro_security_role_starfleet", body, new Header[0]);
+        Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode());
+
+        body = FileHelper.loadFile("restapi/roles_null_array_element_index_permissions.json");
+        response = rh.executePutRequest("/_opendistro/_security/api/roles/opendistro_security_role_starfleet", body, new Header[0]);
+        Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode());
+
+        body = FileHelper.loadFile("restapi/roles_null_array_element_tenant_permissions.json");
+        response = rh.executePutRequest("/_opendistro/_security/api/roles/opendistro_security_role_starfleet", body, new Header[0]);
+        Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode());
+
+        body = FileHelper.loadFile("restapi/roles_null_array_element_index_patterns.json");
+        response = rh.executePutRequest("/_opendistro/_security/api/roles/opendistro_security_role_starfleet", body, new Header[0]);
+        Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode());
+
+        body = FileHelper.loadFile("restapi/roles_null_array_element_masked_fields.json");
+        response = rh.executePutRequest("/_opendistro/_security/api/roles/opendistro_security_role_starfleet", body, new Header[0]);
+        Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode());
+
+        body = FileHelper.loadFile("restapi/roles_null_array_element_allowed_actions.json");
+        response = rh.executePutRequest("/_opendistro/_security/api/roles/opendistro_security_role_starfleet", body, new Header[0]);
+        Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode());
+    }
+
 }

src/test/java/com/amazon/opendistroforelasticsearch/security/dlic/rest/api/RolesMappingApiTest.java

@@ -392,4 +392,26 @@ public void testRolesMappingApiForNonSuperAdmin() throws Exception {
 		Assert.assertEquals(HttpStatus.SC_NOT_FOUND, response.getStatusCode());
 
 	}
+
+	@Test
+	public void checkNullElementsInArray() throws Exception{
+		setup();
+		rh.keystore = "restapi/kirk-keystore.jks";
+		rh.sendAdminCertificate = true;
+
+		String body = FileHelper.loadFile("restapi/rolesmapping_null_array_element_users.json");
+		HttpResponse response = rh.executePutRequest("/_opendistro/_security/api/rolesmapping/opendistro_security_role_starfleet_captains",
+													 body, new Header[0]);
+		Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode());
+
+		body = FileHelper.loadFile("restapi/rolesmapping_null_array_element_backend_roles.json");
+		response = rh.executePutRequest("/_opendistro/_security/api/rolesmapping/opendistro_security_role_starfleet_captains",
+													 body, new Header[0]);
+		Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode());
+
+		body = FileHelper.loadFile("restapi/rolesmapping_null_array_element_hosts.json");
+		response = rh.executePutRequest("/_opendistro/_security/api/rolesmapping/opendistro_security_role_starfleet_captains",
+										 body, new Header[0]);
+		Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode());
+	}
 }

src/test/java/com/amazon/opendistroforelasticsearch/security/dlic/rest/api/UserApiTest.java

@@ -600,4 +600,15 @@ public void testUserApiForNonSuperAdmin() throws Exception {
 
     }
 
+    @Test
+    public void checkNullElementsInArray() throws Exception{
+        setup();
+        rh.keystore = "restapi/kirk-keystore.jks";
+        rh.sendAdminCertificate = true;
+
+        String body = FileHelper.loadFile("restapi/users_null_array_element.json");
+        HttpResponse response = rh.executePutRequest("/_opendistro/_security/api/internalusers/picard", body, new Header[0]);
+        Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode());
+    }
+
 }

src/test/resources/restapi/actiongroup_null_array_element.json

@@ -0,0 +1,4 @@
+{
+ "allowed_actions": [null, "OPENDISTRO_SECURITY_WRITE"]
+}
+

src/test/resources/restapi/nodesdn_null_array_element.json

@@ -0,0 +1,3 @@
+{
+  "nodes_dn" : [null]
+}

src/test/resources/restapi/roles_null_array_element_allowed_actions.json

@@ -0,0 +1,9 @@
+{
+  "cluster_permissions" : ["cluster:monitor1*"],
+  "index_permissions" : [{
+    "index_patterns" : [ "*" ],
+    "masked_fields" : [ "abvfg", "*ip_dest*::/[0-9]{1,3}$/::XXX", "*ip_source*::/[0-9]{1,3}$/::XXX::/^[0-9]{1,3}/::***", "customer.name::SHA-512", "/[0-9]{1,3}/::SHA-512" ],
+    "allowed_actions" : [ "123", null ]
+  }],
+  "tenant_permissions" : [ ]
+}

src/test/resources/restapi/roles_null_array_element_cluster_permissions.json

@@ -0,0 +1,9 @@
+{
+  "cluster_permissions" : [null],
+  "index_permissions" : [ {
+    "index_patterns" : [ "*" ],
+    "masked_fields" : [ "abvfg", "*ip_dest*::/[0-9]{1,3}$/::XXX", "*ip_source*::/[0-9]{1,3}$/::XXX::/^[0-9]{1,3}/::***", "customer.name::SHA-512", "/[0-9]{1,3}/::SHA-512" ],
+    "allowed_actions" : [ "123", "indices:data/read/*" ]
+  } ],
+  "tenant_permissions" : [ ]
+}

src/test/resources/restapi/roles_null_array_element_index_patterns.json

@@ -0,0 +1,9 @@
+{
+  "cluster_permissions" : ["cluster:monitor1*"],
+  "index_permissions" : [{
+    "index_patterns" : [null],
+    "masked_fields" : [ "abvfg", "*ip_dest*::/[0-9]{1,3}$/::XXX", "*ip_source*::/[0-9]{1,3}$/::XXX::/^[0-9]{1,3}/::***", "customer.name::SHA-512", "/[0-9]{1,3}/::SHA-512" ],
+    "allowed_actions" : [ "123", "indices:data/read/*" ]
+  }],
+  "tenant_permissions" : [ ]
+}

src/test/resources/restapi/roles_null_array_element_index_permissions.json

@@ -0,0 +1,5 @@
+{
+  "cluster_permissions" : ["cluster:monitor1*"],
+  "index_permissions" : [null],
+  "tenant_permissions" : [ ]
+}

src/test/resources/restapi/roles_null_array_element_masked_fields.json

@@ -0,0 +1,9 @@
+{
+  "cluster_permissions" : ["cluster:monitor1*"],
+  "index_permissions" : [{
+    "index_patterns" : [ "*" ],
+    "masked_fields" : [ "abvfg", null, "*ip_source*::/[0-9]{1,3}$/::XXX::/^[0-9]{1,3}/::***", "customer.name::SHA-512", "/[0-9]{1,3}/::SHA-512" ],
+    "allowed_actions" : [ "123", "indices:data/read/*" ]
+  }],
+  "tenant_permissions" : [ ]
+}

src/test/resources/restapi/roles_null_array_element_tenant_permissions.json

@@ -0,0 +1,9 @@
+{
+  "cluster_permissions" : ["cluster:monitor1*"],
+  "index_permissions" : [{
+    "index_patterns" : [ "*" ],
+    "masked_fields" : [ "abvfg", "*ip_dest*::/[0-9]{1,3}$/::XXX", "*ip_source*::/[0-9]{1,3}$/::XXX::/^[0-9]{1,3}/::***", "customer.name::SHA-512", "/[0-9]{1,3}/::SHA-512" ],
+    "allowed_actions" : [ "123", "indices:data/read/*" ]
+  }],
+  "tenant_permissions" : [null]
+}

src/test/resources/restapi/rolesmapping_null_array_element_backend_roles.json

@@ -0,0 +1,14 @@
+{
+  "users": [
+    "user1",
+    "user2"
+  ],
+  "backend_roles": [
+    "captains",
+    null
+  ],
+  "hosts": [
+    "8.8.8.8",
+    "8.8.4.4"
+  ]
+}

src/test/resources/restapi/rolesmapping_null_array_element_hosts.json

@@ -0,0 +1,14 @@
+{
+  "users": [
+    "user1",
+    "user2"
+  ],
+  "backend_roles": [
+    "captains",
+    "role2"
+  ],
+  "hosts": [
+    null,
+    "8.8.4.4"
+  ]
+}

src/test/resources/restapi/rolesmapping_null_array_element_users.json

@@ -0,0 +1,11 @@
+{
+  "users": [
+    null
+  ],
+  "backend_roles": [
+    "captains"
+  ],
+  "hosts": [
+    "8.8.8.8"
+  ]
+}

src/test/resources/restapi/users_null_array_element.json

@@ -0,0 +1,5 @@
+{
+  "backend_roles": [
+    null
+  ]
+}