From 65370be888d940899593a001024f53d6b83b4bb0 Mon Sep 17 00:00:00 2001 From: Aaron Lehmann Date: Mon, 21 Dec 2015 15:02:44 -0800 Subject: [PATCH] Send push information to trust code out-of-band The trust code used to parse the console output of `docker push` to extract the digest, tag, and size information and determine what to sign. This is fragile and might give an attacker control over what gets signed if the attacker can find a way to influence what gets printed as part of the push output. This commit sends the push metadata out-of-band. It introduces an `Aux` field in JSONMessage that can carry application-specific data alongside progress updates. Instead of parsing formatted output, the client looks in this field to get the digest, size, and tag from the push. Signed-off-by: Aaron Lehmann --- api/client/build.go | 2 +- api/client/create.go | 2 +- api/client/import.go | 2 +- api/client/load.go | 2 +- api/client/pull.go | 2 +- api/client/push.go | 25 +++---- api/client/trust.go | 78 ++++++--------------- distribution/push_v2.go | 14 +++- pkg/jsonmessage/jsonmessage.go | 11 ++- pkg/jsonmessage/jsonmessage_test.go | 6 +- pkg/progress/progress.go | 10 +++ pkg/streamformatter/streamformatter.go | 14 +++- pkg/streamformatter/streamformatter_test.go | 2 +- 13 files changed, 88 insertions(+), 82 deletions(-) diff --git a/api/client/build.go b/api/client/build.go index 43f41194d1..2a1712265a 100644 --- a/api/client/build.go +++ b/api/client/build.go @@ -255,7 +255,7 @@ func (cli *DockerCli) CmdBuild(args ...string) error { return err } - err = jsonmessage.DisplayJSONMessagesStream(response.Body, buildBuff, cli.outFd, cli.isTerminalOut) + err = jsonmessage.DisplayJSONMessagesStream(response.Body, buildBuff, cli.outFd, cli.isTerminalOut, nil) if err != nil { if jerr, ok := err.(*jsonmessage.JSONError); ok { // If no error code is set, default to 1 diff --git a/api/client/create.go b/api/client/create.go index 5f4f17c313..9165e2e1ed 100644 --- a/api/client/create.go +++ b/api/client/create.go @@ -57,7 +57,7 @@ func (cli *DockerCli) pullImageCustomOut(image string, out io.Writer) error { } defer responseBody.Close() - return jsonmessage.DisplayJSONMessagesStream(responseBody, out, cli.outFd, cli.isTerminalOut) + return jsonmessage.DisplayJSONMessagesStream(responseBody, out, cli.outFd, cli.isTerminalOut, nil) } type cidFile struct { diff --git a/api/client/import.go b/api/client/import.go index ece821aef6..5b8122061c 100644 --- a/api/client/import.go +++ b/api/client/import.go @@ -76,5 +76,5 @@ func (cli *DockerCli) CmdImport(args ...string) error { } defer responseBody.Close() - return jsonmessage.DisplayJSONMessagesStream(responseBody, cli.out, cli.outFd, cli.isTerminalOut) + return jsonmessage.DisplayJSONMessagesStream(responseBody, cli.out, cli.outFd, cli.isTerminalOut, nil) } diff --git a/api/client/load.go b/api/client/load.go index 745c013c01..82cf34d8f2 100644 --- a/api/client/load.go +++ b/api/client/load.go @@ -37,7 +37,7 @@ func (cli *DockerCli) CmdLoad(args ...string) error { defer response.Body.Close() if response.JSON { - return jsonmessage.DisplayJSONMessagesStream(response.Body, cli.out, cli.outFd, cli.isTerminalOut) + return jsonmessage.DisplayJSONMessagesStream(response.Body, cli.out, cli.outFd, cli.isTerminalOut, nil) } _, err = io.Copy(cli.out, response.Body) diff --git a/api/client/pull.go b/api/client/pull.go index 5566d7976f..19220b96c0 100644 --- a/api/client/pull.go +++ b/api/client/pull.go @@ -83,5 +83,5 @@ func (cli *DockerCli) imagePullPrivileged(authConfig types.AuthConfig, imageID, } defer responseBody.Close() - return jsonmessage.DisplayJSONMessagesStream(responseBody, cli.out, cli.outFd, cli.isTerminalOut) + return jsonmessage.DisplayJSONMessagesStream(responseBody, cli.out, cli.outFd, cli.isTerminalOut, nil) } diff --git a/api/client/push.go b/api/client/push.go index bbfbaf5fea..9e4972c308 100644 --- a/api/client/push.go +++ b/api/client/push.go @@ -49,25 +49,26 @@ func (cli *DockerCli) CmdPush(args ...string) error { return cli.trustedPush(repoInfo, tag, authConfig, requestPrivilege) } - return cli.imagePushPrivileged(authConfig, ref.Name(), tag, cli.out, requestPrivilege) -} - -func (cli *DockerCli) imagePushPrivileged(authConfig types.AuthConfig, imageID, tag string, outputStream io.Writer, requestPrivilege client.RequestPrivilegeFunc) error { - encodedAuth, err := encodeAuthToBase64(authConfig) + responseBody, err := cli.imagePushPrivileged(authConfig, ref.Name(), tag, requestPrivilege) if err != nil { return err } + + defer responseBody.Close() + + return jsonmessage.DisplayJSONMessagesStream(responseBody, cli.out, cli.outFd, cli.isTerminalOut, nil) +} + +func (cli *DockerCli) imagePushPrivileged(authConfig types.AuthConfig, imageID, tag string, requestPrivilege client.RequestPrivilegeFunc) (io.ReadCloser, error) { + encodedAuth, err := encodeAuthToBase64(authConfig) + if err != nil { + return nil, err + } options := types.ImagePushOptions{ ImageID: imageID, Tag: tag, RegistryAuth: encodedAuth, } - responseBody, err := cli.client.ImagePush(options, requestPrivilege) - if err != nil { - return err - } - defer responseBody.Close() - - return jsonmessage.DisplayJSONMessagesStream(responseBody, outputStream, cli.outFd, cli.isTerminalOut) + return cli.client.ImagePush(options, requestPrivilege) } diff --git a/api/client/trust.go b/api/client/trust.go index 256ea5b913..f347689972 100644 --- a/api/client/trust.go +++ b/api/client/trust.go @@ -1,19 +1,16 @@ package client import ( - "bufio" "encoding/hex" "encoding/json" "errors" "fmt" - "io" "net" "net/http" "net/url" "os" "path" "path/filepath" - "regexp" "sort" "strconv" "time" @@ -23,8 +20,8 @@ import ( "github.com/docker/distribution/registry/client/auth" "github.com/docker/distribution/registry/client/transport" "github.com/docker/docker/cliconfig" - "github.com/docker/docker/pkg/ansiescape" - "github.com/docker/docker/pkg/ioutils" + "github.com/docker/docker/distribution" + "github.com/docker/docker/pkg/jsonmessage" flag "github.com/docker/docker/pkg/mflag" "github.com/docker/docker/reference" "github.com/docker/docker/registry" @@ -64,8 +61,6 @@ func isTrusted() bool { return !untrusted } -var targetRegexp = regexp.MustCompile(`([\S]+): digest: ([\S]+) size: ([\d]+)`) - type target struct { reference registry.Reference digest digest.Digest @@ -366,60 +361,31 @@ func (cli *DockerCli) trustedPull(repoInfo *registry.RepositoryInfo, ref registr return nil } -func targetStream(in io.Writer) (io.WriteCloser, <-chan []target) { - r, w := io.Pipe() - out := io.MultiWriter(in, w) - targetChan := make(chan []target) - - go func() { - targets := []target{} - scanner := bufio.NewScanner(r) - scanner.Split(ansiescape.ScanANSILines) - for scanner.Scan() { - line := scanner.Bytes() - if matches := targetRegexp.FindSubmatch(line); len(matches) == 4 { - dgst, err := digest.ParseDigest(string(matches[2])) - if err != nil { - // Line does match what is expected, continue looking for valid lines - logrus.Debugf("Bad digest value %q in matched line, ignoring\n", string(matches[2])) - continue - } - s, err := strconv.ParseInt(string(matches[3]), 10, 64) - if err != nil { - // Line does match what is expected, continue looking for valid lines - logrus.Debugf("Bad size value %q in matched line, ignoring\n", string(matches[3])) - continue - } - - targets = append(targets, target{ - reference: registry.ParseReference(string(matches[1])), - digest: dgst, - size: s, - }) - } - } - targetChan <- targets - }() - - return ioutils.NewWriteCloserWrapper(out, w.Close), targetChan -} - func (cli *DockerCli) trustedPush(repoInfo *registry.RepositoryInfo, tag string, authConfig types.AuthConfig, requestPrivilege apiclient.RequestPrivilegeFunc) error { - streamOut, targetChan := targetStream(cli.out) - - reqError := cli.imagePushPrivileged(authConfig, repoInfo.Name(), tag, streamOut, requestPrivilege) - - // Close stream channel to finish target parsing - if err := streamOut.Close(); err != nil { + responseBody, err := cli.imagePushPrivileged(authConfig, repoInfo.Name(), tag, requestPrivilege) + if err != nil { return err } - // Check error from request - if reqError != nil { - return reqError + + defer responseBody.Close() + + targets := []target{} + handleTarget := func(aux *json.RawMessage) { + var pushResult distribution.PushResult + err := json.Unmarshal(*aux, &pushResult) + if err == nil && pushResult.Tag != "" && pushResult.Digest.Validate() == nil { + targets = append(targets, target{ + reference: registry.ParseReference(pushResult.Tag), + digest: pushResult.Digest, + size: int64(pushResult.Size), + }) + } } - // Get target results - targets := <-targetChan + err = jsonmessage.DisplayJSONMessagesStream(responseBody, cli.out, cli.outFd, cli.isTerminalOut, handleTarget) + if err != nil { + return err + } if tag == "" { fmt.Fprintf(cli.out, "No tag specified, skipping trust metadata push\n") diff --git a/distribution/push_v2.go b/distribution/push_v2.go index 2ed6456603..bf68c94c48 100644 --- a/distribution/push_v2.go +++ b/distribution/push_v2.go @@ -26,6 +26,15 @@ import ( "golang.org/x/net/context" ) +// PushResult contains the tag, manifest digest, and manifest size from the +// push. It's used to signal this information to the trust code in the client +// so it can sign the manifest if necessary. +type PushResult struct { + Tag string + Digest digest.Digest + Size int +} + type v2Pusher struct { blobSumService *metadata.BlobSumService ref reference.Named @@ -174,9 +183,10 @@ func (p *v2Pusher) pushV2Tag(ctx context.Context, association reference.Associat } if manifestDigest != "" { if tagged, isTagged := ref.(reference.NamedTagged); isTagged { - // NOTE: do not change this format without first changing the trust client - // code. This information is used to determine what was pushed and should be signed. progress.Messagef(p.config.ProgressOutput, "", "%s: digest: %s size: %d", tagged.Tag(), manifestDigest, manifestSize) + // Signal digest to the trust client so it can sign the + // push, if appropriate. + progress.Aux(p.config.ProgressOutput, PushResult{Tag: tagged.Tag(), Digest: manifestDigest, Size: manifestSize}) } } diff --git a/pkg/jsonmessage/jsonmessage.go b/pkg/jsonmessage/jsonmessage.go index 64472324fc..65cccbce17 100644 --- a/pkg/jsonmessage/jsonmessage.go +++ b/pkg/jsonmessage/jsonmessage.go @@ -102,6 +102,8 @@ type JSONMessage struct { TimeNano int64 `json:"timeNano,omitempty"` Error *JSONError `json:"errorDetail,omitempty"` ErrorMessage string `json:"error,omitempty"` //deprecated + // Aux contains out-of-band data, such as digests for push signing. + Aux *json.RawMessage `json:"aux,omitempty"` } // Display displays the JSONMessage to `out`. `isTerminal` describes if `out` @@ -148,7 +150,7 @@ func (jm *JSONMessage) Display(out io.Writer, isTerminal bool) error { // DisplayJSONMessagesStream displays a json message stream from `in` to `out`, `isTerminal` // describes if `out` is a terminal. If this is the case, it will print `\n` at the end of // each line and move the cursor while displaying. -func DisplayJSONMessagesStream(in io.Reader, out io.Writer, terminalFd uintptr, isTerminal bool) error { +func DisplayJSONMessagesStream(in io.Reader, out io.Writer, terminalFd uintptr, isTerminal bool, auxCallback func(*json.RawMessage)) error { var ( dec = json.NewDecoder(in) ids = make(map[string]int) @@ -163,6 +165,13 @@ func DisplayJSONMessagesStream(in io.Reader, out io.Writer, terminalFd uintptr, return err } + if jm.Aux != nil { + if auxCallback != nil { + auxCallback(jm.Aux) + } + continue + } + if jm.Progress != nil { jm.Progress.terminalFd = terminalFd } diff --git a/pkg/jsonmessage/jsonmessage_test.go b/pkg/jsonmessage/jsonmessage_test.go index b81b656994..558effcc3b 100644 --- a/pkg/jsonmessage/jsonmessage_test.go +++ b/pkg/jsonmessage/jsonmessage_test.go @@ -168,7 +168,7 @@ func TestDisplayJSONMessagesStreamInvalidJSON(t *testing.T) { reader := strings.NewReader("This is not a 'valid' JSON []") inFd, _ = term.GetFdInfo(reader) - if err := DisplayJSONMessagesStream(reader, data, inFd, false); err == nil && err.Error()[:17] != "invalid character" { + if err := DisplayJSONMessagesStream(reader, data, inFd, false, nil); err == nil && err.Error()[:17] != "invalid character" { t.Fatalf("Should have thrown an error (invalid character in ..), got [%v]", err) } } @@ -210,7 +210,7 @@ func TestDisplayJSONMessagesStream(t *testing.T) { inFd, _ = term.GetFdInfo(reader) // Without terminal - if err := DisplayJSONMessagesStream(reader, data, inFd, false); err != nil { + if err := DisplayJSONMessagesStream(reader, data, inFd, false, nil); err != nil { t.Fatal(err) } if data.String() != expectedMessages[0] { @@ -220,7 +220,7 @@ func TestDisplayJSONMessagesStream(t *testing.T) { // With terminal data = bytes.NewBuffer([]byte{}) reader = strings.NewReader(jsonMessage) - if err := DisplayJSONMessagesStream(reader, data, inFd, true); err != nil { + if err := DisplayJSONMessagesStream(reader, data, inFd, true, nil); err != nil { t.Fatal(err) } if data.String() != expectedMessages[1] { diff --git a/pkg/progress/progress.go b/pkg/progress/progress.go index 1f3b34a832..61315cb82c 100644 --- a/pkg/progress/progress.go +++ b/pkg/progress/progress.go @@ -16,6 +16,10 @@ type Progress struct { Current int64 Total int64 + // Aux contains extra information not presented to the user, such as + // digests for push signing. + Aux interface{} + LastUpdate bool } @@ -61,3 +65,9 @@ func Message(out Output, id, message string) { func Messagef(out Output, id, format string, a ...interface{}) { Message(out, id, fmt.Sprintf(format, a...)) } + +// Aux sends auxiliary information over a progress interface, which will not be +// formatted for the UI. This is used for things such as push signing. +func Aux(out Output, a interface{}) { + out.WriteProgress(Progress{Aux: a}) +} diff --git a/pkg/streamformatter/streamformatter.go b/pkg/streamformatter/streamformatter.go index d6700184c1..ce6ea79dee 100644 --- a/pkg/streamformatter/streamformatter.go +++ b/pkg/streamformatter/streamformatter.go @@ -70,16 +70,26 @@ func (sf *StreamFormatter) FormatError(err error) []byte { } // FormatProgress formats the progress information for a specified action. -func (sf *StreamFormatter) FormatProgress(id, action string, progress *jsonmessage.JSONProgress) []byte { +func (sf *StreamFormatter) FormatProgress(id, action string, progress *jsonmessage.JSONProgress, aux interface{}) []byte { if progress == nil { progress = &jsonmessage.JSONProgress{} } if sf.json { + var auxJSON *json.RawMessage + if aux != nil { + auxJSONBytes, err := json.Marshal(aux) + if err != nil { + return nil + } + auxJSON = new(json.RawMessage) + *auxJSON = auxJSONBytes + } b, err := json.Marshal(&jsonmessage.JSONMessage{ Status: action, ProgressMessage: progress.String(), Progress: progress, ID: id, + Aux: auxJSON, }) if err != nil { return nil @@ -116,7 +126,7 @@ func (out *progressOutput) WriteProgress(prog progress.Progress) error { formatted = out.sf.FormatStatus(prog.ID, prog.Message) } else { jsonProgress := jsonmessage.JSONProgress{Current: prog.Current, Total: prog.Total} - formatted = out.sf.FormatProgress(prog.ID, prog.Action, &jsonProgress) + formatted = out.sf.FormatProgress(prog.ID, prog.Action, &jsonProgress, prog.Aux) } _, err := out.out.Write(formatted) if err != nil { diff --git a/pkg/streamformatter/streamformatter_test.go b/pkg/streamformatter/streamformatter_test.go index acf81bef68..438758d5bc 100644 --- a/pkg/streamformatter/streamformatter_test.go +++ b/pkg/streamformatter/streamformatter_test.go @@ -73,7 +73,7 @@ func TestJSONFormatProgress(t *testing.T) { Total: 30, Start: 1, } - res := sf.FormatProgress("id", "action", progress) + res := sf.FormatProgress("id", "action", progress, nil) msg := &jsonmessage.JSONMessage{} if err := json.Unmarshal(res, msg); err != nil { t.Fatal(err)