New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make use of Lookup Tables. #48

Merged
merged 38 commits into from Sep 21, 2017

Conversation

Projects
None yet
2 participants
@dennisoelkers
Member

dennisoelkers commented Aug 17, 2017

This change refactors the existing code base to make use of lookup tables instead of implementing all lookup functionality on its own.

Therefore it creates new data adapters for these lookups:

  • Spamhaus (E)DROP
  • Tor Exit Node
  • Whois

Abuse.ch ransomware tracker lookups are handled using the new DSV over HTTP adapter and AlienVault OTX lookups are handled using the JSON over HTTP adapter which is enhanced by adding custom HTTP headers (for the AlienVault API token).

A migration exists, which checks if an existing legacy configuration exists and adds an existing OTX API token to the new OTX data adapter.

Additionally, a Content Pack is created and automatically imported and applied using a migration, adding initial entities used by the provided pipeline lookup functions.

Requires Graylog2/graylog2-server#4094, Graylog2/graylog2-server#4095 & Graylog2/graylog2-server#4096 to be merged.

Fixes #47, #13, #45

dennisoelkers added some commits Aug 14, 2017

@dennisoelkers dennisoelkers added this to the 2.4.0 milestone Aug 17, 2017

dennisoelkers added some commits Aug 18, 2017

Rework threat intel plugin configuration mechanics.
Instead of individually enabling pipeline functions, configuration now
determines which individual modules are used for the global functions.

@bernd bernd self-assigned this Sep 19, 2017

pom.xml Outdated
@@ -41,6 +41,7 @@
<maven.deploy.skip>true</maven.deploy.skip>
<maven.site.skip>true</maven.site.skip>
<graylog.version>2.4.0-SNAPSHOT</graylog.version>
<graylog-pipelines.version>2.4.0-SNAPSHOT</graylog-pipelines.version>

This comment has been minimized.

@bernd

bernd Sep 19, 2017

Member

This doesn't seem to be used.

This comment has been minimized.

@dennisoelkers
final String domain = valueParam.required(args, context);
if (domain == null) {
LOG.error("NULL parameter passed to OTX threat intel lookup.");
return null;

This comment has been minimized.

@bernd

bernd Sep 19, 2017

Member

Would it make sense to return OTXLookupResult.EMPTY here instead of null?

This comment has been minimized.

@dennisoelkers
}
@Override
public OTXLookupResult evaluate(FunctionArgs args, EvaluationContext context) {
String ip = valueParam.required(args, context);
final String ip = valueParam.required(args, context);
if (ip == null) {
LOG.error("NULL parameter passed to OTX threat intel lookup.");
return null;

This comment has been minimized.

@bernd

bernd Sep 19, 2017

Member

Same here, maybe OTXLookupResult.EMPTY would be better than null?

This comment has been minimized.

@dennisoelkers
}
@Override
public void upgrade() {

This comment has been minimized.

@bernd

bernd Sep 19, 2017

Member

As far as I can see, this creates new database objects on every server start? There is no condition to check if this has been applied already.

This comment has been minimized.

@dennisoelkers
.headers(newHeaders);
config.multiValueJSONPath().ifPresent(updatedConfigBuilder::multiValueJSONPath);
final DataAdapterDto saved = dbDataAdapterService.save(DataAdapterDto.builder()

This comment has been minimized.

@bernd

bernd Sep 19, 2017

Member

This seems to run on every server startup as well. Please add a check so the migration only runs once.

This comment has been minimized.

@dennisoelkers

dennisoelkers added some commits Sep 19, 2017

@JsonIgnore
public boolean isOtxComplete() {
return otxApiKey() != null && !otxApiKey().isEmpty();
public static ThreatIntelPluginConfiguration defaults() {

This comment has been minimized.

@bernd

bernd Sep 20, 2017

Member

Server startup fails for me with the following stacktrace. (guice error) I don't have a ThreadIntelPluginConfiguration object in my cluster config.
I guess the otxApiKey property in ThreadIntelPluginConfiguration needs to be @Nullable.

Caused by: java.lang.IllegalStateException: Missing required properties: otxApiKey
	at org.graylog.plugins.threatintel.AutoValue_ThreatIntelPluginConfiguration$Builder.build(AutoValue_ThreatIntelPluginConfiguration.java:157)
	at org.graylog.plugins.threatintel.ThreatIntelPluginConfiguration.defaults(ThreatIntelPluginConfiguration.java:55)
	at org.graylog.plugins.threatintel.functions.global.AbstractGlobalLookupFunction.<init>(AbstractGlobalLookupFunction.java:28)
	at org.graylog.plugins.threatintel.functions.global.GlobalIpLookupFunction.<init>(GlobalIpLookupFunction.java:42)
	at org.graylog.plugins.threatintel.functions.global.GlobalIpLookupFunction$$FastClassByGuice$$cac62d3d.newInstance(<generated>)
	at com.google.inject.internal.DefaultConstructionProxyFactory$FastClassProxy.newInstance(DefaultConstructionProxyFactory.java:89)
	at com.google.inject.internal.ConstructorInjector.provision(ConstructorInjector.java:111)
	at com.google.inject.internal.ConstructorInjector.construct(ConstructorInjector.java:90)

This comment has been minimized.

@bernd

bernd Sep 20, 2017

Member

In addition, I think it would be good if we don't do a cluster config lookup (and thus MongoDB request) in the constructor of the AbstractGlobalLookupFunction class. (and others, if we do it somewhere else)

This comment has been minimized.

@dennisoelkers

dennisoelkers Sep 21, 2017

Member

Good catches, thanks!

dennisoelkers added some commits Sep 21, 2017

@dennisoelkers

This comment has been minimized.

Member

dennisoelkers commented Sep 21, 2017

Due to a bug in the DSVHTTPDataAdapter in core, this change also required Graylog2/graylog2-server#4169 to be merged before.

@bernd

bernd approved these changes Sep 21, 2017

LGTM 👍

@bernd bernd merged commit ae8f961 into master Sep 21, 2017

2 checks passed

graylog-project/pr Jenkins build graylog-project-pr-snapshot 487 has succeeded
Details
license/cla Contributor License Agreement is signed.
Details

@bernd bernd deleted the migrate-to-lookup-tables branch Sep 21, 2017

This was referenced Oct 2, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment