honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Config file parsing: Improved parsing for edge cases 

Created some tests to improve parsing of files, most importantly fixed an endless loop
in YAML parsing and made the non-yaml parsers a bit more error resistant.

Closes elastic/elasticsearch#40

Original commit: elastic/x-pack-elasticsearch@8873138d0c
This commit is contained in:
Alexander Reelsen 2014-08-26 10:49:48 +02:00
parent 9cd397727f
commit b887fad51a
8 changed files with 160 additions and 68 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
src/main/java/org/elasticsearch/shield/authc/esusers/FileUserPasswdStore.java
View File

@ -100,7 +100,9 @@ public class FileUserPasswdStore extends AbstractComponent implements UserPasswd
lineNr++;
int i = line.indexOf(":");
if (i <= 0 || i == line.length() - 1) {
logger.error("Invalid entry in users file [" + path.toAbsolutePath() + "], line [" + lineNr + "]. Skipping...");
if (logger != null) {
logger.error("Invalid entry in users file [" + path.toAbsolutePath() + "], line [" + lineNr + "]. Skipping...");
}
continue;
}
String username = line.substring(0, i).trim();

22
src/main/java/org/elasticsearch/shield/authc/esusers/FileUserRolesStore.java
View File

@ -93,12 +93,32 @@ public class FileUserRolesStore extends AbstractComponent implements UserRolesSt
lineNr++;
int i = line.indexOf(":");
if (i <= 0 || i == line.length() - 1) {
logger.error("Invalid entry in users file [" + path.toAbsolutePath() + "], line [" + lineNr + "]. Skipping...");
if (logger != null) {
logger.error("Invalid entry in users file [" + path.toAbsolutePath() + "], line [" + lineNr + "]. Skipping...");
}
continue;
}
String username = line.substring(0, i).trim();
if (Strings.isEmpty(username)) {
if (logger != null) {
logger.error("Invalid username entry in users file [" + path.toAbsolutePath() + "], line [" + lineNr + "]. Skipping...");
}
continue;
}
String rolesStr = line.substring(i + 1).trim();
if (Strings.isEmpty(rolesStr)) {
if (logger != null) {
logger.error("Invalid roles entry in users file [" + path.toAbsolutePath() + "], line [" + lineNr + "]. Skipping...");
}
continue;
}
String[] roles = ROLES_DELIM.split(rolesStr);
if (roles.length == 0) {
if (logger != null) {
logger.error("Invalid roles entry in users file [" + path.toAbsolutePath() + "], line [" + lineNr + "]. Skipping...");
}
continue;
}
usersRoles.put(username, roles);
}

6
src/main/java/org/elasticsearch/shield/authz/store/FileRolesStore.java
View File

@ -11,6 +11,8 @@ import org.elasticsearch.common.collect.ImmutableMap;
import org.elasticsearch.common.collect.ImmutableSet;
import org.elasticsearch.common.component.AbstractComponent;
import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.jackson.dataformat.yaml.snakeyaml.error.YAMLException;
import org.elasticsearch.common.jackson.dataformat.yaml.snakeyaml.scanner.ScannerException;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
@ -86,7 +88,7 @@ public class FileRolesStore extends AbstractComponent implements RolesStore {
XContentParser parser = YamlXContent.yamlXContent.createParser(input);
XContentParser.Token token;
String currentFieldName = null;
while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT && token != null) {
if (token == XContentParser.Token.FIELD_NAME) {
currentFieldName = parser.currentName();
} else if (token == XContentParser.Token.START_OBJECT && currentFieldName != null) {
@ -160,7 +162,7 @@ public class FileRolesStore extends AbstractComponent implements RolesStore {
return roles.build();
} catch (IOException ioe) {
} catch (YAMLException|IOException ioe) {
throw new ElasticsearchException("Failed to read roles file [" + path.toAbsolutePath() + "]", ioe);
}
}

5
src/main/java/org/elasticsearch/shield/n2n/IPFilteringN2NAuthenticator.java
View File

@ -11,6 +11,7 @@ import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.component.AbstractComponent;
import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.jackson.dataformat.yaml.snakeyaml.error.YAMLException;
import org.elasticsearch.common.net.InetAddresses;
import org.elasticsearch.common.netty.handler.ipfilter.IpFilterRule;
import org.elasticsearch.common.netty.handler.ipfilter.IpSubnetFilterRule;
@ -103,8 +104,8 @@ public class IPFilteringN2NAuthenticator extends AbstractComponent implements N2
}
}
} catch (IOException ioe) {
throw new ElasticsearchException("Failed to read & parse host access file [" + path.toAbsolutePath() + "]", ioe);
} catch (YAMLException|IOException ioe) {
throw new ElasticsearchParseException("Failed to read & parse host access file [" + path.toAbsolutePath() + "]", ioe);
}
if (rules.size() == 0) {

16
src/test/java/org/elasticsearch/shield/authc/esusers/FileUserPasswdStoreTests.java
View File

@ -10,12 +10,17 @@ import org.elasticsearch.common.settings.ImmutableSettings;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.env.Environment;
import org.elasticsearch.shield.authc.support.Hasher;
import org.elasticsearch.shield.authz.Permission;
import org.elasticsearch.shield.authz.store.FileRolesStore;
import org.elasticsearch.test.ElasticsearchTestCase;
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.watcher.ResourceWatcherService;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
import java.io.BufferedWriter;
import java.io.File;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
@ -31,6 +36,9 @@ import static org.hamcrest.Matchers.*;
*/
public class FileUserPasswdStoreTests extends ElasticsearchTestCase {
@Rule
public TemporaryFolder tempFolder = new TemporaryFolder();
@Test
public void testParseFile() throws Exception {
Path path = Paths.get(getClass().getResource("users").toURI());
@ -99,6 +107,14 @@ public class FileUserPasswdStoreTests extends ElasticsearchTestCase {
threadPool.shutdownNow();
}
}
}
@Test
public void testThatInvalidLineDoesNotResultInLoggerNPE() throws Exception {
File file = tempFolder.newFile();
com.google.common.io.Files.write("NotValidUsername=Password\nuser:pass".getBytes(org.elasticsearch.common.base.Charsets.UTF_8), file);
Map<String, char[]> users = FileUserPasswdStore.parseFile(file.toPath(), null);
assertThat(users, notNullValue());
assertThat(users.keySet(), hasSize(1));
}
}

