Browse code

Correct parent chain in v2 push when v1Compatibility files on the disk are inconsistent

This fixes an issue where two images with the same filesystem contents
and configuration but different remote IDs could share a v1Compatibility
file, resulting in corrupted manifests.

Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>
(cherry picked from commit 0ab6b1d9221f7a2a65c6565fed8f3d6f29fcec2d)

Conflicts:
integration-cli/docker_cli_push_test.go

Aaron Lehmann authored on 2015/11/18 03:39:51
Showing 2 changed files
... ...
@@ -3,6 +3,8 @@ package graph
3 3
 import (
4 4
 	"bufio"
5 5
 	"compress/gzip"
6
+	"encoding/json"
7
+	"errors"
6 8
 	"fmt"
7 9
 	"io"
8 10
 	"io/ioutil"
... ...
@@ -199,6 +201,11 @@ func (p *v2Pusher) pushV2Tag(tag string) error {
199 199
 		p.layersPushed[dgst] = true
200 200
 	}
201 201
 
202
+	// Fix parent chain if necessary
203
+	if err = fixHistory(m); err != nil {
204
+		return err
205
+	}
206
+
202 207
 	logrus.Infof("Signed manifest for %s:%s using daemon's key: %s", p.repo.Name(), tag, p.trustKey.KeyID())
203 208
 	signed, err := manifest.Sign(m, p.trustKey)
204 209
 	if err != nil {
... ...
@@ -220,6 +227,90 @@ func (p *v2Pusher) pushV2Tag(tag string) error {
220 220
 	return manSvc.Put(signed)
221 221
 }
222 222
 
223
+// fixHistory makes sure that the manifest has parent IDs that are consistent
224
+// with its image IDs. Because local image IDs are generated from the
225
+// configuration and filesystem contents, but IDs in the manifest are preserved
226
+// from the original pull, it's possible to have inconsistencies where parent
227
+// IDs don't match up with the other IDs in the manifest. This happens in the
228
+// case where an engine pulls images where are identical except the IDs from the
229
+// manifest - the local ID will be the same, and one of the v1Compatibility
230
+// files gets discarded.
231
+func fixHistory(m *manifest.Manifest) error {
232
+	var lastID string
233
+
234
+	for i := len(m.History) - 1; i >= 0; i-- {
235
+		var historyEntry map[string]*json.RawMessage
236
+		if err := json.Unmarshal([]byte(m.History[i].V1Compatibility), &historyEntry); err != nil {
237
+			return err
238
+		}
239
+
240
+		idJSON, present := historyEntry["id"]
241
+		if !present || idJSON == nil {
242
+			return errors.New("missing id key in v1compatibility file")
243
+		}
244
+		var id string
245
+		if err := json.Unmarshal(*idJSON, &id); err != nil {
246
+			return err
247
+		}
248
+
249
+		parentJSON, present := historyEntry["parent"]
250
+
251
+		if i == len(m.History)-1 {
252
+			// The base layer must not reference a parent layer,
253
+			// otherwise the manifest is incomplete. There is an
254
+			// exception for Windows to handle base layers.
255
+			if present && parentJSON != nil {
256
+				var parent string
257
+				if err := json.Unmarshal(*parentJSON, &parent); err != nil {
258
+					return err
259
+				}
260
+				if parent != "" {
261
+					logrus.Debugf("parent id mismatch detected; fixing. parent reference: %s", parent)
262
+					delete(historyEntry, "parent")
263
+					fixedHistory, err := json.Marshal(historyEntry)
264
+					if err != nil {
265
+						return err
266
+					}
267
+					m.History[i].V1Compatibility = string(fixedHistory)
268
+				}
269
+			}
270
+		} else {
271
+			// For all other layers, the parent ID should equal the
272
+			// ID of the next item in the history list. If it
273
+			// doesn't, fix it up (but preserve all other fields,
274
+			// possibly including fields that aren't known to this
275
+			// engine version).
276
+			if !present || parentJSON == nil {
277
+				return errors.New("missing parent key in v1compatibility file")
278
+			}
279
+			var parent string
280
+			if err := json.Unmarshal(*parentJSON, &parent); err != nil {
281
+				return err
282
+			}
283
+			if parent != lastID {
284
+				logrus.Debugf("parent id mismatch detected; fixing. parent reference: %s actual id: %s", parent, id)
285
+				historyEntry["parent"] = rawJSON(lastID)
286
+				fixedHistory, err := json.Marshal(historyEntry)
287
+				if err != nil {
288
+					return err
289
+				}
290
+				m.History[i].V1Compatibility = string(fixedHistory)
291
+			}
292
+		}
293
+		lastID = id
294
+	}
295
+
296
+	return nil
297
+}
298
+
299
+func rawJSON(value interface{}) *json.RawMessage {
300
+	jsonval, err := json.Marshal(value)
301
+	if err != nil {
302
+		return nil
303
+	}
304
+	return (*json.RawMessage)(&jsonval)
305
+}
306
+
223 307
 func (p *v2Pusher) pushV2Image(bs distribution.BlobService, img *image.Image) (digest.Digest, error) {
224 308
 	out := p.config.OutStream
225 309
 
... ...
@@ -2,13 +2,16 @@ package main
2 2
 
3 3
 import (
4 4
 	"archive/tar"
5
+	"encoding/json"
5 6
 	"fmt"
6 7
 	"io/ioutil"
7 8
 	"os"
8 9
 	"os/exec"
10
+	"path/filepath"
9 11
 	"strings"
10 12
 	"time"
11 13
 
14
+	"github.com/docker/docker/image"
12 15
 	"github.com/go-check/check"
13 16
 )
14 17
 
... ...
@@ -97,6 +100,46 @@ func (s *DockerRegistrySuite) TestPushMultipleTags(c *check.C) {
97 97
 	}
98 98
 }
99 99
 
100
+// TestPushBadParentChain tries to push an image with a corrupted parent chain
101
+// in the v1compatibility files, and makes sure the push process fixes it.
102
+func (s *DockerRegistrySuite) TestPushBadParentChain(c *check.C) {
103
+	repoName := fmt.Sprintf("%v/dockercli/badparent", privateRegistryURL)
104
+
105
+	id, err := buildImage(repoName, `
106
+	    FROM busybox
107
+	    CMD echo "adding another layer"
108
+	    `, true)
109
+	if err != nil {
110
+		c.Fatal(err)
111
+	}
112
+
113
+	// Push to create v1compatibility file
114
+	dockerCmd(c, "push", repoName)
115
+
116
+	// Corrupt the parent in the v1compatibility file from the top layer
117
+	filename := filepath.Join(dockerBasePath, "graph", id, "v1Compatibility")
118
+
119
+	jsonBytes, err := ioutil.ReadFile(filename)
120
+	c.Assert(err, check.IsNil, check.Commentf("Could not read v1Compatibility file: %s", err))
121
+
122
+	var img image.Image
123
+	err = json.Unmarshal(jsonBytes, &img)
124
+	c.Assert(err, check.IsNil, check.Commentf("Could not unmarshal json: %s", err))
125
+
126
+	img.Parent = "1234123412341234123412341234123412341234123412341234123412341234"
127
+
128
+	jsonBytes, err = json.Marshal(&img)
129
+	c.Assert(err, check.IsNil, check.Commentf("Could not marshal json: %s", err))
130
+
131
+	err = ioutil.WriteFile(filename, jsonBytes, 0600)
132
+	c.Assert(err, check.IsNil, check.Commentf("Could not write v1Compatibility file: %s", err))
133
+
134
+	dockerCmd(c, "push", repoName)
135
+
136
+	// pull should succeed
137
+	dockerCmd(c, "pull", repoName)
138
+}
139
+
100 140
 func (s *DockerRegistrySuite) TestPushEmptyLayer(c *check.C) {
101 141
 	repoName := fmt.Sprintf("%v/dockercli/emptylayer", privateRegistryURL)
102 142
 	emptyTarball, err := ioutil.TempFile("", "empty_tarball")