From 9ea45399ceb40e5b70b19e3578da8193ba5f1c2a Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Mon, 11 Oct 2021 14:23:51 -0700 Subject: [PATCH] fix: enable AssumeRoleWithCertificate API only when asked (#13410) This is a breaking change but we need to do this to avoid issues discussed in #13409 based on discussions from #13371 fixes #13371 fixes #13409 --- cmd/config-current.go | 4 ++++ docs/sts/tls.md | 9 +++++++-- internal/config/identity/tls/config.go | 16 ++++++---------- internal/http/server.go | 9 +++++++-- 4 files changed, 24 insertions(+), 14 deletions(-) diff --git a/cmd/config-current.go b/cmd/config-current.go index 7a1dd8ae9..0d37dccaf 100644 --- a/cmd/config-current.go +++ b/cmd/config-current.go @@ -496,6 +496,10 @@ func lookupConfigs(s config.Config, objAPI ObjectLayer) { logger.LogIf(ctx, fmt.Errorf("Unable to initialize X.509/TLS STS API: %w", err)) } + if globalSTSTLSConfig.InsecureSkipVerify { + logger.Info("CRITICAL: enabling %s is not recommended in a production environment", xtls.EnvIdentityTLSSkipVerify) + } + globalOpenIDConfig, err = openid.LookupConfig(s[config.IdentityOpenIDSubSys][config.Default], NewGatewayHTTPTransport(), xhttp.DrainBody) if err != nil { diff --git a/docs/sts/tls.md b/docs/sts/tls.md index 50dedfc35..f1b3a009e 100644 --- a/docs/sts/tls.md +++ b/docs/sts/tls.md @@ -16,9 +16,9 @@ ARGS: MINIO_IDENTITY_TLS_SKIP_VERIFY (on|off) trust client certificates without verification. Defaults to "off" (verify) ``` -The MinIO TLS STS API is enabled by default. However, it can be completely *disabled* by setting: +The MinIO TLS STS API is disabled by default. However, it can be *enabled* by setting environment variable: ``` -MINIO_IDENTITY_TLS_ENABLE=off +export MINIO_IDENTITY_TLS_ENABLE=on ``` ## Example @@ -102,6 +102,11 @@ The returned credentials expiry after a certain period of time that can be confi Further, the temp. S3 credentials will never out-live the client certificate. For example, if the `MINIO_IDENTITY_TLS_STS_EXPIRY` is 7 days but the certificate itself is only valid for the next 3 days, then MinIO will return S3 credentials that are valid for 3 days only. +## Caveat + +*Applications that use direct S3 API will work fine, however interactive users uploading content using (when POSTing to the presigned URL an app generates) a popup becomes visible on browser to provide client certs, you would have to manually cancel and continue. This may be annoying to use but there is no workaround for now.* + + ## Explore Further - [MinIO Admin Complete Guide](https://docs.min.io/docs/minio-admin-complete-guide.html) - [The MinIO documentation website](https://docs.min.io) diff --git a/internal/config/identity/tls/config.go b/internal/config/identity/tls/config.go index f2e3b59fe..bf2d6ddc6 100644 --- a/internal/config/identity/tls/config.go +++ b/internal/config/identity/tls/config.go @@ -23,16 +23,15 @@ import ( "github.com/minio/minio/internal/auth" "github.com/minio/minio/internal/config" - "github.com/minio/minio/internal/logger" "github.com/minio/pkg/env" ) const ( - // EnvEnabled is an environment variable that controls whether the X.509 + // EnvIdentityTLSEnabled is an environment variable that controls whether the X.509 // TLS STS API is enabled. By default, if not set, it is enabled. - EnvEnabled = "MINIO_IDENTITY_TLS_ENABLE" + EnvIdentityTLSEnabled = "MINIO_IDENTITY_TLS_ENABLE" - // EnvSkipVerify is an environment variable that controls whether + // EnvIdentityTLSSkipVerify is an environment variable that controls whether // MinIO verifies the client certificate present by the client // when requesting temp. credentials. // By default, MinIO always verify the client certificate. @@ -41,7 +40,7 @@ const ( // when debugging or testing a setup since it allows arbitrary // clients to obtain temp. credentials with arbitrary policy // permissions - including admin permissions. - EnvSkipVerify = "MINIO_IDENTITY_TLS_SKIP_VERIFY" + EnvIdentityTLSSkipVerify = "MINIO_IDENTITY_TLS_SKIP_VERIFY" ) // Config contains the STS TLS configuration for generating temp. @@ -86,14 +85,11 @@ func Lookup(kvs config.KVS) (Config, error) { if err := config.CheckValidKeys(config.IdentityTLSSubSys, kvs, DefaultKVS); err != nil { return Config{}, err } - insecureSkipVerify, err := config.ParseBool(env.Get(EnvSkipVerify, kvs.Get(skipVerify))) + insecureSkipVerify, err := config.ParseBool(env.Get(EnvIdentityTLSSkipVerify, kvs.Get(skipVerify))) if err != nil { return Config{}, err } - if insecureSkipVerify { - logger.Info("CRITICAL: enabling MINIO_IDENTITY_TLS_SKIP_VERIFY is not recommended in a production environment") - } - enabled, err := config.ParseBool(env.Get(EnvEnabled, config.EnableOn)) + enabled, err := config.ParseBool(env.Get(EnvIdentityTLSEnabled, "")) if err != nil { return Config{}, err } diff --git a/internal/http/server.go b/internal/http/server.go index d558d5e9f..1536e056d 100644 --- a/internal/http/server.go +++ b/internal/http/server.go @@ -32,6 +32,7 @@ import ( "github.com/minio/minio/internal/config" "github.com/minio/minio/internal/config/api" + xtls "github.com/minio/minio/internal/config/identity/tls" "github.com/minio/minio/internal/fips" "github.com/minio/pkg/certs" "github.com/minio/pkg/env" @@ -163,7 +164,6 @@ func (srv *Server) Shutdown() error { // NewServer - creates new HTTP server using given arguments. func NewServer(addrs []string, handler http.Handler, getCert certs.GetCertificateFunc) *Server { secureCiphers := env.Get(api.EnvAPISecureCiphers, config.EnableOn) == config.EnableOn - var tlsConfig *tls.Config if getCert != nil { tlsConfig = &tls.Config{ @@ -171,8 +171,13 @@ func NewServer(addrs []string, handler http.Handler, getCert certs.GetCertificat MinVersion: tls.VersionTLS12, NextProtos: []string{"http/1.1", "h2"}, GetCertificate: getCert, - ClientAuth: tls.RequestClientCert, } + + tlsClientIdentity := env.Get(xtls.EnvIdentityTLSEnabled, "") == config.EnableOn + if tlsClientIdentity { + tlsConfig.ClientAuth = tls.RequestClientCert + } + if secureCiphers || fips.Enabled { tlsConfig.CipherSuites = fips.CipherSuitesTLS() tlsConfig.CurvePreferences = fips.EllipticCurvesTLS()