From 24eb746adec654637f88ae185012dc984cf9b2dc Mon Sep 17 00:00:00 2001 From: Rasmus Dahlberg Date: Fri, 30 Oct 2020 18:10:49 +0100 Subject: added sanity checks on Trillian responses Based on the sanity checks that CTFE does. --- handler.go | 46 ++++++++++++++++++++++---------------------- trillian.go | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 82 insertions(+), 27 deletions(-) diff --git a/handler.go b/handler.go index a09f50a..675c441 100644 --- a/handler.go +++ b/handler.go @@ -44,11 +44,11 @@ func (a appHandler) sendHTTPError(w http.ResponseWriter, statusCode int, err err } func addEntry(ctx context.Context, i *Instance, w http.ResponseWriter, r *http.Request) (int, error) { - glog.Info("in addEntry") + glog.V(3).Info("handling add-entry request") leaf, appendix, err := NewAddEntryRequest(i.LogParameters, r) if err != nil { return http.StatusBadRequest, err - } // request is well-formed, signed, and chains back to a trust anchor + } treq := trillian.QueueLeafRequest{ LogId: i.LogParameters.TreeId, @@ -60,14 +60,15 @@ func addEntry(ctx context.Context, i *Instance, w http.ResponseWriter, r *http.R trsp, err := i.Client.QueueLeaf(ctx, &treq) if err != nil { return http.StatusInternalServerError, fmt.Errorf("backend QueueLeaf request failed: %v", err) - } // note: more detail could be provided here, see addChainInternal in ctfe - glog.Infof("Queued leaf: %v", trsp.QueuedLeaf.Leaf.LeafValue) + } + if status, err := checkQueueLeaf(trsp); err != nil { + return status, err + } sdi, err := GenV1SDI(i.LogParameters, trsp.QueuedLeaf.Leaf.LeafValue) if err != nil { return http.StatusInternalServerError, fmt.Errorf("failed creating signed debug info: %v", err) } - rsp, err := StItemToB64(sdi) if err != nil { return http.StatusInternalServerError, err @@ -80,11 +81,11 @@ func addEntry(ctx context.Context, i *Instance, w http.ResponseWriter, r *http.R // getEntries provides a list of entries from the Trillian backend func getEntries(ctx context.Context, i *Instance, w http.ResponseWriter, r *http.Request) (int, error) { - glog.Info("handling get-entries request") + glog.V(3).Info("handling get-entries request") req, err := NewGetEntriesRequest(i.LogParameters, r) if err != nil { return http.StatusBadRequest, err - } // request can be decoded and is mostly valid (range not cmp vs tree size) + } treq := trillian.GetLeavesByRangeRequest{ LogId: i.LogParameters.TreeId, @@ -95,7 +96,7 @@ func getEntries(ctx context.Context, i *Instance, w http.ResponseWriter, r *http if err != nil { return http.StatusInternalServerError, fmt.Errorf("backend GetLeavesByRange request failed: %v", err) } - if status, err := checkGetLeavesByRange(trsp, req); err != nil { + if status, err := checkGetLeavesByRange(trsp, &req); err != nil { return status, err } @@ -111,7 +112,7 @@ func getEntries(ctx context.Context, i *Instance, w http.ResponseWriter, r *http // getAnchors provides a list of configured trust anchors func getAnchors(_ context.Context, i *Instance, w http.ResponseWriter, _ *http.Request) (int, error) { - glog.Info("in getAnchors") + glog.V(3).Info("handling get-anchors request") data := NewGetAnchorsResponse(i.LogParameters.AnchorList) if err := WriteJsonResponse(data, w); err != nil { return http.StatusInternalServerError, err @@ -121,11 +122,11 @@ func getAnchors(_ context.Context, i *Instance, w http.ResponseWriter, _ *http.R // getProofByHash provides an inclusion proof based on a given leaf hash func getProofByHash(ctx context.Context, i *Instance, w http.ResponseWriter, r *http.Request) (int, error) { - glog.Info("in getProofByHash") + glog.V(3).Info("handling get-proof-by-hash request") req, err := NewGetProofByHashRequest(r) if err != nil { return http.StatusBadRequest, err - } // request can be decoded and is valid + } treq := trillian.GetInclusionProofByHashRequest{ LogId: i.LogParameters.TreeId, @@ -137,13 +138,9 @@ func getProofByHash(ctx context.Context, i *Instance, w http.ResponseWriter, r * if err != nil { return http.StatusInternalServerError, fmt.Errorf("failed fetching inclusion proof from Trillian backend: %v", err) } - // TODO: check the returned tree size in response? - - // Santity check - if len(trsp.Proof) == 0 { - return http.StatusNotFound, fmt.Errorf("get-proof-by-hash backend returned no proof") + if status, err := checkGetInclusionProofByHash(trsp, i.LogParameters); err != nil { + return status, err } - // TODO: verify that proof is valid? rsp, err := StItemToB64(NewInclusionProofV1(i.LogParameters.LogId, uint64(req.TreeSize), trsp.Proof[0])) if err != nil { @@ -157,11 +154,11 @@ func getProofByHash(ctx context.Context, i *Instance, w http.ResponseWriter, r * // getConsistencyProof provides a consistency proof between two STHs func getConsistencyProof(ctx context.Context, i *Instance, w http.ResponseWriter, r *http.Request) (int, error) { - glog.Info("in getConsistencyProof") + glog.V(3).Info("handling get-consistency-proof request") req, err := NewGetConsistencyProofRequest(r) if err != nil { return http.StatusBadRequest, err - } // request can be decoded and is valid + } treq := trillian.GetConsistencyProofRequest{ LogId: i.LogParameters.TreeId, @@ -172,7 +169,9 @@ func getConsistencyProof(ctx context.Context, i *Instance, w http.ResponseWriter if err != nil { return http.StatusInternalServerError, fmt.Errorf("failed fetching consistency proof from Trillian backend: %v", err) } - // TODO: santity-checks? + if status, err := checkGetConsistencyProofResponse(trsp, i.LogParameters); err != nil { + return status, err + } rsp, err := StItemToB64(NewConsistencyProofV1(i.LogParameters.LogId, req.First, req.Second, trsp.Proof)) if err != nil { @@ -186,7 +185,7 @@ func getConsistencyProof(ctx context.Context, i *Instance, w http.ResponseWriter // getSth provides the most recent STH func getSth(ctx context.Context, i *Instance, w http.ResponseWriter, _ *http.Request) (int, error) { - glog.Info("in getSth") + glog.V(3).Info("handling get-sth request") treq := trillian.GetLatestSignedLogRootRequest{ LogId: i.LogParameters.TreeId, } @@ -194,6 +193,9 @@ func getSth(ctx context.Context, i *Instance, w http.ResponseWriter, _ *http.Req if err != nil { return http.StatusInternalServerError, fmt.Errorf("failed fetching signed tree head from Trillian backend: %v", err) } + if status, err := checkTrillianGetLatestSignedLogRoot(trsp, i.LogParameters); err != nil { + return status, err + } th, err := NewTreeHeadV1(i.LogParameters, trsp.SignedLogRoot) if err != nil { @@ -203,8 +205,6 @@ func getSth(ctx context.Context, i *Instance, w http.ResponseWriter, _ *http.Req if err != nil { return http.StatusInternalServerError, fmt.Errorf("failed creating signed tree head: %v", err) } - glog.Infof("%v", sth) - rsp, err := StItemToB64(sth) if err != nil { return http.StatusInternalServerError, err diff --git a/trillian.go b/trillian.go index e5117da..147c5bd 100644 --- a/trillian.go +++ b/trillian.go @@ -5,28 +5,83 @@ import ( "net/http" + "github.com/golang/glog" "github.com/google/trillian" "github.com/google/trillian/types" + "google.golang.org/grpc/codes" ) -// checkGetLeavesByRange does santity-checking on a Trillian response -func checkGetLeavesByRange(rsp *trillian.GetLeavesByRangeResponse, req GetEntriesRequest) (int, error) { +func checkQueueLeaf(rsp *trillian.QueueLeafResponse) (int, error) { + if codes.Code(rsp.QueuedLeaf.GetStatus().GetCode()) == codes.AlreadyExists { + // no need to report this as an invalid request, just (re)issue sdi + glog.V(3).Infof("queued leaf is a duplicate => %X", rsp.QueuedLeaf.Leaf.LeafValue) + } else { + glog.V(3).Infof("queued leaf => %X", rsp.QueuedLeaf.Leaf.LeafValue) + } + return 0, nil +} + +func checkGetLeavesByRange(rsp *trillian.GetLeavesByRangeResponse, req *GetEntriesRequest) (int, error) { if len(rsp.Leaves) > int(req.End-req.Start+1) { return http.StatusInternalServerError, fmt.Errorf("backend GetLeavesByRange returned too many leaves: %d for [%d,%d]", len(rsp.Leaves), req.Start, req.End) } + // Ensure that a bad start parameter results in an error var lr types.LogRootV1 if err := lr.UnmarshalBinary(rsp.GetSignedLogRoot().GetLogRoot()); err != nil { return http.StatusInternalServerError, fmt.Errorf("failed unmarshaling log root: %v", err) } if uint64(req.Start) >= lr.TreeSize { - return http.StatusBadRequest, fmt.Errorf("invalid start(%d): tree size is %d", req.Start, lr.TreeSize) + return http.StatusNotFound, fmt.Errorf("invalid start(%d): tree size is %d", req.Start, lr.TreeSize) } + // Ensure that we got and return expected leaf indices for i, leaf := range rsp.Leaves { if leaf.LeafIndex != req.Start+int64(i) { - return http.StatusInternalServerError, fmt.Errorf("backend GetLeavesByRange returned unexpected leaf index: wanted %d, got %d", req.Start+int64(i), leaf.LeafIndex) + return http.StatusInternalServerError, fmt.Errorf("invalid leaf index: wanted %d, got %d", req.Start+int64(i), leaf.LeafIndex) } } return 0, nil } + +func checkGetInclusionProofByHash(rsp *trillian.GetInclusionProofByHashResponse, lp *LogParameters) (int, error) { + if rsp.Proof == nil || len(rsp.Proof) == 0 { + return http.StatusNotFound, fmt.Errorf("incomplete backend response") + } // maybe redundant, but better safe than sorry + return checkHashPath(lp.HashType.Size(), rsp.Proof[0].Hashes) +} + +func checkGetConsistencyProofResponse(rsp *trillian.GetConsistencyProofResponse, lp *LogParameters) (int, error) { + if rsp.Proof == nil { + return http.StatusNotFound, fmt.Errorf("incomplete backend response") + } // not redundant, out-of-range parameters yield an empty proof w/o error + return checkHashPath(lp.HashType.Size(), rsp.Proof.Hashes) +} + +func checkTrillianGetLatestSignedLogRoot(rsp *trillian.GetLatestSignedLogRootResponse, lp *LogParameters) (int, error) { + if rsp.SignedLogRoot == nil { + return http.StatusInternalServerError, fmt.Errorf("incomplete backend response") + } // maybe redundant + + var lr types.LogRootV1 + if err := lr.UnmarshalBinary(rsp.GetSignedLogRoot().GetLogRoot()); err != nil { + return http.StatusInternalServerError, fmt.Errorf("failed unmarshaling log root: %v", err) + } + if len(lr.RootHash) != lp.HashType.Size() { + return http.StatusInternalServerError, fmt.Errorf("invalid root hash: %v", lr.RootHash) + } // maybe redundant, but would not necessarily be caught by marshal error + return 0, nil +} + +func checkHashPath(hashSize int, path [][]byte) (int, error) { + for _, hash := range path { + if len(hash) != hashSize { + return http.StatusInternalServerError, fmt.Errorf("invalid proof: %v", path) + } + } // maybe redundant, but would not necessarily be caught by marshal error + return 0, nil +} + +func checkLogRoot(hashSize int, rootHash []byte) (int, error) { + return 0, nil +} -- cgit v1.2.3