34
src/test/java/org/elasticsearch/shield/authc/esusers/FileUserRolesStoreTests.java
View File

@ -9,16 +9,22 @@ import com.carrotsearch.ant.tasks.junit4.dependencies.com.google.common.base.Cha
import org.elasticsearch.common.settings.ImmutableSettings;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.env.Environment;
import org.elasticsearch.shield.authz.Permission;
import org.elasticsearch.shield.authz.store.FileRolesStore;
import org.elasticsearch.test.ElasticsearchTestCase;
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.watcher.ResourceWatcherService;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
import java.io.BufferedWriter;
import java.io.File;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.nio.file.StandardOpenOption;
import java.util.Locale;
import java.util.Map;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
@ -30,6 +36,9 @@ import static org.hamcrest.Matchers.*;
*/
public class FileUserRolesStoreTests extends ElasticsearchTestCase {
@Rule
public TemporaryFolder tempFolder = new TemporaryFolder();
@Test
public void testParseFile() throws Exception {
Path path = Paths.get(getClass().getResource("users_roles").toURI());
@ -102,6 +111,31 @@ public class FileUserRolesStoreTests extends ElasticsearchTestCase {
threadPool.shutdownNow();
}
}
}
@Test
public void testThatEmptyFileIsParsed() throws Exception {
assertInvalidInputIsSilentlyIngored("");
assertInvalidInputIsSilentlyIngored("#");
}
@Test
public void testThatEmptyUserNameDoesNotThrowException() throws Exception {
assertInvalidInputIsSilentlyIngored(":role1,role2");
assertInvalidInputIsSilentlyIngored(" :role1,role2");
}
@Test
public void testThatEmptyRoleDoesNotThrowException() throws Exception {
assertInvalidInputIsSilentlyIngored("user:");
assertInvalidInputIsSilentlyIngored("user: ");
assertInvalidInputIsSilentlyIngored("user: , ");
}
private void assertInvalidInputIsSilentlyIngored(String input) throws Exception {
File file = tempFolder.newFile();
com.google.common.io.Files.write(input.getBytes(Charsets.UTF_8), file);
Map<String, String[]> usersRoles = FileUserRolesStore.parseFile(file.toPath(), null);
assertThat(String.format(Locale.ROOT, "Expected userRoles to be empty, but was %s", usersRoles.keySet()), usersRoles.keySet(), hasSize(0));
}
}

16
src/test/java/org/elasticsearch/shield/authz/store/FileRolesStoreTests.java
View File

