Added S3 third party support

[balpert89]

This PR introduces the ability to support a third party (a.k.a. non-AWS) S3-compatible endpoint. It is analog to the gcshandler as it sets the config from URL, utilizing a new check for subdomain and program flow based on this check.

It introduces a check for sub domain ("com"/"org"/"io" etc.) and a parse func to determine bucket name and path portion
(s3.example.com/bucket-name/path/to/dir -> bucketName: "bucket-name", path: "/path/to/dir") which is then passed to the s3 session object.

An example for the relevant parts can be found here: https://play.golang.org/p/jfUmxpBp9Vl

[gravitational-jenkins]

Can one of the admins verify this patch?

[balpert89]

I don't know if that counts, but I build the binaries from scratch to verify the functionality :-)

The screenshot is from a S3 compatible storage backend:

So I am able to record sessions and replay them.

@klizhentas would be great if you could take a look here.

[webvictim]

ok to test

[balpert89]

@webvictim where do I get the read access for that jenkins job? Accessing results in a permission denied.

[webvictim]

@balpert89 Access to Jenkins is only for Gravitational staff I'm afraid. The failure was nothing to do with your code, however.

[balpert89]

@webvictim can you hold this back again, I have made some further tests when the endpoint is only an ip address (such as localhost), the patch does not work. I would like to extend it with that.

Use case: we are further ecrypting the recorded sessions because the .tar is only a plain text. Depending on the logs there could be critical information found inside it. Has Teleport this functionality out of the box, encrypting the recorded sessions?

[webvictim]

I've changed the title of the PR to indicate that it's a work in progress to avoid merging for now.

By default, Teleport will enable encryption for any S3 buckets that it creates on AWS. It doesn't include any encryption of its own - it relies on encryption at the storage level. If your third party S3-compatible provider also provides encryption at rest, it should be fairly easy to enable that too.

[balpert89]

New iteration, I have used the same approach as the "region" param, as detecting if this is a hostname/ip with or without port is not trivial.

Updated playground: https://play.golang.org/p/U-3e9Pr71Ah
Usage: s3://bucketname/path/to/dir?endpoint=s3.example.com

[balpert89]

@webvictim @klizhentas checks are ok, is there anything I have to do? :)

[webvictim]

I'll leave this for @klizhentas to review and merge when ready.

[klizhentas]

would it be possible to omit this flag?

[balpert89]

I will describe the use case for that.

In an above comment (#3057 (comment)) I've asked if Teleport is capable in encrypting the recorded session, however it relies on the s3 buckets functionality together with KMS to achieve that. Some cloud providers don't have that (MS Azure for example has another technology, Key Vault). Therefore we have developed our own encryption proxy that is deployed in the same pod as the auth server in a side container, it is targetted only over localhost: s3://bucket-name/path/to/dir?endpoint=127.0.0.1:8080. As it is only using localhost communication cannot be sniffed.

The flag also makes testing that much easier, its the same functionality you provide wit the etcd related insecure flag (https://gravitational.com/teleport/docs/admin-guide/#using-etcd) or the overall --insecure and --insecure-no-tls flags. Maybe renaming that parameter to insecurehttp or something like that would indicate to not use it in production environments?

[balpert89]

@klizhentas is this use case description sufficient for you? Can we proceed? :)

[klizhentas]

There are a couple of other things we'd have to do before we proceed:

• DisableSSL -> InsecureDisableTLS SSL is a protocol that is no longer used, so this term is inaccurate.
• Tests. This pull request should add appropriate tests
• Scheme. Adding query parameters to s3:// scheme seems like a viable solution to me, however I wonder if we can come up with something more elegant here. Still scratching my head about this one though.

Thanks for your patience.

[balpert89]

Then I would opt for --insecure-no-tls to be compliant with the other parameter flag. I will also add appropriate tests to the PR.

[balpert89]

The test suite also assumes a) running solely on AWS and b) there is only a TLS connection. Not sure how this test suite is set up, does it create an AWS bucket in the background? Seems like it.

I would suggest using https://github.com/johannesboyne/gofakes3 for a testing API but I would like to check with @klizhentas first :-).

[balpert89]

@klizhentas I have added the TestSuite for third party s3, please review it.

I've changed the title of the PR to indicate that it's a work in progress to avoid merging for now.

By default, Teleport will enable encryption for any S3 buckets that it creates on AWS. It doesn't include any encryption of its own - it relies on encryption at the storage level. If your third party S3-compatible provider also provides encryption at rest, it should be fairly easy to enable that too.

The commit also has a new switch to disable ServerSideEncryption, as there are some providers that do not support that. Also, I did not manage to have the tests for the 3rd party usage in the same s3handler_test.go file, so I opted to use a separate one.

Test Output:

=== RUN   TestS3ThirdParty
2019/10/15 13:00:16 INFO HEAD BUCKET teleport-test-5358bf35-5497-4425-89ff-c69530e636c3
2019/10/15 13:00:16 INFO bucketname: teleport-test-5358bf35-5497-4425-89ff-c69530e636c3
2019/10/15 13:00:16 INFO CREATE BUCKET: teleport-test-5358bf35-5497-4425-89ff-c69530e636c3
2019/10/15 13:00:16 INFO PUT VERSIONING: Enabled
2019/10/15 13:00:16 INFO CREATE OBJECT: teleport-test-5358bf35-5497-4425-89ff-c69530e636c3 test/350e8284-161b-4fe4-82fd-0bb1d4be0a7a.tar
2019/10/15 13:00:16 INFO CREATED VERSION: teleport-test-5358bf35-5497-4425-89ff-c69530e636c3 test/350e8284-161b-4fe4-82fd-0bb1d4be0a7a.tar 3/60O30C1G60O30C1G60O30C1G60O30C1G60O30C1G60O30C1H00NOD62MJ6G7VI0=
2019/10/15 13:00:16 INFO GET OBJECT
2019/10/15 13:00:16 INFO Bucket: teleport-test-5358bf35-5497-4425-89ff-c69530e636c3
2019/10/15 13:00:16 INFO └── Object: test/350e8284-161b-4fe4-82fd-0bb1d4be0a7a.tar
OK: 2 passed
--- PASS: TestS3ThirdParty (0.01s)
PASS

