refactored add-entry code path

3 files changed, 63 insertions, 55 deletions

diff --git a/handler.go b/handler.go
index b6a52f8..863bbbb 100644
--- a/handler.go
+++ b/handler.go
@@ -5,11 +5,9 @@ import (
 	"fmt"
 
 	"encoding/json"
-	"io/ioutil"
 	"net/http"
 
 	"github.com/golang/glog"
-	"github.com/google/certificate-transparency-go/tls"
 	"github.com/google/trillian"
 )
 
@@ -47,29 +45,23 @@ 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")
-	var request AddEntryRequest
-	if err := unpackRequest(r, &request); err != nil {
+	request, err := NewAddEntryRequest(r)
+	if err != nil {
 		return http.StatusBadRequest, err
-	}
+	} // request can be decoded
 
-	item, err := verifyAddEntryRequest(request)
+	leaf, err := VerifyAddEntryRequest(request)
 	if err != nil {
 		return http.StatusBadRequest, err
-	}
-	glog.Infof("got item: %s", item)
+	} // leaf is valid, e.g., signed by a trust anchor
 
-	serializedItem, err := tls.Marshal(*item)
-	if err != nil {
-		return http.StatusInternalServerError, fmt.Errorf("tls marshal failed: %v", err)
-	}
 	trillianRequest := trillian.QueueLeafRequest{
 		LogId: i.logID,
 		Leaf: &trillian.LogLeaf{
-			LeafValue: serializedItem,
+			LeafValue: leaf,
 			//TODO: add appendix here w/ chain + signature
 		},
 	}
-
 	trillianResponse, err := i.client.QueueLeaf(ctx, &trillianRequest)
 	if err != nil {
 		return http.StatusInternalServerError, fmt.Errorf("backend QueueLeaf request failed: %v", err)
@@ -78,40 +70,12 @@ func addEntry(ctx context.Context, i *instance, w http.ResponseWriter, r *http.R
 		return http.StatusInternalServerError, fmt.Errorf("missing QueueLeaf response")
 	}
 	// TODO: check that we got gRPC OK as specified in Trillian's API doc
+	glog.Infof("Queued leaf: %v", trillianResponse.QueuedLeaf.Leaf.LeafValue)
 
-	queuedLeaf := trillianResponse.QueuedLeaf
-	glog.Infof("Queued leaf: %v", queuedLeaf.Leaf.LeafValue)
 	// TODO: respond with an SDI
-
 	return http.StatusOK, nil
 }
 
-// verifyAddEntryRequest
-func verifyAddEntryRequest(r AddEntryRequest) (*StItem, error) {
-	item, err := StItemFromB64(r.Item)
-	if err != nil {
-		return nil, fmt.Errorf("failed decoding StItem: %v", err)
-	}
-	if item.Format != StFormatChecksumV1 {
-		return nil, fmt.Errorf("invalid StItem format: %s", item.Format)
-	}
-	// TODO: verify checksum length
-	// TODO: verify r.Signature and r.Certificate
-	return item, nil
-}
-
-// unpackRequest tries to unpack a json-encoded HTTP POST request into `unpack`
-func unpackRequest(r *http.Request, unpack interface{}) error {
-	body, err := ioutil.ReadAll(r.Body)
-	if err != nil {
-		return fmt.Errorf("failed reading request body: %v", err)
-	}
-	if err := json.Unmarshal(body, &unpack); err != nil {
-		return fmt.Errorf("failed parsing json body: %v", err)
-	}
-	return nil
-}
-
 // 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("in getEntries")