@ -5,6 +5,7 @@
*/
package org.elasticsearch.shield.authz.store;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.common.base.Charsets;
import org.elasticsearch.common.settings.ImmutableSettings;
import org.elasticsearch.common.settings.Settings;
@ -19,6 +20,7 @@ import org.junit.Test;
import org.junit.rules.TemporaryFolder;
import java.io.BufferedWriter;
import java.io.File;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
@ -147,6 +149,20 @@ public class FileRolesStoreTests extends ElasticsearchTestCase {
threadPool.shutdownNow();
}
}
}
@Test
public void testThatEmptyFileDoesNotResultInLoop() throws Exception {
File file = tempFolder.newFile();
com.google.common.io.Files.write("#".getBytes(Charsets.UTF_8), file);
Map<String, Permission.Global> roles = FileRolesStore.parseFile(file.toPath());
assertThat(roles.keySet(), is(empty()));
}
@Test(expected = ElasticsearchException.class)
public void testThatInvalidYAMLThrowsElasticsearchException() throws Exception {
File file = tempFolder.newFile();
com.google.common.io.Files.write("user: cluster: ALL indices: '.*': ALL".getBytes(Charsets.UTF_8), file);
FileRolesStore.parseFile(file.toPath());
}
}

125
src/test/java/org/elasticsearch/shield/n2n/IPFilteringN2NAuthenticatorTests.java
View File

