Repository: distribution/distribution
Pull request #3576 (2022-01-24 12:05:21)

optimize: disable insecure cipher suites
### background
github code scan detects the insecure configurations of distribution.

### solution
This commit removes the following cipher suites that are known to be insecure:

TLS_RSA_WITH_RC4_128_SHA
TLS_RSA_WITH_AES_128_CBC_SHA256
TLS_ECDHE_ECDSA_WITH_RC4_128_SHA
TLS_ECDHE_RSA_WITH_RC4_128_SHA
TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256
TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256

And this commit deletes the tlsVersions of tls1.0 and tls1.1. The tls1.2 is the minimal supported tls version for creating a safer tls configuration.

Signed-off-by: baojiangnan <baojn1998@163.com>

justadogistaken: ref: https://github.com/distribution/distribution/pull/3569/checks?check_run_id=4919268905
justadogistaken: /ptal @milosgajdos @thaJeztah 
codecov-commenter: # [Codecov](https://codecov.io/gh/distribution/distribution/pull/3576?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) Report
> Merging [#3576](https://codecov.io/gh/distribution/distribution/pull/3576?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) (d070824) into [main](https://codecov.io/gh/distribution/distribution/commit/5f1974ab8b537b5bbce467971b05ee62c441fd7a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) (5f1974a) will **not change** coverage.
> The diff coverage is `n/a`.

[![Impacted file tree graph](https://codecov.io/gh/distribution/distribution/pull/3576/graphs/tree.svg?width=650&height=150&src=pr&token=q4gaG5NuY3&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None)](https://codecov.io/gh/distribution/distribution/pull/3576?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None)

```diff
@@           Coverage Diff           @@
##             main    #3576   +/-   ##
=======================================
  Coverage   56.36%   56.36%           
=======================================
  Files         101      101           
  Lines        7310     7310           
=======================================
  Hits         4120     4120           
  Misses       2535     2535           
  Partials      655      655           
```


| [Impacted Files](https://codecov.io/gh/distribution/distribution/pull/3576?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | |
|---|---|---|
| [...com/distribution/distribution/registry/registry.go](https://codecov.io/gh/distribution/distribution/pull/3576/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2dpdGh1Yi5jb20vZGlzdHJpYnV0aW9uL2Rpc3RyaWJ1dGlvbi9yZWdpc3RyeS9yZWdpc3RyeS5nbw==) | `45.79% <0.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/distribution/distribution/pull/3576?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/distribution/distribution/pull/3576?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None). Last update [5f1974a...d070824](https://codecov.io/gh/distribution/distribution/pull/3576?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None).


Patch:
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
index 8fd38703..2502a94d 100644
--- a/.github/workflows/build.yml
+++ b/.github/workflows/build.yml
@@ -30,20 +30,8 @@ jobs:
         with:
           images: |
             ${{ env.DOCKERHUB_SLUG }}
-          ### versioning strategy
-          ### push semver tag v3.2.1 on main (default branch)
-          # distribution/distribution:3.2.1
-          # distribution/distribution:3.2
-          # distribution/distribution:3
-          # distribution/distribution:latest
-          ### push semver prelease tag v3.0.0-beta.1 on main (default branch)
-          # distribution/distribution:3.0.0-beta.1
-          ### push on main
-          # distribution/distribution:edge
           tags: |
             type=semver,pattern={{version}}
-            type=semver,pattern={{major}}.{{minor}}
-            type=semver,pattern={{major}}
             type=ref,event=pr
             type=edge
           labels: |
@@ -93,6 +81,5 @@ jobs:
           files: |
             bin/*.tar.gz
             bin/*.zip
-            bin/*.sha256
         env:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
diff --git a/Dockerfile b/Dockerfile
index d80ac59d..723949c3 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -31,12 +31,11 @@ RUN --mount=type=bind,target=/src,rw \
 FROM scratch AS artifacts
 COPY --from=build /out/*.tar.gz /
 COPY --from=build /out/*.zip /
-COPY --from=build /out/*.sha256 /
 
 FROM scratch AS binary
 COPY --from=build /usr/local/bin/registry* /
 
-FROM alpine:3.15
+FROM alpine:3.14
 RUN apk add --no-cache ca-certificates
 COPY cmd/registry/config-dev.yml /etc/docker/registry/config.yml
 COPY --from=build /usr/local/bin/registry /bin/registry
diff --git a/docker-bake.hcl b/docker-bake.hcl
index e1457bb8..10a22b75 100644
--- a/docker-bake.hcl
+++ b/docker-bake.hcl
@@ -20,7 +20,10 @@ target "artifact" {
 target "artifact-all" {
   inherits = ["artifact"]
   platforms = [
+    "darwin/amd64",
+    "darwin/arm64",
     "linux/amd64",
+    "linux/arm/v5",
     "linux/arm/v6",
     "linux/arm/v7",
     "linux/arm64",
diff --git a/registry/registry.go b/registry/registry.go
index 229d1339..625102df 100644
--- a/registry/registry.go
+++ b/registry/registry.go
@@ -37,22 +37,16 @@ import (
 // a map of TLS cipher suite names to constants in https://golang.org/pkg/crypto/tls/#pkg-constants
 var cipherSuites = map[string]uint16{
 	// TLS 1.0 - 1.2 cipher suites
-	"TLS_RSA_WITH_RC4_128_SHA":                      tls.TLS_RSA_WITH_RC4_128_SHA,
 	"TLS_RSA_WITH_3DES_EDE_CBC_SHA":                 tls.TLS_RSA_WITH_3DES_EDE_CBC_SHA,
 	"TLS_RSA_WITH_AES_128_CBC_SHA":                  tls.TLS_RSA_WITH_AES_128_CBC_SHA,
 	"TLS_RSA_WITH_AES_256_CBC_SHA":                  tls.TLS_RSA_WITH_AES_256_CBC_SHA,
-	"TLS_RSA_WITH_AES_128_CBC_SHA256":               tls.TLS_RSA_WITH_AES_128_CBC_SHA256,
 	"TLS_RSA_WITH_AES_128_GCM_SHA256":               tls.TLS_RSA_WITH_AES_128_GCM_SHA256,
 	"TLS_RSA_WITH_AES_256_GCM_SHA384":               tls.TLS_RSA_WITH_AES_256_GCM_SHA384,
-	"TLS_ECDHE_ECDSA_WITH_RC4_128_SHA":              tls.TLS_ECDHE_ECDSA_WITH_RC4_128_SHA,
 	"TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA":          tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA,
 	"TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA":          tls.TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA,
-	"TLS_ECDHE_RSA_WITH_RC4_128_SHA":                tls.TLS_ECDHE_RSA_WITH_RC4_128_SHA,
 	"TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA":           tls.TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA,
 	"TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA":            tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA,
 	"TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA":            tls.TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA,
-	"TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256":       tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256,
-	"TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256":         tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256,
 	"TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256":         tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,
 	"TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256":       tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,
 	"TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384":         tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,
@@ -82,8 +76,6 @@ var defaultCipherSuites = []uint16{
 var defaultTLSVersionStr = "tls1.2"
 var tlsVersions = map[string]uint16{
 	// user specified values
-	"tls1.0": tls.VersionTLS10,
-	"tls1.1": tls.VersionTLS11,
 	"tls1.2": tls.VersionTLS12,
 	"tls1.3": tls.VersionTLS13,
 }
@@ -230,11 +222,18 @@ func (registry *Registry) ListenAndServe() error {
 		}
 		dcontext.GetLogger(registry.app).Infof("restricting TLS version to %s or higher", config.HTTP.TLS.MinimumTLS)
 
-		tlsCipherSuites, err := getCipherSuites(config.HTTP.TLS.CipherSuites)
-		if err != nil {
-			return err
+		var tlsCipherSuites []uint16
+		// configuring cipher suites are no longer supported after the tls1.3.
+		// (https://go.dev/blog/tls-cipher-suites)
+		if tlsMinVersion > tls.VersionTLS12 {
+			dcontext.GetLogger(registry.app).Warnf("restricting TLS cipher suites to empty. Because configuring cipher suites is no longer supported in %s", config.HTTP.TLS.MinimumTLS)
+		} else {
+			tlsCipherSuites, err = getCipherSuites(config.HTTP.TLS.CipherSuites)
+			if err != nil {
+				return err
+			}
+			dcontext.GetLogger(registry.app).Infof("restricting TLS cipher suites to: %s", strings.Join(getCipherSuiteNames(tlsCipherSuites), ","))
 		}
-		dcontext.GetLogger(registry.app).Infof("restricting TLS cipher suites to: %s", strings.Join(getCipherSuiteNames(tlsCipherSuites), ","))
 
 		tlsConf := &tls.Config{
 			ClientAuth:               tls.NoClientCert,


Test patch:
diff --git a/registry/proxy/proxyblobstore_test.go b/registry/proxy/proxyblobstore_test.go
index e95bf20c..2ba26738 100644
--- a/registry/proxy/proxyblobstore_test.go
+++ b/registry/proxy/proxyblobstore_test.go
@@ -76,7 +76,6 @@ func (sbs statsBlobStore) ServeBlob(ctx context.Context, w http.ResponseWriter,
 }
 
 func (sbs statsBlobStore) Stat(ctx context.Context, dgst digest.Digest) (distribution.Descriptor, error) {
-
 	sbsMu.Lock()
 	sbs.stats["stat"]++
 	sbsMu.Unlock()
diff --git a/registry/registry_test.go b/registry/registry_test.go
index e0d5c218..15c91363 100644
--- a/registry/registry_test.go
+++ b/registry/registry_test.go
@@ -135,7 +135,10 @@ func TestGetCipherSuite(t *testing.T) {
 		)
 	}
 
-	resp, err = getCipherSuites([]string{"TLS_RSA_WITH_AES_128_CBC_SHA", "TLS_AES_128_GCM_SHA256"})
+	resp, err = getCipherSuites([]string{
+		"TLS_RSA_WITH_AES_128_CBC_SHA",
+		"TLS_AES_128_GCM_SHA256",
+	})
 	if err != nil || len(resp) != 2 ||
 		resp[0] != tls.TLS_RSA_WITH_AES_128_CBC_SHA || resp[1] != tls.TLS_AES_128_GCM_SHA256 {
 		t.Errorf("expected cipher suites %q, got %q",
@@ -148,6 +151,22 @@ func TestGetCipherSuite(t *testing.T) {
 	if err == nil {
 		t.Error("did not return expected error about unknown cipher suite")
 	}
+
+	var insecureCipherSuites = []string{
+		"TLS_RSA_WITH_RC4_128_SHA",
+		"TLS_RSA_WITH_AES_128_CBC_SHA256",
+		"TLS_ECDHE_ECDSA_WITH_RC4_128_SHA",
+		"TLS_ECDHE_RSA_WITH_RC4_128_SHA",
+		"TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256",
+		"TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256",
+	}
+
+	for _, suite := range insecureCipherSuites {
+		_, err = getCipherSuites([]string{suite})
+		if err == nil {
+			t.Errorf("Unexpected insecure cipher suite: %s", suite)
+		}
+	}
 }
 
 func buildRegistryTLSConfig(name, keyType string, cipherSuites []string) (*registryTLSConfig, error) {
diff --git a/registry/storage/purgeuploads_test.go b/registry/storage/purgeuploads_test.go
index 7ee127c0..4949ad18 100644
--- a/registry/storage/purgeuploads_test.go
+++ b/registry/storage/purgeuploads_test.go
@@ -46,7 +46,7 @@ func TestPurgeGather(t *testing.T) {
 	fs, ctx := testUploadFS(t, uploadCount, "test-repo", time.Now())
 	uploadData, errs := getOutstandingUploads(ctx, fs)
 	if len(errs) != 0 {
-		t.Errorf("Unexepected errors: %q", errs)
+		t.Errorf("Unexpected errors: %q", errs)
 	}
 	if len(uploadData) != uploadCount {
 		t.Errorf("Unexpected upload file count: %d != %d", uploadCount, len(uploadData))