diff --git a/internal/service/attestation/aux.go b/internal/service/attestation/aux.go index e6d8dbe..89c2105 100644 --- a/internal/service/attestation/aux.go +++ b/internal/service/attestation/aux.go @@ -9,55 +9,6 @@ import ( "github.com/Amnesic-Systems/veil/internal/nonce" ) -// Builder is a helper for setting auxiliary attestation both at initialization -// time and at attestation time. -type Builder struct { - attester enclave.Attester - aux enclave.AuxInfo -} - -type AuxField func(*Builder) - -// NewBuilder returns a new Builder with the given attester and sets the given -// auxiliary fields. -func NewBuilder(attester enclave.Attester, opts ...AuxField) *Builder { - b := &Builder{attester: attester} - for _, opt := range opts { - opt(b) - } - return b -} - -// Attest returns an attestation document with the auxiliary fields that were -// either already set, or are now passed in as options. -func (b *Builder) Attest(opts ...AuxField) (*enclave.RawDocument, error) { - for _, opt := range opts { - opt(b) - } - return b.attester.Attest(&b.aux) -} - -// WithHashes sets the given hashes in an auxiliary field. -func WithHashes(h *Hashes) AuxField { - return func(b *Builder) { - b.aux.PublicKey = h.Serialize() // TODO: safe? - } -} - -// WithNonce sets the given nonce in an auxiliary field. -func WithNonce(n *nonce.Nonce) AuxField { - return func(b *Builder) { - b.aux.Nonce = n.ToSlice() // TODO: safe? - } -} - -// WithSHA256 sets the given SHA256 hash in an auxiliary field. -func WithSHA256(sha [sha256.Size]byte) AuxField { - return func(b *Builder) { - b.aux.UserData = sha[:] - } -} - // GetNonce returns the nonce from the given auxiliary info. func GetNonce(aux *enclave.AuxInfo) (*nonce.Nonce, error) { if aux.Nonce == nil { diff --git a/internal/service/attestation/aux_test.go b/internal/service/attestation/aux_test.go index 4283386..7b90a17 100644 --- a/internal/service/attestation/aux_test.go +++ b/internal/service/attestation/aux_test.go @@ -6,8 +6,6 @@ import ( "github.com/Amnesic-Systems/veil/internal/addr" "github.com/Amnesic-Systems/veil/internal/enclave" - "github.com/Amnesic-Systems/veil/internal/enclave/nitro" - "github.com/Amnesic-Systems/veil/internal/enclave/noop" "github.com/Amnesic-Systems/veil/internal/errs" "github.com/Amnesic-Systems/veil/internal/nonce" "github.com/Amnesic-Systems/veil/internal/util" @@ -75,68 +73,3 @@ func TestGetters(t *testing.T) { }) } } - -func TestBuilder(t *testing.T) { - attester := noop.NewAttester() - if nitro.IsEnclave() { - attester = nitro.NewAttester() - } - nonce1, nonce2 := util.Must(nonce.New()), util.Must(nonce.New()) - sha1, sha2 := sha256.Sum256([]byte("foo")), sha256.Sum256([]byte("bar")) - hashes1 := &Hashes{TlsKeyHash: addr.Of(sha256.Sum256([]byte("foo")))} - hashes2 := &Hashes{TlsKeyHash: addr.Of(sha256.Sum256([]byte("bar")))} - - cases := []struct { - name string - initFields []AuxField - attestFields []AuxField - wantAux *enclave.AuxInfo - }{ - { - name: "empty", - wantAux: &enclave.AuxInfo{}, - }, - { - name: "nonce at initialization", - initFields: []AuxField{WithNonce(nonce1)}, - wantAux: &enclave.AuxInfo{Nonce: nonce1.ToSlice()}, - }, - { - name: "nonce at attestation", - attestFields: []AuxField{WithNonce(nonce1)}, - wantAux: &enclave.AuxInfo{Nonce: nonce1.ToSlice()}, - }, - { - name: "nonce being overwritten", - initFields: []AuxField{WithNonce(nonce1)}, - attestFields: []AuxField{WithNonce(nonce2)}, - wantAux: &enclave.AuxInfo{Nonce: nonce2.ToSlice()}, - }, - { - name: "everything overwritten", - initFields: []AuxField{WithHashes(hashes1), WithNonce(nonce1), WithSHA256(sha1)}, - attestFields: []AuxField{WithHashes(hashes2), WithNonce(nonce2), WithSHA256(sha2)}, - wantAux: &enclave.AuxInfo{ - Nonce: nonce2.ToSlice(), - PublicKey: hashes2.Serialize(), - UserData: sha2[:], - }, - }, - } - - for _, c := range cases { - t.Run(c.name, func(t *testing.T) { - b := NewBuilder(attester, c.initFields...) - rawDoc, err := b.Attest(c.attestFields...) - require.NoError(t, err) - - // Verify the attestation document. We expect no error but if the - // test is run inside a Nitro Enclave, we will get ErrDebugMode. - doc, err := attester.Verify(rawDoc, nil) - if err != nil { - require.ErrorIs(t, err, nitro.ErrDebugMode) - } - require.Equal(t, c.wantAux, &doc.AuxInfo) - }) - } -} diff --git a/internal/service/attestation/builder.go b/internal/service/attestation/builder.go new file mode 100644 index 0000000..cf609a5 --- /dev/null +++ b/internal/service/attestation/builder.go @@ -0,0 +1,72 @@ +package attestation + +import ( + "crypto/sha256" + + "github.com/Amnesic-Systems/veil/internal/enclave" + "github.com/Amnesic-Systems/veil/internal/nonce" +) + +// Builder is an abstraction purpose-built for veil's HTTP handlers. It bundles +// an attester with auxiliary fields because these two are always used together. +// As a Builder is passed through the stack, its auxiliary fields are updated +// and eventually used to create an attestation document. +type Builder struct { + enclave.Attester + enclave.AuxInfo +} + +type auxField func(*Builder) + +// NewBuilder returns a new Builder with the given attester and sets the given +// auxiliary fields. +func NewBuilder(attester enclave.Attester, opts ...auxField) *Builder { + b := &Builder{Attester: attester} + for _, opt := range opts { + opt(b) + } + return b +} + +// Update updates the builder with the given auxiliary fields. +func (b *Builder) Update(opts ...auxField) { + for _, opt := range opts { + opt(b) + } +} + +// Attest returns an attestation document with the auxiliary fields that were +// either already set, or are now passed in as options. +func (b *Builder) Attest(opts ...auxField) (*enclave.RawDocument, error) { + for _, opt := range opts { + opt(b) + } + return b.Attester.Attest(&b.AuxInfo) +} + +// WithHashes sets the given hashes in an auxiliary field. +func WithHashes(h *Hashes) auxField { + return func(b *Builder) { + if h == nil { + return + } + b.PublicKey = h.Serialize() + } +} + +// WithNonce sets the given nonce in an auxiliary field. +func WithNonce(n *nonce.Nonce) auxField { + return func(b *Builder) { + if n == nil { + return + } + b.Nonce = n.ToSlice() + } +} + +// WithSHA256 sets the given SHA256 hash in an auxiliary field. +func WithSHA256(sha [sha256.Size]byte) auxField { + return func(b *Builder) { + b.UserData = sha[:] + } +} diff --git a/internal/service/attestation/builder_test.go b/internal/service/attestation/builder_test.go new file mode 100644 index 0000000..4d1ada4 --- /dev/null +++ b/internal/service/attestation/builder_test.go @@ -0,0 +1,79 @@ +package attestation + +import ( + "crypto/sha256" + "testing" + + "github.com/Amnesic-Systems/veil/internal/addr" + "github.com/Amnesic-Systems/veil/internal/enclave" + "github.com/Amnesic-Systems/veil/internal/enclave/nitro" + "github.com/Amnesic-Systems/veil/internal/enclave/noop" + "github.com/Amnesic-Systems/veil/internal/nonce" + "github.com/Amnesic-Systems/veil/internal/util" + "github.com/stretchr/testify/require" +) + +func TestBuilder(t *testing.T) { + attester := noop.NewAttester() + if nitro.IsEnclave() { + attester = nitro.NewAttester() + } + nonce1, nonce2 := util.Must(nonce.New()), util.Must(nonce.New()) + sha1, sha2 := sha256.Sum256([]byte("foo")), sha256.Sum256([]byte("bar")) + hashes1 := &Hashes{TlsKeyHash: addr.Of(sha256.Sum256([]byte("foo")))} + hashes2 := &Hashes{TlsKeyHash: addr.Of(sha256.Sum256([]byte("bar")))} + + cases := []struct { + name string + initFields []auxField + attestFields []auxField + wantAux *enclave.AuxInfo + }{ + { + name: "empty", + wantAux: &enclave.AuxInfo{}, + }, + { + name: "nonce at initialization", + initFields: []auxField{WithNonce(nonce1)}, + wantAux: &enclave.AuxInfo{Nonce: nonce1.ToSlice()}, + }, + { + name: "nonce at attestation", + attestFields: []auxField{WithNonce(nonce1)}, + wantAux: &enclave.AuxInfo{Nonce: nonce1.ToSlice()}, + }, + { + name: "nonce being overwritten", + initFields: []auxField{WithNonce(nonce1)}, + attestFields: []auxField{WithNonce(nonce2)}, + wantAux: &enclave.AuxInfo{Nonce: nonce2.ToSlice()}, + }, + { + name: "everything overwritten", + initFields: []auxField{WithHashes(hashes1), WithNonce(nonce1), WithSHA256(sha1)}, + attestFields: []auxField{WithHashes(hashes2), WithNonce(nonce2), WithSHA256(sha2)}, + wantAux: &enclave.AuxInfo{ + Nonce: nonce2.ToSlice(), + PublicKey: hashes2.Serialize(), + UserData: sha2[:], + }, + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + b := NewBuilder(attester, c.initFields...) + rawDoc, err := b.Attest(c.attestFields...) + require.NoError(t, err) + + // Verify the attestation document. We expect no error but if the + // test is run inside a Nitro Enclave, we will get ErrDebugMode. + doc, err := attester.Verify(rawDoc, nil) + if err != nil { + require.ErrorIs(t, err, nitro.ErrDebugMode) + } + require.Equal(t, c.wantAux, &doc.AuxInfo) + }) + } +} diff --git a/internal/service/handle/encode.go b/internal/service/handle/encode.go index 27a432d..d3bfb41 100644 --- a/internal/service/handle/encode.go +++ b/internal/service/handle/encode.go @@ -4,11 +4,9 @@ import ( "crypto/sha256" "encoding/json" "fmt" - "log" "net/http" "github.com/Amnesic-Systems/veil/internal/httperr" - "github.com/Amnesic-Systems/veil/internal/httpx" "github.com/Amnesic-Systems/veil/internal/service/attestation" ) @@ -23,37 +21,17 @@ func encode[T any](w http.ResponseWriter, status int, v T) { } } -func encodeAndMaybeAttest[T any]( - w http.ResponseWriter, - r *http.Request, - status int, - builder *attestation.Builder, - v T, -) { - // Depending on if the request contains a nonce, either return the JSON - // response without attestation or include an attestation document in the - // response. - if _, err := httpx.ExtractNonce(r); err != nil { - encode(w, status, v) - } else { - encodeAndAttest(w, r, status, builder, v) - } -} - func encodeAndAttest[T any]( w http.ResponseWriter, - r *http.Request, status int, builder *attestation.Builder, v T, ) { - // Try to extract the client's nonce from the request. If this fails, abort - // attestation because the client no longer has a way to verify the - // attestation document's freshness. - n, err := httpx.ExtractNonce(r) - if err != nil { - log.Println(err) - encode(w, http.StatusBadRequest, httperr.New("found no valid nonce in HTTP request")) + // It's a bug if the caller didn't set a nonce in the builder. Attestation + // documents can be replayed if they're not tied to a nonce, so it's best to + // return an error. + if builder.Nonce == nil { + encode(w, http.StatusInternalServerError, httperr.New("caller didn't set nonce")) return } @@ -67,7 +45,6 @@ func encodeAndAttest[T any]( // hash and the client's nonce. hash := sha256.Sum256(body) attestation, err := builder.Attest( - attestation.WithNonce(n), attestation.WithSHA256(hash), ) if err != nil { diff --git a/internal/service/handle/encode_test.go b/internal/service/handle/encode_test.go index 9df7532..ff56b07 100644 --- a/internal/service/handle/encode_test.go +++ b/internal/service/handle/encode_test.go @@ -1,64 +1,91 @@ package handle import ( - "fmt" + "encoding/json" "net/http" "net/http/httptest" - "net/url" "testing" "github.com/stretchr/testify/require" + "github.com/Amnesic-Systems/veil/internal/enclave" + "github.com/Amnesic-Systems/veil/internal/enclave/nitro" "github.com/Amnesic-Systems/veil/internal/enclave/noop" "github.com/Amnesic-Systems/veil/internal/httperr" + "github.com/Amnesic-Systems/veil/internal/nonce" "github.com/Amnesic-Systems/veil/internal/service/attestation" + "github.com/Amnesic-Systems/veil/internal/util" ) func TestEncodeAndAttest(t *testing.T) { + attester := nitro.NewAttester() + if !nitro.IsEnclave() { + attester = noop.NewAttester() + } + cases := []struct { name string - nonce string - status int - builder *attestation.Builder - body interface{} - wantBody string + nonce *nonce.Nonce + body any wantStatus int + wantBody string }{ { - name: "bad nonce", - wantStatus: http.StatusBadRequest, - }, - { - name: "bad body", - nonce: "hJkjpaP/6cVT+vikk06HcN0aOdU=", + name: "bad body", // Trigger an error by passing a channel, which is not supported by // json.Marshal. body: make(chan int), wantStatus: http.StatusInternalServerError, }, { - name: "valid encoding", - nonce: "hJkjpaP/6cVT+vikk06HcN0aOdU=", - status: http.StatusOK, - builder: attestation.NewBuilder(noop.NewAttester()), + name: "missing nonce", + wantStatus: http.StatusInternalServerError, body: httperr.New("random error"), - wantBody: `{"error":"random error"}`, + }, + { + name: "everything valid", + nonce: util.Must(nonce.New()), wantStatus: http.StatusOK, + body: httperr.New("random error"), + wantBody: `{"error":"random error"}` + "\n", }, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { rec := httptest.NewRecorder() - req := httptest.NewRequest( - http.MethodGet, - fmt.Sprintf("/foo?nonce=%s", url.QueryEscape(c.nonce)), - nil, - ) - encodeAndAttest(rec, req, c.status, c.builder, c.body) + builder := attestation.NewBuilder(attester, attestation.WithNonce(c.nonce)) + encodeAndAttest(rec, http.StatusOK, builder, c.body) resp := rec.Result() require.Equal(t, c.wantStatus, resp.StatusCode, httperr.FromBody(resp)) + + if c.wantBody != "" { + require.Equal(t, c.wantBody, rec.Body.String()) + } + + if resp.StatusCode != http.StatusOK { + return + } + + // Extract the attestation document from the response header. + var rawDoc enclave.RawDocument + err := json.Unmarshal([]byte(resp.Header.Get(attestationHeader)), &rawDoc) + require.NoError(t, err) + + // The call to Verify is going to fail if we're inside a Nitro + // Enclave because the attestation document was produced in debug + // mode, which we need to see the test output. + if attester.Type() == enclave.TypeNitro { + return + } + doc, err := attester.Verify(&rawDoc, c.nonce) + require.NoError(t, err) + + // Ensure that the nonce is correct. + n, err := nonce.FromSlice(doc.AuxInfo.Nonce) + require.NoError(t, err) + require.Equal(t, c.nonce, n) }) } } diff --git a/internal/service/handle/handlers.go b/internal/service/handle/handlers.go index a829ee3..fee270f 100644 --- a/internal/service/handle/handlers.go +++ b/internal/service/handle/handlers.go @@ -35,7 +35,16 @@ func Config( cfg *config.Config, ) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { - encodeAndMaybeAttest(w, r, http.StatusOK, builder, cfg) + // If the client provided a nonce, we will add an attestation document + // to the response header. Otherwise there's no need to be pedantic + // because this isn't a security-sensitive endpoint, so we simply return + // the configuration without attestation. + if n, err := httpx.ExtractNonce(r); err == nil { + builder.Update(attestation.WithNonce(n)) + encodeAndAttest(w, http.StatusOK, builder, cfg) + } else { + encode(w, http.StatusOK, cfg) + } } }