@ -6,6 +6,7 @@
package org.elasticsearch.shield.n2n;
import com.google.common.io.Files;
import org.elasticsearch.ElasticsearchParseException;
import org.elasticsearch.common.base.Charsets;
import org.elasticsearch.common.net.InetAddresses;
import org.elasticsearch.common.settings.Settings;
@ -14,13 +15,16 @@ import org.elasticsearch.env.Environment;
import org.elasticsearch.test.ElasticsearchTestCase;
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.watcher.ResourceWatcherService;
import org.junit.After;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
import java.io.File;
import java.io.IOException;
import java.security.Principal;
import java.util.Locale;
import static org.elasticsearch.common.settings.ImmutableSettings.settingsBuilder;
import static org.hamcrest.Matchers.is;
@ -42,122 +46,119 @@ public class IPFilteringN2NAuthenticatorTests extends ElasticsearchTestCase {
private ResourceWatcherService resourceWatcherService;
private File configFile;
private Settings settings;
private IPFilteringN2NAuthenticator ipFilteringN2NAuthenticator;
@Before
public void init() throws Exception {
configFile = temporaryFolder.newFile();
Settings settings = settingsBuilder().put("watcher.interval.medium", TimeValue.timeValueMillis(200)).build();
resourceWatcherService = new ResourceWatcherService(settings, new ThreadPool("resourceWatcher")).start();
Settings resourceWatcherServiceSettings = settingsBuilder().put("watcher.interval.medium", TimeValue.timeValueMillis(200)).build();
resourceWatcherService = new ResourceWatcherService(resourceWatcherServiceSettings, new ThreadPool("resourceWatcher")).start();
settings = settingsBuilder().put("shield.n2n.file", configFile.getPath()).build();
}
@After
public void shutdown() {
resourceWatcherService.stop();
}
@Test
public void testThatIpV4AddressesCanBeProcessed() throws Exception {
String testData = "allow: 127.0.0.1\ndeny: 10.0.0.0/8";
Files.write(testData.getBytes(Charsets.UTF_8), configFile);
writeConfigFile("allow: 127.0.0.1\ndeny: 10.0.0.0/8");
Settings settings = settingsBuilder().put("shield.n2n.file", configFile.getPath()).build();
IPFilteringN2NAuthenticator ipFilteringN2NAuthenticator = new IPFilteringN2NAuthenticator(settings, new Environment(), resourceWatcherService);
assertThat(ipFilteringN2NAuthenticator.authenticate(NULL_PRINCIPAL, InetAddresses.forString("127.0.0.1"), 1024), is(true));
assertThat(ipFilteringN2NAuthenticator.authenticate(NULL_PRINCIPAL, InetAddresses.forString("10.2.3.4"), 1024), is(false));
assertAddressIsAllowed(ipFilteringN2NAuthenticator, "127.0.0.1");
assertAddressIsDenied(ipFilteringN2NAuthenticator, "10.2.3.4");
}
@Test
public void testThatIpV6AddressesCanBeProcessed() throws Exception {
String testData = "allow: 2001:0db8:1234::/48\ndeny: 1234:0db8:85a3:0000:0000:8a2e:0370:7334";
Files.write(testData.getBytes(Charsets.UTF_8), configFile);
writeConfigFile("allow: 2001:0db8:1234::/48\ndeny: 1234:0db8:85a3:0000:0000:8a2e:0370:7334");
Settings settings = settingsBuilder().put("shield.n2n.file", configFile.getPath()).build();
IPFilteringN2NAuthenticator ipFilteringN2NAuthenticator = new IPFilteringN2NAuthenticator(settings, new Environment(), resourceWatcherService);
assertThat(ipFilteringN2NAuthenticator.authenticate(NULL_PRINCIPAL, InetAddresses.forString("2001:0db8:1234:0000:0000:8a2e:0370:7334"), 1024), is(true));
assertThat(ipFilteringN2NAuthenticator.authenticate(NULL_PRINCIPAL, InetAddresses.forString("1234:0db8:85a3:0000:0000:8a2e:0370:7334"), 1024), is(false));
assertAddressIsAllowed(ipFilteringN2NAuthenticator, "2001:0db8:1234:0000:0000:8a2e:0370:7334");
assertAddressIsDenied(ipFilteringN2NAuthenticator, "1234:0db8:85a3:0000:0000:8a2e:0370:7334");
}
@Test
public void testThatHostnamesCanBeProcessed() throws Exception {
String testData = "allow: localhost\ndeny: '*.google.com'";
Files.write(testData.getBytes(Charsets.UTF_8), configFile);
writeConfigFile("allow: localhost\ndeny: '*.google.com'");
Settings settings = settingsBuilder().put("shield.n2n.file", configFile.getPath()).build();
IPFilteringN2NAuthenticator ipFilteringN2NAuthenticator = new IPFilteringN2NAuthenticator(settings, new Environment(), resourceWatcherService);
assertThat(ipFilteringN2NAuthenticator.authenticate(NULL_PRINCIPAL, InetAddresses.forString("127.0.0.1"), 1024), is(true));
assertThat(ipFilteringN2NAuthenticator.authenticate(NULL_PRINCIPAL, InetAddresses.forString("173.194.70.100"), 1024), is(false));
assertAddressIsAllowed(ipFilteringN2NAuthenticator, "127.0.0.1");
assertAddressIsDenied(ipFilteringN2NAuthenticator, "173.194.70.100");
}
@Test
public void testThatFileDeletionResultsInAllowingAll() throws Exception {
String testData = "allow: 127.0.0.1";
Files.write(testData.getBytes(Charsets.UTF_8), configFile);
writeConfigFile("allow: 127.0.0.1");
Settings settings = settingsBuilder().put("shield.n2n.file", configFile.getPath()).build();
IPFilteringN2NAuthenticator ipFilteringN2NAuthenticator = new IPFilteringN2NAuthenticator(settings, new Environment(), resourceWatcherService);
assertThat(ipFilteringN2NAuthenticator.authenticate(NULL_PRINCIPAL, InetAddresses.forString("127.0.0.1"), 1024), is(true));
assertAddressIsAllowed(ipFilteringN2NAuthenticator, "127.0.0.1");
configFile.delete();
assertThat(configFile.exists(), is(false));
sleep(250);
assertThat(ipFilteringN2NAuthenticator.authenticate(NULL_PRINCIPAL, InetAddresses.forString("127.0.0.1"), 1024), is(false));
assertAddressIsDenied(ipFilteringN2NAuthenticator, "127.0.0.1");
}
@Test
public void testThatAnAllowAllAuthenticatorWorks() throws Exception {
String testData = "allow: all";
Files.write(testData.getBytes(Charsets.UTF_8), configFile);
writeConfigFile("allow: all");
Settings settings = settingsBuilder().put("shield.n2n.file", configFile.getPath()).build();
IPFilteringN2NAuthenticator ipFilteringN2NAuthenticator = new IPFilteringN2NAuthenticator(settings, new Environment(), resourceWatcherService);
assertThat(ipFilteringN2NAuthenticator.authenticate(NULL_PRINCIPAL, InetAddresses.forString("127.0.0.1"), 1024), is(true));
assertThat(ipFilteringN2NAuthenticator.authenticate(NULL_PRINCIPAL, InetAddresses.forString("173.194.70.100"), 1024), is(true));
assertAddressIsAllowed(ipFilteringN2NAuthenticator, "127.0.0.1");
assertAddressIsAllowed(ipFilteringN2NAuthenticator, "173.194.70.100");
}
@Test
public void testThatCommaSeparatedValuesWork() throws Exception {
String testData = "allow: 192.168.23.0/24, localhost\ndeny: all";
Files.write(testData.getBytes(Charsets.UTF_8), configFile);
writeConfigFile("allow: 192.168.23.0/24, localhost\ndeny: all");
Settings settings = settingsBuilder().put("shield.n2n.file", configFile.getPath()).build();
IPFilteringN2NAuthenticator ipFilteringN2NAuthenticator = new IPFilteringN2NAuthenticator(settings, new Environment(), resourceWatcherService);
assertThat(ipFilteringN2NAuthenticator.authenticate(NULL_PRINCIPAL, InetAddresses.forString("192.168.23.1"), 1024), is(true));
assertThat(ipFilteringN2NAuthenticator.authenticate(NULL_PRINCIPAL, InetAddresses.forString("127.0.0.1"), 1024), is(true));
assertThat(ipFilteringN2NAuthenticator.authenticate(NULL_PRINCIPAL, InetAddresses.forString("10.1.2.3"), 1024), is(false));
assertAddressIsAllowed(ipFilteringN2NAuthenticator, "192.168.23.1");
assertAddressIsAllowed(ipFilteringN2NAuthenticator, "127.0.0.1");
assertAddressIsDenied(ipFilteringN2NAuthenticator, "10.1.2.3");
}
@Test
public void testThatOrderIsImportant() throws Exception {
String testData = "deny: localhost\nallow: localhost";
Files.write(testData.getBytes(Charsets.UTF_8), configFile);
writeConfigFile("deny: localhost\nallow: localhost");
Settings settings = settingsBuilder().put("shield.n2n.file", configFile.getPath()).build();
IPFilteringN2NAuthenticator ipFilteringN2NAuthenticator = new IPFilteringN2NAuthenticator(settings, new Environment(), resourceWatcherService);
assertThat(ipFilteringN2NAuthenticator.authenticate(NULL_PRINCIPAL, InetAddresses.forString("127.0.0.1"), 1024), is(false));
assertAddressIsDenied(ipFilteringN2NAuthenticator, "127.0.0.1");
}
@Test
public void testThatOrderIsImportantViceVersa() throws Exception {
String testData = "allow: localhost\ndeny: localhost";
Files.write(testData.getBytes(Charsets.UTF_8), configFile);
writeConfigFile("allow: localhost\ndeny: localhost");
Settings settings = settingsBuilder().put("shield.n2n.file", configFile.getPath()).build();
IPFilteringN2NAuthenticator ipFilteringN2NAuthenticator = new IPFilteringN2NAuthenticator(settings, new Environment(), resourceWatcherService);
assertThat(ipFilteringN2NAuthenticator.authenticate(NULL_PRINCIPAL, InetAddresses.forString("127.0.0.1"), 1024), is(true));
assertAddressIsAllowed(ipFilteringN2NAuthenticator, "127.0.0.1");
}
@Test
public void testThatEmptyFileDoesNotLeadIntoLoop() throws Exception {
String testData = "# \n\n";
Files.write(testData.getBytes(Charsets.UTF_8), configFile);
writeConfigFile("# \n\n");
Settings settings = settingsBuilder().put("shield.n2n.file", configFile.getPath()).build();
IPFilteringN2NAuthenticator ipFilteringN2NAuthenticator = new IPFilteringN2NAuthenticator(settings, new Environment(), resourceWatcherService);
assertAddressIsDenied(ipFilteringN2NAuthenticator, "127.0.0.1");
}
assertThat(ipFilteringN2NAuthenticator.authenticate(NULL_PRINCIPAL, InetAddresses.forString("127.0.0.1"), 1024), is(false));
@Test(expected = ElasticsearchParseException.class)
public void testThatInvalidFileThrowsCorrectException() throws Exception {
writeConfigFile("deny: all allow: all \n\n");
IPFilteringN2NAuthenticator.parseFile(configFile.toPath());
}
private void writeConfigFile(String data) throws IOException {
Files.write(data.getBytes(Charsets.UTF_8), configFile);
ipFilteringN2NAuthenticator = new IPFilteringN2NAuthenticator(settings, new Environment(), resourceWatcherService);
}
private void assertAddressIsAllowed(IPFilteringN2NAuthenticator ipFilteringN2NAuthenticator, String ... inetAddresses) {
for (String inetAddress : inetAddresses) {
String message = String.format(Locale.ROOT, "Expected address %s to be allowed", inetAddress);
assertThat(message, ipFilteringN2NAuthenticator.authenticate(NULL_PRINCIPAL, InetAddresses.forString(inetAddress), 1024), is(true));
}
}
private void assertAddressIsDenied(IPFilteringN2NAuthenticator ipFilteringN2NAuthenticator, String ... inetAddresses) {
for (String inetAddress : inetAddresses) {
String message = String.format(Locale.ROOT, "Expected address %s to be denied", inetAddress);
assertThat(message, ipFilteringN2NAuthenticator.authenticate(NULL_PRINCIPAL, InetAddresses.forString(inetAddress), 1024), is(false));
}
}
}