From abfdce281a68e0020d980d8e9b8652829aa6ef3f Mon Sep 17 00:00:00 2001 From: Jonathan Boulle Date: Mon, 22 Dec 2014 15:15:30 -0800 Subject: [PATCH] *: switch cas using first half of sha512 This changes the CAS to use only the first half of a SHA512 when creating the keys to store images on disk. Similarly, rkt stage1 uses only the first half of the sum when creating the directories for each app image. This is exposed slightly to stage0/run because it attempts to verify the hash of the image. This should be revisited+cleaned up. --- cas/cas.go | 92 +++++++++++++++++++++++++++++----------------- cas/cas_test.go | 21 ++++++----- cas/remote.go | 2 +- cas/z_last_test.go | 2 +- path/path.go | 4 +- rkt/run.go | 3 +- stage0/run.go | 17 +++++---- 7 files changed, 86 insertions(+), 55 deletions(-) diff --git a/cas/cas.go b/cas/cas.go index 76ca7dc..1bb6d9e 100644 --- a/cas/cas.go +++ b/cas/cas.go @@ -5,6 +5,7 @@ import ( "bytes" "crypto/sha512" "fmt" + "hash" "io" "io/ioutil" "os" @@ -12,6 +13,7 @@ import ( "strings" "github.com/appc/spec/aci" + "github.com/coreos/rocket/Godeps/_workspace/src/github.com/peterbourgon/diskv" ) @@ -23,17 +25,20 @@ const ( defaultPathPerm os.FileMode = 0777 - hashPrefix = "sha512-" - lenHashPrefix = len(hashPrefix) - lenHash = 128 + // To ameliorate excessively long paths, keys for the (blob)store use + // only the first half of a sha512 rather than the entire sum + hashPrefix = "sha512-" + lenHash = sha512.Size // raw byte size + lenHashKey = (lenHash / 2) * 2 // half length, in hex characters + lenKey = len(hashPrefix) + lenHashKey ) var otmap = [...]string{ "blob", - "remote", - "tmp", + "remote", // remote is a temporary secondary index } +// Store encapsulates a content-addressable-storage for storing ACIs on disk. type Store struct { base string stores []*diskv.Diskv @@ -64,29 +69,35 @@ func (ds Store) tmpFile() (*os.File, error) { return ioutil.TempFile(dir, "") } -// ResolveKey resolves a key of prefixed format sha512-0c45e8c0ab2 to a full key -// by using the cas store for resolution. -// -// If the key is already of proper length, just returns the key. -func (ds Store) ResolveKey(keyPrefix string) (string, error) { - if strings.HasPrefix(keyPrefix, hashPrefix) && len(keyPrefix) == lenHash+lenHashPrefix { - return keyPrefix, nil +// ResolveKey resolves a partial key (of format `sha512-0c45e8c0ab2`) to a full +// key by considering the key a prefix and using the store for resolution. +// If the key is already of the full key length, it returns the key unaltered. +// If the key is longer than the full key length, it is first truncated. +func (ds Store) ResolveKey(key string) (string, error) { + if len(key) > lenKey { + key = key[:lenKey] + } + if strings.HasPrefix(key, hashPrefix) && len(key) == lenKey { + return key, nil } cancel := make(chan struct{}) - var key string + var k string keyCount := 0 - for key = range ds.stores[blobType].KeysPrefix(keyPrefix, cancel) { + for k = range ds.stores[blobType].KeysPrefix(key, cancel) { keyCount++ if keyCount > 1 { close(cancel) break } } - if keyCount != 1 { - return "", fmt.Errorf("ambiguous key: %q", keyPrefix) + if keyCount == 0 { + return "", fmt.Errorf("no keys found") } - return key, nil + if keyCount != 1 { + return "", fmt.Errorf("ambiguous key: %q", key) + } + return k, nil } func (ds Store) ReadStream(key string) (io.ReadCloser, error) { @@ -97,43 +108,47 @@ func (ds Store) WriteStream(key string, r io.Reader) error { return ds.stores[blobType].WriteStream(key, r, true) } -func (ds Store) WriteACI(tmpKey string, orig io.Reader) (string, error) { +// WriteACI takes an ACI encapsulated in an io.Reader, decompresses it if +// necessary, and then stores it in the store under a key based on the image ID +// (i.e. the hash of the uncompressed ACI) +func (ds Store) WriteACI(r io.Reader) (string, error) { // Peek at the first 512 bytes of the reader to detect filetype - br := bufio.NewReaderSize(orig, 512) + br := bufio.NewReaderSize(r, 512) hd, err := br.Peek(512) switch err { case nil: case io.EOF: // We may have still peeked enough to guess some types, so fall through default: - return "", err + return "", fmt.Errorf("error reading image header: %v", err) } typ, err := aci.DetectFileType(bytes.NewBuffer(hd)) if err != nil { - return "", err + return "", fmt.Errorf("error detecting image type: %v", err) } dr, err := decompress(br, typ) if err != nil { - return "", err + return "", fmt.Errorf("error decompressing image: %v", err) } - // Write the uncompressed image (tar) to a temporary file on disk, and + // Write the decompressed image (tar) to a temporary file on disk, and // tee so we can generate the hash - hash := sha512.New() - tr := io.TeeReader(dr, hash) + h := sha512.New() + tr := io.TeeReader(dr, h) fh, err := ds.tmpFile() if err != nil { - return "", err + return "", fmt.Errorf("error creating image: %v", err) } if _, err := io.Copy(fh, tr); err != nil { - return "", err + return "", fmt.Errorf("error copying image: %v", err) + } + if err := fh.Close(); err != nil { + return "", fmt.Errorf("error closing image: %v", err) } - fh.Close() - // Import the decompressed tar to the store using the hash as the key - key := fmt.Sprintf("sha512-%x", hash.Sum(nil)) - err = ds.stores[blobType].Import(fh.Name(), key, true) - if err != nil { - return "", err + // Import the uncompressed image into the store at the real key + key := HashToKey(h) + if err = ds.stores[blobType].Import(fh.Name(), key, true); err != nil { + return "", fmt.Errorf("error importing image: %v", err) } return key, nil @@ -182,3 +197,14 @@ func (ds Store) Dump(hex bool) { fmt.Printf("%d total keys\n", keyCount) } } + +// HashToKey takes a hash.Hash (which currently _MUST_ represent a full SHA512), +// calculates its sum, and returns a string which should be used as the key to +// store the data matching the hash. +func HashToKey(h hash.Hash) string { + s := h.Sum(nil) + if len(s) != lenHash { + panic(fmt.Sprintf("bad hash passed to hashToKey: %s", s)) + } + return fmt.Sprintf("%s%x", hashPrefix, s)[0:lenKey] +} diff --git a/cas/cas_test.go b/cas/cas_test.go index 526ad81..287097a 100644 --- a/cas/cas_test.go +++ b/cas/cas_test.go @@ -93,18 +93,21 @@ func TestResolveKey(t *testing.T) { } } - // Full key already - should just be returned untouched and without checking the store - fk := "sha512-67147019a5b56f5e2ee01e989a8aa4787f56b8445960be2d8678391cf111009bc0780f31001fd181a2b61507547aee4caa44cda4b8bdb238d0e4ba830069ed2c" - k, err := ds.ResolveKey(fk) - if k != fk { - t.Errorf("expected ResolveKey to return unaltered key, but got %q", k) - } - if err != nil { - t.Errorf("expected err=nil, got %v", err) + // Full key already - should return short version of the full key + fkl := "sha512-67147019a5b56f5e2ee01e989a8aa4787f56b8445960be2d8678391cf111009bc0780f31001fd181a2b61507547aee4caa44cda4b8bdb238d0e4ba830069ed2c" + fks := "sha512-67147019a5b56f5e2ee01e989a8aa4787f56b8445960be2d8678391cf111009b" + for _, k := range []string{fkl, fks} { + key, err := ds.ResolveKey(k) + if key != fks { + t.Errorf("expected ResolveKey to return unaltered short key, but got %q", key) + } + if err != nil { + t.Errorf("expected err=nil, got %v", err) + } } // Unambiguous prefix match - k, err = ds.ResolveKey("sha512-123") + k, err := ds.ResolveKey("sha512-123") if k != "sha512-1234567890" { t.Errorf("expected %q, got %q", "sha512-1234567890", k) } diff --git a/cas/remote.go b/cas/remote.go index 320714a..56cd60f 100644 --- a/cas/remote.go +++ b/cas/remote.go @@ -79,7 +79,7 @@ func (r Remote) Download(ds Store) (*Remote, error) { return nil, fmt.Errorf("bad HTTP status code: %d", res.StatusCode) } - key, err := ds.WriteACI(r.Hash(), reader) + key, err := ds.WriteACI(reader) if err != nil { return nil, err } diff --git a/cas/z_last_test.go b/cas/z_last_test.go index 8a9a498..a391f4a 100644 --- a/cas/z_last_test.go +++ b/cas/z_last_test.go @@ -21,7 +21,7 @@ func interestingGoroutines() (gs []string) { } stack := strings.TrimSpace(sl[1]) if stack == "" || - strings.Contains(stack, "created by testing.RunTests") || + strings.Contains(stack, "testing.RunTests") || strings.Contains(stack, "testing.Main(") || strings.Contains(stack, "runtime.goexit") || strings.Contains(stack, "created by runtime.gc") || diff --git a/path/path.go b/path/path.go index 78e8cc4..46f92c1 100644 --- a/path/path.go +++ b/path/path.go @@ -30,7 +30,7 @@ func ContainerManifestPath(root string) string { // AppImagePath returns the path where an app image (i.e. unpacked ACI) is rooted (i.e. // where its contents are extracted during stage0), based on the app image ID. func AppImagePath(root string, imageID types.Hash) string { - return filepath.Join(root, Stage1Dir, stage2Dir, imageID.String()) + return filepath.Join(root, Stage1Dir, stage2Dir, types.ShortHash(imageID.String())) } // AppRootfsPath returns the path to an app's rootfs. @@ -42,7 +42,7 @@ func AppRootfsPath(root string, imageID types.Hash) string { // RelAppImagePath returns the path of an application image relative to the // stage1 chroot func RelAppImagePath(imageID types.Hash) string { - return filepath.Join(stage2Dir, imageID.String()) + return filepath.Join(stage2Dir, types.ShortHash(imageID.String())) } // RelAppImagePath returns the path of an application's rootfs relative to the diff --git a/rkt/run.go b/rkt/run.go index 6412176..ab8c419 100644 --- a/rkt/run.go +++ b/rkt/run.go @@ -62,8 +62,7 @@ func findImages(args []string, ds *cas.Store) (out []types.Hash, err error) { // import the local file if it exists file, err := os.Open(img) if err == nil { - tmp := types.NewHashSHA512([]byte(img)).String() - key, err := ds.WriteACI(tmp, file) + key, err := ds.WriteACI(file) file.Close() if err != nil { return nil, fmt.Errorf("%s: %v", img, err) diff --git a/stage0/run.go b/stage0/run.go index 4997eac..e21642b 100644 --- a/stage0/run.go +++ b/stage0/run.go @@ -48,8 +48,9 @@ type Config struct { Stage1Init string // binary to be execed as stage1 Stage1Rootfs string // compressed bundle containing a rootfs for stage1 Debug bool - Images []types.Hash // application images - Volumes map[string]string // map of volumes that rocket can provide to applications + // TODO(jonboulle): These images are partially-populated hashes, this should be clarified. + Images []types.Hash // application images + Volumes map[string]string // map of volumes that rocket can provide to applications } func init() { @@ -266,9 +267,10 @@ func unpackBuiltinRootfs(dir string) error { } // setupImage attempts to load the image by the given hash from the store, -// verifies that the image matches the given hash and extracts the image -// into a directory in the given dir. -// It returns the ImageManifest that the image contains +// verifies that the image matches the hash, and extracts the image into a +// directory in the given dir. +// It returns the ImageManifest that the image contains. +// TODO(jonboulle): tighten up the Hash type here; currently it is partially-populated (i.e. half-length sha512) func setupImage(cfg Config, img types.Hash, dir string) (*schema.ImageManifest, error) { log.Println("Loading image", img.String()) @@ -295,11 +297,12 @@ func setupImage(cfg Config, img types.Hash, dir string) (*schema.ImageManifest, return nil, fmt.Errorf("error reading ACI: %v", err) } - if id := fmt.Sprintf("%x", hash.Sum(nil)); id != img.Val { + // TODO(jonboulle): clean this up, leaky abstraction with the store. + if g := cas.HashToKey(hash); g != img.String() { if err := os.RemoveAll(ad); err != nil { fmt.Fprintf(os.Stderr, "error cleaning up directory: %v\n", err) } - return nil, fmt.Errorf("image hash does not match expected (%v != %v)", id, img.Val) + return nil, fmt.Errorf("image hash does not match expected (%v != %v)", g, img.String()) } err = os.MkdirAll(filepath.Join(ad, "rootfs/tmp"), 0777)