diff --git a/reqres.go b/reqres.go
index a18b65d..ab05ad5 100644
--- a/reqres.go
+++ b/reqres.go
@@ -5,7 +5,9 @@ import (
 	"strconv"
 
 	"encoding/base64"
+	"encoding/json"
 	"net/http"
+	"io/ioutil"
 
 	"github.com/google/certificate-transparency-go/tls"
 	"github.com/google/trillian"
@@ -13,21 +15,21 @@ import (
 
 // AddEntryRequest is a collection of add-entry input parameters
 type AddEntryRequest struct {
-	Item        string `json:"item"`
-	Signature   string `json:"signature"`
-	Certificate string `json:"certificate"`
+	Item        string `json:"item"`        // base64-encoded StItem
+	Signature   string `json:"signature"`   // base64-encoded DigitallySigned
+	Certificate string `json:"certificate"` // base64-encoded X.509 certificate
 }
 
 // GetEntriesRequest is a collection of get-entry input parameters
 type GetEntriesRequest struct {
-	Start int64 `json:"start"`
-	End   int64 `json:"end"`
+	Start int64 `json:"start"` // 0-based and inclusive start-index
+	End   int64 `json:"end"`   // 0-based and inclusive end-index
 }
 
 // GetProofByHashRequest is a collection of get-proof-by-hash input parameters
 type GetProofByHashRequest struct {
-	Hash     []byte `json:"hash"`
-	TreeSize int64  `json:"tree_size"`
+	Hash     []byte `json:"hash"`      // base64-encoded leaf hash
+	TreeSize int64  `json:"tree_size"` // Tree head size to base proof on
 }
 
 // GetEntryResponse is an assembled log entry and its associated appendix
@@ -47,6 +49,27 @@ type GetProofByHashResponse struct {
 	InclusionProof string `json:"inclusion_proof"` // base64-encoded StItem
 }
 
+// NewAddEntryRequest parses and sanitizes the JSON-encoded add-entry
+// parameters from an incoming HTTP post.  The resulting AddEntryRequest is
+// well-formed, but not necessarily trusted (further sanitization is needed).
+func NewAddEntryRequest(r *http.Request) (AddEntryRequest, error) {
+	var ret AddEntryRequest
+	if err := UnpackJsonPost(r, &ret); err != nil {
+		return ret, err
+	}
+
+	item, err := StItemFromB64(ret.Item)
+	if err != nil {
+		return ret, fmt.Errorf("failed decoding StItem: %v", err)
+	}
+	if item.Format != StFormatChecksumV1 {
+		return ret, fmt.Errorf("invalid StItem format: %s", item.Format)
+	}
+	// TODO: verify that we got a checksum length
+	// TODO: verify that we got a signature and certificate
+	return ret, nil
+}
+
 // NewGetEntriesRequest parses and sanitizes the URL-encoded get-entries
 // parameters from an incoming HTTP request.
 func NewGetEntriesRequest(httpRequest *http.Request) (GetEntriesRequest, error) {
@@ -117,3 +140,24 @@ func NewGetProofByHashResponse(treeSize uint64, inclusionProof *trillian.Proof)
 		InclusionProof: base64.StdEncoding.EncodeToString(b),
 	}, nil
 }
+
+// VerifyAddEntryRequest determines whether a well-formed AddEntryRequest should
+// be inserted into the log.  If so, the serialized leaf value is returned.
+func VerifyAddEntryRequest(r AddEntryRequest) ([]byte, error) {
+	item, _ := StItemFromB64(r.Item) // r.Item is a well-formed ChecksumV1
+	// TODO: verify r.Signature and r.Certificate
+	leaf, _ := tls.Marshal(item) // again, r.Item is well-formed
+	return leaf, nil
+}
+
+// UnpackJsonPost unpacks a json-encoded HTTP POST request into `unpack`
+func UnpackJsonPost(r *http.Request, unpack interface{}) error {
+	body, err := ioutil.ReadAll(r.Body)
+	if err != nil {
+		return fmt.Errorf("failed reading request body: %v", err)
+	}
+	if err := json.Unmarshal(body, &unpack); err != nil {
+		return fmt.Errorf("failed parsing json body: %v", err)
+	}
+	return nil
+}
diff --git a/type.go b/type.go
index d47996e..9166209 100644
--- a/type.go
+++ b/type.go
@@ -121,18 +121,18 @@ func (i InclusionProofV1) String() string {
 }
 
 // StItemFromB64 creates an StItem from a serialized and base64-encoded string
-func StItemFromB64(s string) (*StItem, error) {
+func StItemFromB64(s string) (StItem, error) {
 	b, err := base64.StdEncoding.DecodeString(s)
 	if err != nil {
-		return nil, fmt.Errorf("base64 decoding failed: %v", err)
+		return StItem{}, fmt.Errorf("base64 decoding failed: %v", err)
 	}
 
 	var item StItem
 	extra, err := tls.Unmarshal(b, &item)
 	if err != nil {
-		return nil, fmt.Errorf("tls unmarshal failed: %v", err)
+		return StItem{}, fmt.Errorf("tls unmarshal failed: %v", err)
 	} else if len(extra) > 0 {
-		return nil, fmt.Errorf("tls unmarshal found extra data: %v", extra)
+		return StItem{}, fmt.Errorf("tls unmarshal found extra data: %v", extra)
 	}
-	return &item, nil
+	return item, nil
 }