Skip to content

Commit

Permalink
dir/server/serverlog: stabilize root backup reference
Browse files Browse the repository at this point in the history
The recently introduced root backup from local disk to storage server
intended to create a consistent per-user reference, but used ECDSA
signing which includes some randomness. The reference is desired to
be unguessable so that outsiders can't watch time and sequence.
There is no encryption or sign/verify involved. A better
cryptographic tool is HKDF (RFC 5869) already used as part of
key wrapping in pack/ee.

Add HKDF to the Factotum interface, and use it in place of the
old reference construction.

Add test that catches the old problem.

Add errors.Str in storagetest to avoid "unqualified type" complaint.

Fix #555.

Change-Id: Ibb083d9a630fd82beac835166d17144b5d97ac2a
Reviewed-on: https://upspin-review.googlesource.com/17560
Reviewed-by: Andrew Gerrand <adg@golang.org>
Reviewed-by: David Presotto <presotto@gmail.com>
  • Loading branch information
n2vi committed Dec 14, 2017
1 parent 93ca68f commit 0af281d
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 10 deletions.
4 changes: 2 additions & 2 deletions cloud/storage/storagetest/memory.go
Expand Up @@ -35,7 +35,7 @@ func (m *mem) Download(ref string) ([]byte, error) {

b, ok := m.m[ref]
if !ok {
return nil, errors.E(errors.NotExist, ref)
return nil, errors.E(errors.NotExist, errors.Str(ref))
}
return append([]byte{}, b...), nil
}
Expand All @@ -54,7 +54,7 @@ func (m *mem) Delete(ref string) error {

_, ok := m.m[ref]
if !ok {
return errors.E(errors.NotExist, ref)
return errors.E(errors.NotExist, errors.Str(ref))
}
delete(m.m, ref)
return nil
Expand Down
18 changes: 10 additions & 8 deletions dir/server/serverlog/log.go
Expand Up @@ -5,7 +5,6 @@
package serverlog

import (
"crypto/sha256"
"encoding/binary"
"fmt"
"io"
Expand Down Expand Up @@ -151,17 +150,17 @@ type root struct {
// name for the backup.
func newRoot(rootFile string, fac upspin.Factotum, s storage.Storage) (*root, error) {
var rootRef string
var err error
if s != nil {
// Use the provided factotum to generate the secret reference.
if fac == nil {
return nil, errors.Str("cannot backup root: config has no factotum")
}
base := filepath.Base(rootFile)
sig, err := fac.Sign(hashRoot(base))
rootRef, err = hashRoot(base, fac)
if err != nil {
return nil, err
}
rootRef = fmt.Sprintf("%s.%s-%s", base, sig.R, sig.S)

// Try to access the storage backend now
// so a misconfiguration is caught at startup.
Expand All @@ -184,11 +183,14 @@ func newRoot(rootFile string, fac upspin.Factotum, s storage.Storage) (*root, er
return r, nil
}

func hashRoot(base string) []byte {
h := sha256.New()
h.Write([]byte("@@hashRoot!!")) // Don't sign raw user string.
h.Write([]byte(base))
return h.Sum(nil)
func hashRoot(base string, fac upspin.Factotum) (string, error) {
salt := []byte("@@hashRoot!!")
suffix := make([]byte, 8)
err := fac.HKDF(salt, []byte(base), suffix)
if err != nil {
return "", err
}
return fmt.Sprintf("%s.%x", base, suffix), nil
}

func (r *root) saveLoop(s storage.Storage) {
Expand Down
9 changes: 9 additions & 0 deletions dir/server/serverlog/log_test.go
Expand Up @@ -558,6 +558,15 @@ func TestIndex(t *testing.T) {
t.Errorf("recoveredRoot = %v, want = %v", recoveredRoot, root)
}

// Check that root.ref is stable.
rootRef, err := hashRoot("tree.root.reallylongusernamefoo@bar.com", fac)
if err != nil {
t.Fatal(err)
}
if rootRef != user.root.ref {
t.Errorf("rootRef = %v, want = %v", rootRef, user.root.ref)
}

// Check that the root was put to the storage backend.
<-store.onPut
b, err := store.Storage.Download(user.root.ref)
Expand Down
16 changes: 16 additions & 0 deletions factotum/factotum.go
Expand Up @@ -12,12 +12,15 @@ import (
"crypto/rand"
"crypto/sha256"
"fmt"
"io"
"io/ioutil"
"math/big"
"os"
"path/filepath"
"strings"

"golang.org/x/crypto/hkdf"

"upspin.io/errors"
"upspin.io/upspin"
)
Expand Down Expand Up @@ -280,6 +283,19 @@ func Verify(hash []byte, sig upspin.Signature, key upspin.PublicKey) error {
return nil
}

// HKDF cryptographically mixes salt, info, and the Factotum secret and
// writes the result to out, which may be of any length but is typically
// 8 or 16 bytes. The result is unguessable without the secret, and does
// not leak the secret. For more information, see package
// golang.org/x/crypto/hkdf.
func (f factotum) HKDF(salt, info, out []byte) error {
hash := sha256.New
secret := []byte(f.keys[f.current].private)
hkdf := hkdf.New(hash, secret, salt, info)
_, err := io.ReadFull(hkdf, out)
return err
}

// Pop derives a Factotum by switching default from the current to the previous key.
func (f factotum) Pop() upspin.Factotum {
// Arbitrarily keep f.previous unchanged, so Pop() is idempotent.
Expand Down
7 changes: 7 additions & 0 deletions upspin/upspin.go
Expand Up @@ -144,6 +144,13 @@ type Factotum interface {
// no longer than your key's curve order. Don't use without a security consult.
Sign(hash []byte) (Signature, error)

// HKDF cryptographically mixes salt, info, and the Factotum secret and
// writes the result to out, which may be of any length but is typically
// 8 or 16 bytes. The result is unguessable without the secret, and does
// not leak the secret. For more information, see package
// golang.org/x/crypto/hkdf.
HKDF(salt, info, out []byte) error

// Pop derives a Factotum that defaults to the previous key.
Pop() Factotum

Expand Down

0 comments on commit 0af281d

Please sign in to comment.