Skip to content

Commit

Permalink
Merge pull request #134 from kbase/s3-disable-ssl-verify
Browse files Browse the repository at this point in the history
option to disable SSL cert verification
  • Loading branch information
kkellerlbl authored Dec 22, 2020
2 parents 5db1e52 + 255201d commit 848cd2e
Show file tree
Hide file tree
Showing 10 changed files with 147 additions and 46 deletions.
6 changes: 5 additions & 1 deletion RELEASE_NOTES.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
# 0.1.2

- Support for disabling SSL verification of remote S3 certificates (default false) with the s3-disable-ssl-verify option in the configuration file.

# 0.1.1

- Added seek & length parameters to file download requests

# 0.1.0

- Initial release
- Initial release
8 changes: 5 additions & 3 deletions app/blobstore.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,11 @@ import (

const (
name = "blobstore"
version = "0.1.1"
version = "0.1.2"
shockname = "Shock"
shockver = "0.9.6" // do not increment
deprecation = "The id and version fields are deprecated."
httpTimeout = 24 * time.Hour
)

// expect initialization via go build -ldflags "-X main.gitCommit=$GIT_COMMIT"
Expand Down Expand Up @@ -53,6 +54,7 @@ func main() {
ServerVersionCompat: shockver,
DeprecationWarning: deprecation,
GitCommit: gitCommit,
HTTPTimeout: httpTimeout,
},
)
if err != nil {
Expand All @@ -62,8 +64,8 @@ func main() {
server := &http.Server{
Addr: cfg.Host,
Handler: serv,
ReadTimeout: 24 * time.Hour,
WriteTimeout: 24 * time.Hour,
ReadTimeout: httpTimeout,
WriteTimeout: httpTimeout,
}

// TODO BUGNASTY figure out how to abort when no more data is being sent https://groups.google.com/forum/#!topic/golang-nuts/Hmjf5Ws8g5w
Expand Down
8 changes: 8 additions & 0 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ const (
// KeyS3DisableSSL is the configuration key that determines whether SSL is to be used.
// any value other than 'true' is treated as false.
KeyS3DisableSSL = "s3-disable-ssl"
// KeyS3DisableSSLVerify is the configuration key that determines whether to verify the
// SSL certificate of the remote servers (in case a self-signed cert is used).
// any value other than 'true' is treated as false.
KeyS3DisableSSLVerify = "s3-disable-ssl-verify"
// KeyS3Region is the configuration key where the value is the S3 region
KeyS3Region = "s3-region"
// KeyAuthURL is the configuration key where the value is the KBase auth server URL
Expand Down Expand Up @@ -69,6 +73,8 @@ type Config struct {
S3AccessSecret string
// S3DisableSSL determines whether SSL should be used
S3DisableSSL bool
// S3DisableSSLVerify determines whether SSL certs should be verified
S3DisableSSLVerify bool
// S3Region is the S3 region
S3Region string
// AuthURL is the KBase auth server URL. It is never nil.
Expand Down Expand Up @@ -102,6 +108,7 @@ func New(configFilePath string) (*Config, error) {
s3key, err := getString(err, configFilePath, sec, KeyS3AccessKey, true)
s3secret, err := getString(err, configFilePath, sec, KeyS3AccessSecret, true)
s3disableSSL, err := getString(err, configFilePath, sec, KeyS3DisableSSL, false)
s3disableSSLVerify, err := getString(err, configFilePath, sec, KeyS3DisableSSLVerify, false)
s3region, err := getString(err, configFilePath, sec, KeyS3Region, true)
authurl, err := getURL(err, configFilePath, sec, KeyAuthURL)
roles, err := getStringList(err, configFilePath, sec, KeyAuthAdminRoles)
Expand All @@ -126,6 +133,7 @@ func New(configFilePath string) (*Config, error) {
S3AccessKey: s3key,
S3AccessSecret: s3secret,
S3DisableSSL: "true" == s3disableSSL,
S3DisableSSLVerify: "true" == s3disableSSLVerify,
S3Region: s3region,
AuthURL: authurl,
AuthAdminRoles: roles,
Expand Down
5 changes: 5 additions & 0 deletions config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ func (t *TestSuite) TestMinimalConfig() {
S3AccessSecret: "sooporsekrit",
S3Region: "us-west-1",
S3DisableSSL: false,
S3DisableSSLVerify: false,
AuthURL: u,
AuthAdminRoles: &[]string{},
DontTrustXIPHeaders: false,
Expand All @@ -109,6 +110,7 @@ func (t *TestSuite) TestMinimalConfigWhitespaceFields() {
"s3-access-key = akey",
"s3-access-secret = sooporsekrit",
"s3-disable-ssl = \t tru ",
"s3-disable-ssl-verify = \t tru ",
"s3-region = us-west-1 \t ",
"kbase-auth-url = https://kbase.us/authyauth",
"kbase-auth-admin-roles = \t ",
Expand All @@ -127,6 +129,7 @@ func (t *TestSuite) TestMinimalConfigWhitespaceFields() {
S3AccessSecret: "sooporsekrit",
S3Region: "us-west-1",
S3DisableSSL: false,
S3DisableSSLVerify: false,
AuthURL: u,
AuthAdminRoles: &[]string{},
DontTrustXIPHeaders: false,
Expand All @@ -147,6 +150,7 @@ func (t *TestSuite) TestMaximalConfig() {
"s3-access-secret = sooporsekrit",
"s3-region = us-west-1",
"s3-disable-ssl= true ",
"s3-disable-ssl-verify= true ",
"kbase-auth-url = https://kbase.us/authyauth",
"kbase-auth-admin-roles = \t , foo , \tbar\t , , baz ,,",
"dont-trust-x-ip-headers = true \t ",
Expand All @@ -165,6 +169,7 @@ func (t *TestSuite) TestMaximalConfig() {
S3AccessKey: "akey",
S3AccessSecret: "sooporsekrit",
S3DisableSSL: true,
S3DisableSSLVerify: true,
S3Region: "us-west-1",
AuthURL: u,
AuthAdminRoles: &[]string{"foo", "bar", "baz"},
Expand Down
7 changes: 5 additions & 2 deletions deploy.cfg.example
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ mongodb-database = blobstore
#mongodb-user = [username]
#mongodb-pwd = [password]

# S3 API parameters. All are required other than disable-ssl.
# S3 API parameters. All are required other than s3-disable-ssl and s3-disable-ssl-verify.
# disable-ssl treats any value other than 'true' as false.
s3-host = localhost:9000
# The bucket name must obey https://docs.aws.amazon.com/AmazonS3/latest/dev/BucketRestrictions.html
Expand All @@ -20,7 +20,10 @@ s3-bucket = blobstore
s3-access-key = [access key goes here]
s3-access-secret = [access secret goes here]
s3-region = us-west-1
# Use plaintext to talk to destination S3. Default false.
#s3-disable-ssl = false
# Disable verifying the destination S3 SSL cert (e.g. for self-signed certs). Default false.
#s3-disable-ssl-verify = false

# KBase auth server parameters.
# The root url of the auth server.
Expand All @@ -31,4 +34,4 @@ kbase-auth-admin-roles = KBASE_ADMIN, BLOBSTORE_ADMIN
# If "true", make the server ignore the X-Forwarded-For and X-Real-IP headers. Otherwise
# (the default behavior), the logged IP address for a request, in order of precedence, is
# 1) the first address in X-Forwarded-For, 2) X-Real-IP, and 3) the address of the client.
dont-trust-x-ip-headers = false
dont-trust-x-ip-headers = false
3 changes: 2 additions & 1 deletion deployment/conf/deployment.cfg.templ
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ s3-access-key = {{ default .Env.s3_access_key "" }}
s3-access-secret = {{ default .Env.s3_access_secret "" }}
s3-region = {{ default .Env.s3_region "us-west-1" }}
s3-disable-ssl = {{ default .Env.s3_disable_ssl "false" }}
s3-disable-ssl-verify = {{ default .Env.s3_disable_ssl_verify "false" }}

# KBase auth server parameters.
# The root url of the auth server.
Expand All @@ -31,4 +32,4 @@ kbase-auth-admin-roles = {{ default .Env.kbase_auth_admin_roles "KBASE_ADMIN, BL
# If "true", make the server ignore the X-Forwarded-For and X-Real-IP headers. Otherwise
# (the default behavior), the logged IP address for a request, in order of precedence, is
# 1) the first address in X-Forwarded-For, 2) X-Real-IP, and 3) the address of the client.
dont-trust-x-ip-headers = {{ default .Env.dont_trust_x_ip_headers "false" }}
dont-trust-x-ip-headers = {{ default .Env.dont_trust_x_ip_headers "false" }}
14 changes: 11 additions & 3 deletions filestore/s3.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,17 +33,22 @@ type S3FileStore struct {
s3client *s3.S3
minioClient *minio.Client
bucket string
httpClient *http.Client
}

// NewS3FileStore creates a new S3 based file store. Files will be stored in the provided
// bucket, which will be created if it doesn't exist. The provided clients must have write
// privileges for the bucket.
// Two clients are currently required because they are better at different operations.
// Three clients are currently required because they are better at different operations:
// s3client: an aws-sdk s3 client
// minioClient: a minio-go client
// httpClient: an http.Client client (used directly for faster PUTs)
// This may change in a future version if one client provides all the necessary operations.
func NewS3FileStore(
s3client *s3.S3,
minioClient *minio.Client,
bucket string,
httpClient *http.Client,
) (*S3FileStore, error) {

if s3client == nil {
Expand All @@ -52,6 +57,9 @@ func NewS3FileStore(
if minioClient == nil {
return nil, errors.New("minioClient cannot be nil")
}
if httpClient == nil {
return nil, errors.New("httpClient cannot be nil")
}
bucket, err := checkBucketName(bucket)
if err != nil {
return nil, err
Expand All @@ -62,7 +70,7 @@ func NewS3FileStore(
// Ignore for now.
return nil, err
}
return &S3FileStore{s3client: s3client, minioClient: minioClient, bucket: bucket}, nil
return &S3FileStore{s3client: s3client, minioClient: minioClient, bucket: bucket, httpClient: httpClient}, nil
}

func checkBucketName(bucket string) (string, error) {
Expand Down Expand Up @@ -129,7 +137,7 @@ func (fs *S3FileStore) StoreFile(le *logrus.Entry, p *StoreFileParams) (out *Fil
req.Header.Set("x-amz-meta-Filename", p.filename)
req.Header.Set("x-amz-meta-Format", p.format)

resp, err := http.DefaultClient.Do(req)
resp, err := fs.httpClient.Do(req)
if err != nil {
// don't expose the presigned url in the returned error
errstr := err.(*url.Error).Err.Error()
Expand Down
Loading

0 comments on commit 848cd2e

Please sign in to comment.