[opendistro-1.4][backport] Add validation for null elements in JSON array (#1157)

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,17 @@ 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]);
+        Settings settings = Settings.builder().loadFromSource(response.getBody(), XContentType.JSON).build();
+        Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode());
+        Assert.assertEquals(AbstractConfigurationValidator.ErrorType.NULL_ARRAY_ELEMENT.getMessage(), settings.get("reason"));
+    }
+
 }

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

@@ -18,14 +18,18 @@
 import com.amazon.opendistroforelasticsearch.security.auditlog.impl.AuditCategory;
 import com.amazon.opendistroforelasticsearch.security.auditlog.impl.AuditMessage;
 import com.amazon.opendistroforelasticsearch.security.auditlog.integration.TestAuditlogImpl;
+import com.amazon.opendistroforelasticsearch.security.dlic.rest.validation.AbstractConfigurationValidator;
 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;
 import com.google.common.collect.ImmutableMap;
 import org.apache.http.Header;
 import org.apache.http.HttpStatus;
 import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.xcontent.XContentType;
 import org.junit.Assert;
 import org.junit.Test;
 
@@ -98,6 +102,15 @@ private void testCrudScenarios(final int expectedStatus, final Header... headers
         assertThat(response.getBody(), response.getStatusCode(), equalTo(expectedStatus));
     }
 
+    private void checkNullElementsInArray(final Header headers) throws Exception{
+
+        String body = FileHelper.loadFile("restapi/nodesdn_null_array_element.json");
+        HttpResponse response = rh.executePutRequest("_opendistro/_security/api/nodesdn/cluster1", body, headers);
+        Settings settings = Settings.builder().loadFromSource(response.getBody(), XContentType.JSON).build();
+        Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode());
+        Assert.assertEquals(AbstractConfigurationValidator.ErrorType.NULL_ARRAY_ELEMENT.getMessage(), settings.get("reason"));
+    }
+
     @Test
     public void testNodesDnApiWithDynamicConfigDisabled() throws Exception {
         setup();
@@ -138,6 +151,12 @@ public void testNodesDnApi() throws Exception {
             testCrudScenarios(HttpStatus.SC_OK, nonAdminCredsHeader);
         }
 
+        {
+            rh.keystore = "restapi/kirk-keystore.jks";
+            rh.sendAdminCertificate = true;
+            checkNullElementsInArray(nonAdminCredsHeader);
+        }
+
         {
             // any creds, admin certificate, disallowed key - FORBIDDEN
             rh.keystore = "restapi/kirk-keystore.jks";
@@ -192,4 +211,5 @@ public void testNodesDnApiAuditComplianceLogging() throws Exception {
 
         assertThat(actualCategoryCounts, equalTo(expectedCategoryCounts));
     }
+
 }

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

@@ -20,12 +20,13 @@
 import com.amazon.opendistroforelasticsearch.security.DefaultObjectMapper;
 import org.apache.http.Header;
 import org.apache.http.HttpStatus;
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.xcontent.XContentType;
 import org.junit.Assert;
 import org.junit.Test;
 
 import com.fasterxml.jackson.databind.JsonNode;
 import com.amazon.opendistroforelasticsearch.security.dlic.rest.validation.AbstractConfigurationValidator;
-import com.amazon.opendistroforelasticsearch.security.dlic.rest.validation.AbstractConfigurationValidator.ErrorType;
 import com.amazon.opendistroforelasticsearch.security.support.SecurityJsonNode;
 import com.amazon.opendistroforelasticsearch.security.test.helper.file.FileHelper;
 import com.amazon.opendistroforelasticsearch.security.test.helper.rest.RestHelper.HttpResponse;
@@ -346,7 +347,7 @@ public void testRolesApi() throws Exception {
         Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode());
         settings = DefaultObjectMapper.readTree(response.getBody());
         Assert.assertEquals(settings.get("status").asText(), "error");
-        Assert.assertEquals(settings.get("reason").asText(), ErrorType.INVALID_CONFIGURATION.getMessage());
+        Assert.assertEquals(settings.get("reason").asText(), AbstractConfigurationValidator.ErrorType.INVALID_CONFIGURATION.getMessage());
 
         // -- PATCH
         // PATCH on non-existing resource
@@ -502,4 +503,47 @@ 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]);
+        Settings settings = Settings.builder().loadFromSource(response.getBody(), XContentType.JSON).build();
+        Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode());
+        Assert.assertEquals(AbstractConfigurationValidator.ErrorType.NULL_ARRAY_ELEMENT.getMessage(), settings.get("reason"));
+
+        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]);
+        settings = Settings.builder().loadFromSource(response.getBody(), XContentType.JSON).build();
+        Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode());
+        Assert.assertEquals(AbstractConfigurationValidator.ErrorType.NULL_ARRAY_ELEMENT.getMessage(), settings.get("reason"));
+
+        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]);
+        settings = Settings.builder().loadFromSource(response.getBody(), XContentType.JSON).build();
+        Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode());
+        Assert.assertEquals(AbstractConfigurationValidator.ErrorType.NULL_ARRAY_ELEMENT.getMessage(), settings.get("reason"));
+
+        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]);
+        settings = Settings.builder().loadFromSource(response.getBody(), XContentType.JSON).build();
+        Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode());
+        Assert.assertEquals(AbstractConfigurationValidator.ErrorType.NULL_ARRAY_ELEMENT.getMessage(), settings.get("reason"));
+
+        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]);
+        settings = Settings.builder().loadFromSource(response.getBody(), XContentType.JSON).build();
+        Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode());
+        Assert.assertEquals(AbstractConfigurationValidator.ErrorType.NULL_ARRAY_ELEMENT.getMessage(), settings.get("reason"));
+
+        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]);
+        settings = Settings.builder().loadFromSource(response.getBody(), XContentType.JSON).build();
+        Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode());
+        Assert.assertEquals(AbstractConfigurationValidator.ErrorType.NULL_ARRAY_ELEMENT.getMessage(), settings.get("reason"));
+    }
+
 }

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

@@ -392,4 +392,32 @@ public void testRolesMappingApiForNonSuperAdmin() throws Exception {
 		Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, 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]);
+		Settings settings = Settings.builder().loadFromSource(response.getBody(), XContentType.JSON).build();
+		Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode());
+		Assert.assertEquals(AbstractConfigurationValidator.ErrorType.NULL_ARRAY_ELEMENT.getMessage(), settings.get("reason"));
+
+		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]);
+		settings = Settings.builder().loadFromSource(response.getBody(), XContentType.JSON).build();
+		Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode());
+		Assert.assertEquals(AbstractConfigurationValidator.ErrorType.NULL_ARRAY_ELEMENT.getMessage(), settings.get("reason"));
+
+		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]);
+		settings = Settings.builder().loadFromSource(response.getBody(), XContentType.JSON).build();
+		Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode());
+		Assert.assertEquals(AbstractConfigurationValidator.ErrorType.NULL_ARRAY_ELEMENT.getMessage(), settings.get("reason"));
+	}
 }

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

@@ -603,4 +603,17 @@ 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]);
+        Settings settings = Settings.builder().loadFromSource(response.getBody(), XContentType.JSON).build();
+        Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode());
+        Assert.assertEquals(AbstractConfigurationValidator.ErrorType.NULL_ARRAY_ELEMENT.getMessage(), settings.get("reason"));
+    }
+
 }

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
+  ]
+}