Process finished with exit code 0

[klizhentas]

@kontsevoy @benarent this is a valid use case, however this implementation uses syntax should be improved:

The problem with s3 scheme that it is impossible to differentiate the endpoint from bucket name and path:

s3://example.com/path/to/dir
# example.com can be bucket name or endpoint

The current solution proposes URL query parameters that is quite error prone:

s3://bucket-name/path/to/dir?endpoint=127.0.0.1:8080&insecure=true

Instead, I think we should use schemes:

That denotes the S3 compatible storage

# HTTPS enabled S3 compatible,
# first host is host, example.com is a bucket name
# and the rest is a path
s3compat://127.0.0.1:8080/example.com/path/to/dir

Insecure version for cases with custom proxies that turns off security

# HTTP enabled S3 compatible,
# first host is host, example.com is a bucket name
# and the rest is a path
s3insecure://127.0.0.1:8080/example.com/path/to/dir

Let me know what you think so we can update the spec and ask for changes.

[klizhentas]

@balpert89 per our design process, all changes affecting configuration should go through product team review. I have asked @benarent to get the opinion on the proposed design.

[benarent]

I did a dive into this yesterday, and these are some of my findings.

Since S3 can have a range of endpoints depending on your setup. I would suggest we provide this as an option to be provide a great range of AWS accounts setup, and provides a disableSSL/TLS

e.g
s3_region
s3_govcloud
s3_china *China follows the same pattern, but it's url ends with .cn.

The one issue with the below proposal while some services provide offical docs with s3 refernces, their object store is called something else. I did find one other product using s3compat and I do like this as it makes it clear that the s3 compatible and not all of the s3 features.

The second issue with below, is we'll need to be explicit for DynamoDB region & permissions.

Proposal
For a simple S3 Setup

teleport:
  storage:
      # The region setting sets the default AWS region for all AWS services 
      # Teleport may consume (DynamoDB, S3)

      s3_options:
               region: us-west-1

      audit_sessions_uri: s3://example.com/path/to/bucket

For a S3 Setup in GovCloud

teleport:
  storage:
      # The region setting sets the default AWS region for all AWS services 
      # Teleport may consume (DynamoDB, S3)

      s3_options:
               endpoint: s3-website-us-gov-west-1.amazonaws.com

      audit_sessions_uri: s3://example.com/path/to/bucket

For Open Telekon Cloud

teleport:
  storage:
      # The region setting sets the default AWS region for all AWS services 
      # Teleport may consume (DynamoDB, S3)

      s3_options:
               region: eu-de
               endpoint: obs.eu-de.otc.t-systems.com

      audit_sessions_uri: s3://example.com/path/to/bucket

For a IBM Cloud

teleport:
  storage:
      # The region setting sets the default AWS region for all AWS services 
      # Teleport may consume (DynamoDB, S3)

      s3_options:
               region: us-standard
               endpoint: s3-api.us-geo.objectstorage.softlayer.net

      audit_sessions_uri: s3://example.com/path/to/bucket

For a IBM Cloud Insecure

teleport:
  storage:
      # The region setting sets the default AWS region for all AWS services 
      # Teleport may consume (DynamoDB, S3)

      s3_options:
               region: us-standard
               endpoint: s3-api.us-geo.objectstorage.softlayer.net
               insecure-no-tls : true

      audit_sessions_uri: s3://example.com/path/to/bucket

InsecureDisableTLS

For IBM Cloud, this is how the recommend setting

[klizhentas]

This won't allow scenarios of supporting 2 different URIs with different options applied to them, not sure if it's such a bad thing. Also our URI is a list, perhaps we can do something like this:

audit_sessions_uri: ['stdout:///', {type: s3, bucket: , region:, all other optoins}, ]

or less alien looking without yaml flow:

audit_sessions_uri:
   - 'stdout:///'
   - type: s3
     bucket: 
     region:
     whatever:

this will finally solve this attempts to put all this stuff in the string, and all prior cases will be supported.

[benarent]

Yeah, that looks much better and more concise.

The one consideration is how we treat 'endpoints', I would assume if not set it'll default to the AWS endpoint convention of s3.[region].amazonaws.com and if present, it'll be replaced the full URL for that endpoint.

[balpert89]

@klizhentas will you pick this up or what exactly is the approach here? Should I introduce those changes to configuration? Not sure :)

[klizhentas]

given the scope of changes, we will take your PR and introduce the changes on top of it. No actions is necessary from your side. Thanks!

[klizhentas]

can you please squash your changes in one commit, so we can preserve the original attribution when building on top of your pull request. Thanks

[benarent]

@klizhentas Should we add this into 4.2 or 4.3?

[klizhentas]

@benarent I'd have to merge this one in 4.2 per our agreement with customer, forgot about this, sorry

[klizhentas]

@balpert89 thanks for all the work you have done on this. Ultimately I ended up using your suggested flow.

I have changed your commit a bit, but it's all largely unmodified and with preserved author name here:

#3234

Thanks again, closing this PR in favor of the one above.