Skip to content

Commit f1f5221

Browse files
openshift-merge-robotopenshift-merge-robot
authored
Merge pull request #16885 from dmage/layers-metadata
Automatic merge from submit-queue. Verify layer sizes in the integrated registry cc @miminar @legionus fixes bug https://bugzilla.redhat.com/show_bug.cgi?id=1491589
2 parents 145cdaf + fd23261 commit f1f5221

15 files changed

+756
-469
lines changed

‎pkg/cmd/dockerregistry/dockerregistry.go‎

Lines changed: 54 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -141,13 +141,22 @@ func Execute(configFile io.Reader) {
141141
if err != nil {
142142
log.Fatalf("error parsing configuration file: %s", err)
143143
}
144+
145+
err = Start(dockerConfig, extraConfig)
146+
if err != nil {
147+
log.Fatal(err)
148+
}
149+
}
150+
151+
// Start runs the Docker registry. Start always returns a non-nil error.
152+
func Start(dockerConfig *configuration.Configuration, extraConfig *registryconfig.Configuration) error {
144153
setDefaultMiddleware(dockerConfig)
145154
setDefaultLogParameters(dockerConfig)
146155

147156
ctx := context.Background()
148-
ctx, err = configureLogging(ctx, dockerConfig)
157+
ctx, err := configureLogging(ctx, dockerConfig)
149158
if err != nil {
150-
log.Fatalf("error configuring logger: %v", err)
159+
return fmt.Errorf("error configuring logger: %v", err)
151160
}
152161

153162
log.WithFields(versionFields()).Info("start registry")
@@ -171,69 +180,64 @@ func Execute(configFile io.Reader) {
171180

172181
if dockerConfig.HTTP.TLS.Certificate == "" {
173182
context.GetLogger(ctx).Infof("listening on %v", dockerConfig.HTTP.Addr)
174-
if err := http.ListenAndServe(dockerConfig.HTTP.Addr, handler); err != nil {
175-
context.GetLogger(ctx).Fatalln(err)
183+
return http.ListenAndServe(dockerConfig.HTTP.Addr, handler)
184+
}
185+
186+
var (
187+
minVersion uint16
188+
cipherSuites []uint16
189+
)
190+
if s := os.Getenv("REGISTRY_HTTP_TLS_MINVERSION"); len(s) > 0 {
191+
minVersion, err = crypto.TLSVersion(s)
192+
if err != nil {
193+
return fmt.Errorf("invalid TLS version %q specified in REGISTRY_HTTP_TLS_MINVERSION: %v (valid values are %q)", s, err, crypto.ValidTLSVersions())
176194
}
177-
} else {
178-
var (
179-
minVersion uint16
180-
cipherSuites []uint16
181-
)
182-
if s := os.Getenv("REGISTRY_HTTP_TLS_MINVERSION"); len(s) > 0 {
183-
minVersion, err = crypto.TLSVersion(s)
195+
}
196+
if s := os.Getenv("REGISTRY_HTTP_TLS_CIPHERSUITES"); len(s) > 0 {
197+
for _, cipher := range strings.Split(s, ",") {
198+
cipherSuite, err := crypto.CipherSuite(cipher)
184199
if err != nil {
185-
context.GetLogger(ctx).Fatalln(fmt.Errorf("invalid TLS version %q specified in REGISTRY_HTTP_TLS_MINVERSION: %v (valid values are %q)", s, err, crypto.ValidTLSVersions()))
186-
}
187-
}
188-
if s := os.Getenv("REGISTRY_HTTP_TLS_CIPHERSUITES"); len(s) > 0 {
189-
for _, cipher := range strings.Split(s, ",") {
190-
cipherSuite, err := crypto.CipherSuite(cipher)
191-
if err != nil {
192-
context.GetLogger(ctx).Fatalln(fmt.Errorf("invalid cipher suite %q specified in REGISTRY_HTTP_TLS_CIPHERSUITES: %v (valid suites are %q)", s, err, crypto.ValidCipherSuites()))
193-
}
194-
cipherSuites = append(cipherSuites, cipherSuite)
200+
return fmt.Errorf("invalid cipher suite %q specified in REGISTRY_HTTP_TLS_CIPHERSUITES: %v (valid suites are %q)", s, err, crypto.ValidCipherSuites())
195201
}
202+
cipherSuites = append(cipherSuites, cipherSuite)
196203
}
204+
}
197205

198-
tlsConf := crypto.SecureTLSConfig(&tls.Config{
199-
ClientAuth: tls.NoClientCert,
200-
MinVersion: minVersion,
201-
CipherSuites: cipherSuites,
202-
})
203-
204-
if len(dockerConfig.HTTP.TLS.ClientCAs) != 0 {
205-
pool := x509.NewCertPool()
206+
tlsConf := crypto.SecureTLSConfig(&tls.Config{
207+
ClientAuth: tls.NoClientCert,
208+
MinVersion: minVersion,
209+
CipherSuites: cipherSuites,
210+
})
206211

207-
for _, ca := range dockerConfig.HTTP.TLS.ClientCAs {
208-
caPem, err := ioutil.ReadFile(ca)
209-
if err != nil {
210-
context.GetLogger(ctx).Fatalln(err)
211-
}
212+
if len(dockerConfig.HTTP.TLS.ClientCAs) != 0 {
213+
pool := x509.NewCertPool()
212214

213-
if ok := pool.AppendCertsFromPEM(caPem); !ok {
214-
context.GetLogger(ctx).Fatalln(fmt.Errorf("Could not add CA to pool"))
215-
}
215+
for _, ca := range dockerConfig.HTTP.TLS.ClientCAs {
216+
caPem, err := ioutil.ReadFile(ca)
217+
if err != nil {
218+
return err
216219
}
217220

218-
for _, subj := range pool.Subjects() {
219-
context.GetLogger(ctx).Debugf("CA Subject: %s", string(subj))
221+
if ok := pool.AppendCertsFromPEM(caPem); !ok {
222+
return fmt.Errorf("could not add CA to pool")
220223
}
221-
222-
tlsConf.ClientAuth = tls.RequireAndVerifyClientCert
223-
tlsConf.ClientCAs = pool
224224
}
225225

226-
context.GetLogger(ctx).Infof("listening on %v, tls", dockerConfig.HTTP.Addr)
227-
server := &http.Server{
228-
Addr: dockerConfig.HTTP.Addr,
229-
Handler: handler,
230-
TLSConfig: tlsConf,
226+
for _, subj := range pool.Subjects() {
227+
context.GetLogger(ctx).Debugf("CA Subject: %s", string(subj))
231228
}
232229

233-
if err := server.ListenAndServeTLS(dockerConfig.HTTP.TLS.Certificate, dockerConfig.HTTP.TLS.Key); err != nil {
234-
context.GetLogger(ctx).Fatalln(err)
235-
}
230+
tlsConf.ClientAuth = tls.RequireAndVerifyClientCert
231+
tlsConf.ClientCAs = pool
232+
}
233+
234+
context.GetLogger(ctx).Infof("listening on %v, tls", dockerConfig.HTTP.Addr)
235+
server := &http.Server{
236+
Addr: dockerConfig.HTTP.Addr,
237+
Handler: handler,
238+
TLSConfig: tlsConf,
236239
}
240+
return server.ListenAndServeTLS(dockerConfig.HTTP.TLS.Certificate, dockerConfig.HTTP.TLS.Key)
237241
}
238242

239243
// configureLogging prepares the context with a logger using the

‎pkg/dockerregistry/server/blobdescriptorservice_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ func TestBlobDescriptorServiceIsApplied(t *testing.T) {
8787
t.Fatalf("error parsing server url: %v", err)
8888
}
8989

90-
desc, _, err := registrytest.UploadRandomTestBlob(serverURL, nil, "user/app")
90+
desc, _, err := registrytest.UploadRandomTestBlob(ctx, serverURL, nil, "user/app")
9191
if err != nil {
9292
t.Fatal(err)
9393
}

‎pkg/dockerregistry/server/manifesthandler.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,9 @@ type ManifestHandler interface {
2424
// Manifest returns a deserialized manifest object.
2525
Manifest() distribution.Manifest
2626

27+
// Layers returns image layers and a value for the dockerLayersOrder annotation.
28+
Layers(ctx context.Context) (order string, layers []imageapiv1.ImageLayer, err error)
29+
2730
// Payload returns manifest's media type, complete payload with signatures and canonical payload without
2831
// signatures or an error if the information could not be fetched.
2932
Payload() (mediaType string, payload []byte, canonical []byte, err error)

‎pkg/dockerregistry/server/manifestschema1handler.go‎

Lines changed: 45 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@ import (
1111
"github.com/docker/distribution/manifest/schema1"
1212
"github.com/docker/distribution/reference"
1313
"github.com/docker/libtrust"
14+
15+
imageapi "github.com/openshift/origin/pkg/image/apis/image"
16+
imageapiv1 "github.com/openshift/origin/pkg/image/apis/image/v1"
1417
)
1518

1619
func unmarshalManifestSchema1(content []byte, signatures [][]byte) (distribution.Manifest, error) {
@@ -41,8 +44,9 @@ func unmarshalManifestSchema1(content []byte, signatures [][]byte) (distribution
4144
}
4245

4346
type manifestSchema1Handler struct {
44-
repo *repository
45-
manifest *schema1.SignedManifest
47+
repo *repository
48+
manifest *schema1.SignedManifest
49+
blobsCache map[digest.Digest]distribution.Descriptor
4650
}
4751

4852
var _ ManifestHandler = &manifestSchema1Handler{}
@@ -59,6 +63,44 @@ func (h *manifestSchema1Handler) Manifest() distribution.Manifest {
5963
return h.manifest
6064
}
6165

66+
func (h *manifestSchema1Handler) statBlob(ctx context.Context, dgst digest.Digest) (distribution.Descriptor, error) {
67+
desc, ok := h.blobsCache[dgst]
68+
if ok {
69+
return desc, nil
70+
}
71+
72+
desc, err := h.repo.Blobs(ctx).Stat(ctx, dgst)
73+
if err != nil {
74+
return desc, err
75+
}
76+
77+
if h.blobsCache == nil {
78+
h.blobsCache = make(map[digest.Digest]distribution.Descriptor)
79+
}
80+
h.blobsCache[dgst] = desc
81+
82+
return desc, nil
83+
}
84+
85+
func (h *manifestSchema1Handler) Layers(ctx context.Context) (string, []imageapiv1.ImageLayer, error) {
86+
layers := make([]imageapiv1.ImageLayer, len(h.manifest.FSLayers))
87+
for i, fslayer := range h.manifest.FSLayers {
88+
desc, err := h.statBlob(ctx, fslayer.BlobSum)
89+
if err != nil {
90+
return "", nil, err
91+
}
92+
93+
// In a schema1 manifest the layers are ordered from the youngest to
94+
// the oldest. But we want to have layers in different order.
95+
revidx := (len(h.manifest.FSLayers) - 1) - i // n-1, n-2, ..., 1, 0
96+
97+
layers[revidx].Name = fslayer.BlobSum.String()
98+
layers[revidx].LayerSize = desc.Size
99+
layers[revidx].MediaType = schema1.MediaTypeManifestLayer
100+
}
101+
return imageapi.DockerImageLayersOrderAscending, layers, nil
102+
}
103+
62104
func (h *manifestSchema1Handler) Payload() (mediaType string, payload []byte, canonical []byte, err error) {
63105
mt, payload, err := h.manifest.Payload()
64106
return mt, payload, h.manifest.Canonical, err
@@ -72,7 +114,6 @@ func (h *manifestSchema1Handler) Verify(ctx context.Context, skipDependencyVerif
72114
// and since we use pullthroughBlobStore all the layer existence checks will be
73115
// successful. This means that the docker client will not attempt to send them
74116
// to us as it will assume that the registry has them.
75-
repo := h.repo
76117

77118
if len(path.Join(h.repo.config.registryAddr, h.manifest.Name)) > reference.NameTotalLengthMax {
78119
errs = append(errs,
@@ -116,7 +157,7 @@ func (h *manifestSchema1Handler) Verify(ctx context.Context, skipDependencyVerif
116157
}
117158

118159
for _, fsLayer := range h.manifest.References() {
119-
_, err := repo.Blobs(ctx).Stat(ctx, fsLayer.Digest)
160+
_, err := h.statBlob(ctx, fsLayer.Digest)
120161
if err != nil {
121162
if err != distribution.ErrBlobUnknown {
122163
errs = append(errs, err)

‎pkg/dockerregistry/server/manifestschema2handler.go‎

Lines changed: 44 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,9 @@ import (
1010
"github.com/docker/distribution/context"
1111
"github.com/docker/distribution/digest"
1212
"github.com/docker/distribution/manifest/schema2"
13+
14+
imageapi "github.com/openshift/origin/pkg/image/apis/image"
15+
imageapiv1 "github.com/openshift/origin/pkg/image/apis/image/v1"
1316
)
1417

1518
var (
@@ -65,11 +68,51 @@ func (h *manifestSchema2Handler) Manifest() distribution.Manifest {
6568
return h.manifest
6669
}
6770

71+
func (h *manifestSchema2Handler) Layers(ctx context.Context) (string, []imageapiv1.ImageLayer, error) {
72+
layers := make([]imageapiv1.ImageLayer, len(h.manifest.Layers))
73+
for i, layer := range h.manifest.Layers {
74+
layers[i].Name = layer.Digest.String()
75+
layers[i].LayerSize = layer.Size
76+
layers[i].MediaType = layer.MediaType
77+
}
78+
return imageapi.DockerImageLayersOrderAscending, layers, nil
79+
}
80+
6881
func (h *manifestSchema2Handler) Payload() (mediaType string, payload []byte, canonical []byte, err error) {
6982
mt, p, err := h.manifest.Payload()
7083
return mt, p, p, err
7184
}
7285

86+
func (h *manifestSchema2Handler) verifyLayer(ctx context.Context, fsLayer distribution.Descriptor) error {
87+
if fsLayer.MediaType == schema2.MediaTypeForeignLayer {
88+
// Clients download this layer from an external URL, so do not check for
89+
// its presense.
90+
if len(fsLayer.URLs) == 0 {
91+
return errMissingURL
92+
}
93+
return nil
94+
}
95+
96+
if len(fsLayer.URLs) != 0 {
97+
return errUnexpectedURL
98+
}
99+
100+
desc, err := h.repo.Blobs(ctx).Stat(ctx, fsLayer.Digest)
101+
if err != nil {
102+
return err
103+
}
104+
105+
if fsLayer.Size != desc.Size {
106+
return ErrManifestBlobBadSize{
107+
Digest: fsLayer.Digest,
108+
ActualSize: desc.Size,
109+
SizeInManifest: fsLayer.Size,
110+
}
111+
}
112+
113+
return nil
114+
}
115+
73116
func (h *manifestSchema2Handler) Verify(ctx context.Context, skipDependencyVerification bool) error {
74117
var errs distribution.ErrManifestVerification
75118

@@ -82,35 +125,9 @@ func (h *manifestSchema2Handler) Verify(ctx context.Context, skipDependencyVerif
82125
// and since we use pullthroughBlobStore all the layer existence checks will be
83126
// successful. This means that the docker client will not attempt to send them
84127
// to us as it will assume that the registry has them.
85-
repo := h.repo
86-
87-
target := h.manifest.Target()
88-
_, err := repo.Blobs(ctx).Stat(ctx, target.Digest)
89-
if err != nil {
90-
if err != distribution.ErrBlobUnknown {
91-
errs = append(errs, err)
92-
}
93-
94-
// On error here, we always append unknown blob errors.
95-
errs = append(errs, distribution.ErrManifestBlobUnknown{Digest: target.Digest})
96-
}
97128

98129
for _, fsLayer := range h.manifest.References() {
99-
var err error
100-
if fsLayer.MediaType != schema2.MediaTypeForeignLayer {
101-
if len(fsLayer.URLs) == 0 {
102-
_, err = repo.Blobs(ctx).Stat(ctx, fsLayer.Digest)
103-
} else {
104-
err = errUnexpectedURL
105-
}
106-
} else {
107-
// Clients download this layer from an external URL, so do not check for
108-
// its presense.
109-
if len(fsLayer.URLs) == 0 {
110-
err = errMissingURL
111-
}
112-
}
113-
if err != nil {
130+
if err := h.verifyLayer(ctx, fsLayer); err != nil {
114131
if err != distribution.ErrBlobUnknown {
115132
errs = append(errs, err)
116133
continue

‎pkg/dockerregistry/server/manifestservice.go‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,20 @@ import (
2121
quotautil "github.com/openshift/origin/pkg/quota/util"
2222
)
2323

24+
// ErrManifestBlobBadSize is returned when the blob size in a manifest does
25+
// not match the actual size. The docker/distribution does not check this and
26+
// therefore does not provide an error for this.
27+
type ErrManifestBlobBadSize struct {
28+
Digest digest.Digest
29+
ActualSize int64
30+
SizeInManifest int64
31+
}
32+
33+
func (err ErrManifestBlobBadSize) Error() string {
34+
return fmt.Sprintf("the blob %s has the size (%d) different from the one specified in the manifest (%d)",
35+
err.Digest, err.ActualSize, err.SizeInManifest)
36+
}
37+
2438
var _ distribution.ManifestService = &manifestService{}
2539

2640
type manifestService struct {
@@ -104,6 +118,7 @@ func (m *manifestService) Put(ctx context.Context, manifest distribution.Manifes
104118
if err != nil {
105119
return "", regapi.ErrorCodeManifestInvalid.WithDetail(err)
106120
}
121+
107122
mediaType, payload, _, err := mh.Payload()
108123
if err != nil {
109124
return "", regapi.ErrorCodeManifestInvalid.WithDetail(err)
@@ -134,6 +149,11 @@ func (m *manifestService) Put(ctx context.Context, manifest distribution.Manifes
134149
return "", err
135150
}
136151

152+
layerOrder, layers, err := mh.Layers(ctx)
153+
if err != nil {
154+
return "", err
155+
}
156+
137157
// Upload to openshift
138158
ism := imageapiv1.ImageStreamMapping{
139159
ObjectMeta: metav1.ObjectMeta{
@@ -146,12 +166,14 @@ func (m *manifestService) Put(ctx context.Context, manifest distribution.Manifes
146166
Annotations: map[string]string{
147167
imageapi.ManagedByOpenShiftAnnotation: "true",
148168
imageapi.ImageManifestBlobStoredAnnotation: "true",
169+
imageapi.DockerImageLayersOrderAnnotation: layerOrder,
149170
},
150171
},
151172
DockerImageReference: fmt.Sprintf("%s/%s/%s@%s", m.repo.config.registryAddr, m.repo.namespace, m.repo.name, dgst.String()),
152173
DockerImageManifest: string(payload),
153174
DockerImageManifestMediaType: mediaType,
154175
DockerImageConfig: string(config),
176+
DockerImageLayers: layers,
155177
},
156178
}
157179

0 commit comments

Comments
 (0)