Refactors Drivers for OutputCredentialsProvider

[Ryxias]

to: @ryandeivert
cc: @airbnb/streamalert-maintainers
related to:
resolves:

Background

This PR sits on top of #875, in response to this comment. I split this into a separate PR to make it easier to understand.

Changes

I split out the new OutputCredentialsProvider guts into this new concept—CredentialsProvidingDriver. The idea is these drivers each are strategies for fetching Credentials from somewhere.

CredentialsProvidingDrivers

I implemented 4 drivers: S3, local files, spooled file, and ephemeral. I tried to design the interface to be plug-and-play-ready for a SSM migration in the future (#240).

New Way

The new algorithm can be found in setup_drivers(). The pseudocode works like this:

• Ask Ephemeral Driver if credentials are already cached in memory. If so, load and return them.
• Ask S3 Driver to load credentials from AWS S3
• S3 credentials are downloaded, encrypted, and saved into a spool using the Spooled Driver
• Credentials are read from the spool, decrypted, and cached into Ephemeral Driver
• Credentials are returned

Other stuff

I removed a lot of bloat from OutputDispatcher. These old methods were S3-specific when the OutputDispatcher assumed all of its credentials came from S3. Now that the OutputDispatchers are de-coupled from S3, these methods are no longer relevant.

Testing

unit_test

--------------------------------------------------------------------------------------------
TOTAL                                                           4570    163    96%
[success] 6.55% tests.unit.streamalert.classifier.clients.test_firehose.TestFirehoseClient.test_send_batch: 0.8364s
[success] 2.40% tests.unit.stream_alert_alert_merger.test_main.TestAlertMerger.test_dispatch: 0.3069s
[success] 2.03% tests.unit.stream_alert_shared.test_config.TestConfigLoading.test_load_all: 0.2598s
[success] 1.57% tests.unit.stream_alert_alert_processor.test_outputs.test_output_base.TestOutputDispatcher.test_load_creds: 0.2000s
[success] 1.54% tests.unit.stream_alert_shared.test_rule_table.TestRuleTable.test_toggle_staged_state_false: 0.1970s
[success] 1.12% tests.unit.stream_alert_cli.test_cli_config.TestCLIConfig.test_aggregate_alarm_creation: 0.1433s
[success] 1.06% tests.unit.stream_alert_cli.test_cli_config.TestCLIConfig.test_add_threat_intel_downloader: 0.1360s
[success] 1.06% tests.unit.stream_alert_cli.test_cli_config.TestCLIConfig.test_toggle_metric: 0.1351s
[success] 1.05% tests.unit.stream_alert_cli.test_cli_config.TestCLIConfig.test_add_threat_intel_without_table_name: 0.1339s
[success] 1.04% tests.unit.stream_alert_cli.test_cli_config.TestCLIConfig.test_cluster_alarm_creation: 0.1332s
----------------------------------------------------------------------
Ran 874 tests in 12.904s

OK

rule_test

Summary:

Total Tests: 77
Pass: 77
Fail: 0

[ryandeivert]

I don't think region will ever be used for the 'local' one right?

[Ryxias]

Unfortunately I need it to pass into the Credentials constructor, because that class requires the region to use KMS to decrypt the credentials. I honestly could've passed in a CredentialsFactory kind of thing that would avoid having to hot-potato around all of these region, service_name and so on things.

[ryandeivert]

but only the S3 driver would need to do the decryption right? or am I misunderstanding this?

if that's the case - the S3 driver constructor can take different params than the base (super) class and just call the super's init with the right params also. I could be misunderstanding the inheritance model, though

[Ryxias]

Not exactly; the S3 driver (none of the "drivers") actually does not do any decryption on their own. I delegated decryption to this Credentials class.

I didn't actually use any traditional inheritance. The three "base" classes are actually supposed to behave like interfaces:

• FileDescriptorProvider
• CredentialsCachingDriver
• CredentialsProvidingDriver (The important one)

The implementations of CredentialsProvidingDriver are supposed to return Credentials, which can be is_encrypted() or not. The idea is (was?) to allow credentials to be passed between drivers while still encrypted. The only thing the driver needs to know is whether or not Credentials are encrypted "at rest" or not.

This is why all of the Drivers need a region; because they need to pass it to the Credentials class to be able to decrypt the ciphertext after being read from disk/memory/whatever. The exception is the EphemeralDriver, which is the only unencrypted-at-rest Driver.

[coveralls]

Coverage increased (+0.2%) to 97.267% when pulling 419010d on dw--refactor-drivers into 5ffadd5 on dw--output-credentials-provider.

[ryandeivert]

subclasses of OutputCredentialsProvider that use 'boto3' clients and need a region could use the default_config function to return a botocore config (see: https://github.com/airbnb/streamalert/blob/master/stream_alert/shared/helpers/boto.py#L28)

TBH, I'd advocate for using the default_config method anywhere we are creating boto3 clients, since it enforces a shorter timeout (which is better for efficiency/cost in lambda).

[Ryxias]

👍
So we would do something like

client = boto3.client('zzz', config=default_config(region=self._region))

Instead of

client = boto3.client('zzz', region_name=self._region)

?

[ryandeivert]

no you could omit region entirely from the call to default_config (unless it could/would ever be something other than the current region.. which I don't think it would be in this case)

[ryandeivert]

client = boto3.client('zzz', config=default_config())

[Ryxias]

I remembered something when I was confirming whether or not region can be globally configured; what about on the CLI handler? It looks like the region (among other configurations) is passed in from ... somewhere. How does the CLI handler know which REGION the code runs in? Is it inferred from the aws2fa.sh session?