[#562] Empty default value for TLS termination header param

If the service is accessed not through a proxy and the
default value of the parameter with the header key is
not empty, then the system administrator does not
control disabling TLS verification in any way, because
the client can simply add a known header, thereby
skipping the verification. Therefore, the default value
of the header parameter is made empty. If it is empty,
then TLS verification cannot be disabled in any way.
Thus, the system administrator will be able to control
the enabling/disabling of TLS.

Signed-off-by: Roman Loginov <r.loginov@yadro.com>

api/handler/handlers_test.go

@@ -78,6 +78,7 @@ type configMock struct {
 	defaultCopiesNumbers          []uint32
 	bypassContentEncodingInChunks bool
 	md5Enabled                    bool
+	tlsTerminationHeader          string
 }
 
 func (c *configMock) DefaultPlacementPolicy(_ string) netmap.PlacementPolicy {
@@ -142,7 +143,7 @@ func (c *configMock) RetryStrategy() RetryStrategy {
 }
 
 func (c *configMock) TLSTerminationHeader() string {
-	return "X-Frostfs-TLS-Termination"
+	return c.tlsTerminationHeader
 }
 
 func prepareHandlerContext(t *testing.T) *handlerContext {

api/handler/put.go

@@ -387,17 +387,7 @@ func (h *handler) formEncryptionParamsBase(r *http.Request, isCopySource bool) (
 		return
 	}
 
-	needCheckTLS := true
+	if h.isTLSCheckRequired(r) && r.TLS == nil {
-	if tlsTerminationStr := r.Header.Get(h.cfg.TLSTerminationHeader()); len(tlsTerminationStr) > 0 {
-		tlsTermination, err := strconv.ParseBool(tlsTerminationStr)
-		if err != nil {
-			h.reqLogger(r.Context()).Warn(logs.WarnInvalidTypeTLSTerminationHeader, zap.String("header", tlsTerminationStr), zap.Error(err))
-		} else {
-			needCheckTLS = !tlsTermination
-		}
-	}
-
-	if needCheckTLS && r.TLS == nil {
 		return enc, apierr.GetAPIError(apierr.ErrInsecureSSECustomerRequest)
 	}
 
@@ -445,6 +435,26 @@ func (h *handler) formEncryptionParamsBase(r *http.Request, isCopySource bool) (
 	return enc, err
 }
 
+func (h *handler) isTLSCheckRequired(r *http.Request) bool {
+	header := h.cfg.TLSTerminationHeader()
+	if header == "" {
+		return true
+	}
+
+	tlsTerminationStr := r.Header.Get(header)
+	if tlsTerminationStr == "" {
+		return true
+	}
+
+	tlsTermination, err := strconv.ParseBool(tlsTerminationStr)
+	if err != nil {
+		h.reqLogger(r.Context()).Warn(logs.WarnInvalidTypeTLSTerminationHeader, zap.String("header", tlsTerminationStr), zap.Error(err))
+		return true
+	}
+
+	return !tlsTermination
+}
+
 func (h *handler) PostObject(w http.ResponseWriter, r *http.Request) {
 	var (
 		tagSet   map[string]string

api/handler/put_test.go

@@ -636,8 +636,6 @@ func TestPutObjectWithContentLanguage(t *testing.T) {
 }
 
 func TestFormEncryptionParamsBase(t *testing.T) {
-	hc := prepareHandlerContext(t)
-
 	userSecret := "test1customer2secret3with32char4"
 	expectedEncKey := []byte(userSecret)
 	emptyEncKey := []byte(nil)
@@ -770,7 +768,8 @@ func TestFormEncryptionParamsBase(t *testing.T) {
 		},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
-			r := prepareRequestForEncryption(tc.algo, tc.key, tc.md5, tc.tlsTermination, tc.reqWithoutTLS, tc.reqWithoutSSE, tc.isCopySource)
+			hc := prepareHandlerContext(t)
+			r := prepareRequestForEncryption(hc, tc.algo, tc.key, tc.md5, tc.tlsTermination, tc.reqWithoutTLS, tc.reqWithoutSSE, tc.isCopySource)
 
 			enc, err := hc.h.formEncryptionParamsBase(r, tc.isCopySource)
 			if tc.err != nil {
@@ -789,7 +788,7 @@ func TestFormEncryptionParamsBase(t *testing.T) {
 	}
 }
 
-func prepareRequestForEncryption(algo, key, md5, tlsTermination string, reqWithoutTLS, reqWithoutSSE, isCopySource bool) *http.Request {
+func prepareRequestForEncryption(hc *handlerContext, algo, key, md5, tlsTermination string, reqWithoutTLS, reqWithoutSSE, isCopySource bool) *http.Request {
 	r := httptest.NewRequest(http.MethodPost, "/", nil)
 
 	if !reqWithoutTLS {
@@ -808,8 +807,10 @@ func prepareRequestForEncryption(algo, key, md5, tlsTermination string, reqWitho
 		}
 	}
 
+	customHeader := "X-Frostfs-TLS-Termination"
 	if tlsTermination != "" {
-		r.Header.Set("X-Frostfs-TLS-Termination", tlsTermination)
+		hc.config.tlsTerminationHeader = customHeader
+		r.Header.Set(customHeader, tlsTermination)
 	}
 
 	return r

cmd/s3-gw/app_settings.go

@@ -59,10 +59,9 @@ const (
 
 	defaultAccessBoxCacheRemovingCheckInterval = 5 * time.Minute
 
-	defaultNamespaceHeader      = "X-Frostfs-Namespace"
+	defaultNamespaceHeader  = "X-Frostfs-Namespace"
-	defaultVHSHeader            = "X-Frostfs-S3-VHS"
+	defaultVHSHeader        = "X-Frostfs-S3-VHS"
-	defaultServernameHeader     = "X-Frostfs-Servername"
+	defaultServernameHeader = "X-Frostfs-Servername"
-	defaultTLSTerminationHeader = "X-Frostfs-TLS-Termination"
 
 	defaultMultinetFallbackDelay = 300 * time.Millisecond
 
@@ -950,9 +949,6 @@ func newSettings() *viper.Viper {
 	// multinet
 	v.SetDefault(cfgMultinetFallbackDelay, defaultMultinetFallbackDelay)
 
-	// encryption
-	v.SetDefault(cfgEncryptionTLSTerminationHeader, defaultTLSTerminationHeader)
-
 	// Bind flags
 	if err := bindFlags(v, flags); err != nil {
 		panic(fmt.Errorf("bind flags: %w", err))

docs/configuration.md

@@ -869,6 +869,6 @@ encryption:
   tls_termination_header: X-Frostfs-TLS-Termination
 ```
 
-| Parameter                | Type     | SIGHUP reload | Default value               | Description                                                                                                                                                                                                                                |
+| Parameter                | Type     | SIGHUP reload | Default value | Description                                                                                                                                                                                                                                                                                                                                   |
-|--------------------------|----------|---------------|-----------------------------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
+|--------------------------|----------|---------------|---------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
-| `tls_termination_header` | `string` | yes           | `X-Frostfs-TLS-Termination` | The header for determining whether TLS needs to be checked. If the system requests come through a proxy server and TLS can terminate at the proxy level, you should use this header to disable TLS verification at server-side encryption. |
+| `tls_termination_header` | `string` | yes           |               | The header for determining whether TLS needs to be checked. If the system requests come through a proxy server and TLS can terminate at the proxy level, you should use this header to disable TLS verification at server-side encryption. If the header is not specified or an empty string is set as the value, TLS will always be checked. |