Merge pull request #133 from kkellerlbl/s3-disable-ssl-verify

improvements to s3-disable-ssl-verify config option

RELEASE_NOTES.md

@@ -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

app/blobstore.go

@@ -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"
@@ -53,6 +54,7 @@ func main() {
 			ServerVersionCompat: shockver,
 			DeprecationWarning:  deprecation,
 			GitCommit:           gitCommit,
+			HTTPTimeout:         httpTimeout,
 		},
 	)
 	if err != nil {
@@ -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

config/config_test.go

@@ -90,6 +90,7 @@ func (t *TestSuite) TestMinimalConfig() {
 		S3AccessSecret:      "sooporsekrit",
 		S3Region:            "us-west-1",
 		S3DisableSSL:        false,
+		S3DisableSSLVerify:  false,
 		AuthURL:             u,
 		AuthAdminRoles:      &[]string{},
 		DontTrustXIPHeaders: false,
@@ -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     ",
@@ -127,6 +129,7 @@ func (t *TestSuite) TestMinimalConfigWhitespaceFields() {
 		S3AccessSecret:      "sooporsekrit",
 		S3Region:            "us-west-1",
 		S3DisableSSL:        false,
+		S3DisableSSLVerify:  false,
 		AuthURL:             u,
 		AuthAdminRoles:      &[]string{},
 		DontTrustXIPHeaders: false,
@@ -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  ",
@@ -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"},

deploy.cfg.example

@@ -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
@@ -20,7 +20,7 @@ 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. (false is not tested)
+# 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

filestore/s3.go

@@ -6,7 +6,6 @@ import (
 	"io"
 	"net/http"
 	"net/url"
-	"crypto/tls"
 	"strconv"
 	"strings"
 	"time"
@@ -34,19 +33,22 @@ type S3FileStore struct {
 	s3client    *s3.S3
 	minioClient *minio.Client
 	bucket      string
-	disableSSLverify	bool
+	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,
-	disableSSLverify bool,
+	httpClient *http.Client,
 ) (*S3FileStore, error) {
 
 	if s3client == nil {
@@ -55,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
@@ -65,7 +70,7 @@ func NewS3FileStore(
 		// Ignore for now.
 		return nil, err
 	}
-	return &S3FileStore{s3client: s3client, minioClient: minioClient, bucket: bucket, disableSSLverify: disableSSLverify}, nil
+	return &S3FileStore{s3client: s3client, minioClient: minioClient, bucket: bucket, httpClient: httpClient}, nil
 }
 
 func checkBucketName(bucket string) (string, error) {
@@ -132,16 +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)
 
-	// disable SSL verify if necessary
-	customTransport := &http.Transport{
-            TLSClientConfig: &tls.Config{InsecureSkipVerify: fs.disableSSLverify},
-        }
-//		  Timeout: time.Second * 10,
-	httpClient := &http.Client{
-	    Transport: customTransport,
-	}
-
-	resp, err := httpClient.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()