Skip to content
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

S3 Source Connector does not enforce SSL/TLS for non-S3 endpoints #17481

Closed
brianjlai opened this issue Sep 30, 2022 · 5 comments · Fixed by #17800
Closed

S3 Source Connector does not enforce SSL/TLS for non-S3 endpoints #17481

brianjlai opened this issue Sep 30, 2022 · 5 comments · Fixed by #17800

Comments

@brianjlai
Copy link
Contributor

Tell us about the problem you're trying to solve

For the Amazon S3 source, the TLS option is not on by default and users can leave it off.

S3 only allows encrypted connections so this does only apply to non-S3 endpoints.

Describe the solution you’d like

We should always assume this value is true in the code. And to reduce confusion, this configuration option should also be removed when setting up a connector.

Describe the alternative you’ve considered or used

A clear and concise description of any alternative solutions or features you've considered or are using today.

Additional context

Add any other context or screenshots about the feature request here.

Are you willing to submit a PR?

@YowanR
Copy link
Contributor

YowanR commented Oct 5, 2022

@lazebnyi This is the S3 issue that needs to be tackled asap. Thanks!
cc @YuliiaNahoha

@edgao
Copy link
Contributor

edgao commented Oct 12, 2022

@brianjlai I think we should only enforce this in cloud? Currently this means that if I have OSS and minio running side-by-side on my laptop, I would need to set up HTTPS also. On the DBs side we allow insecure connections in OSS, under the assumption that users are more likely to e.g. put everything inside their firewall or whatever

@brianjlai
Copy link
Contributor Author

It does feel like something of a risk to assume that all OSS users will adhere to this and we're leaving the door open to a security loophole connection if misconfigured. But if that's the paradigm we're subscribing to for DBs for convenience of OSS that is fine too. But for DBs, how does the connector know to allow insecure connections? Is the connector aware of whether its running on cloud vs. OSS via some sort of input or is the config validated upstream?

@edgao
Copy link
Contributor

edgao commented Oct 13, 2022

yeah, there's definitely some interesting discussion about whether this paradigm is correct. There's always going to be some need for users who don't want to deal with these restrictions - but maybe this should be opt-in for OSS users who know they have a secured environment and can safely ignore them.

there's an env var for whether the container is running as OSS or cloud, but it would need to be ported to Python (cc @tuliren) - https://github.com/airbytehq/airbyte/blob/master/airbyte-integrations/bases/base-java/src/main/java/io/airbyte/integrations/base/adaptive/AdaptiveSourceRunner.java

example usage in source-postgres: https://github.com/airbytehq/airbyte/blob/master/airbyte-integrations/connectors/source-postgres/src/main/java/io/airbyte/integrations/source/postgres/PostgresSourceRunner.java#L12-L15

Essentially:

  • In OSS -> run using the normal spec
  • in cloud -> run using the "strict encrypt" version, which is mostly identical, except that it removes the insecure connection options from the spec

@YowanR
Copy link
Contributor

YowanR commented Oct 13, 2022

This is a really interesting discussion! Should we consider moving this to Slack so more folks can chime in?

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

Successfully merging a pull request may close this issue.

5 participants