fix: import/export PEM voucher (#28)

* fix: import/export PEM voucher

This patch changes the way importing and exporting a voucher was
handled. For some reason, it was dealing with jsons of voucher/owner
keys. This makes it so that it deals only with PEM voucher and owner
keys verification is left out to be implemented elsewhere.

Signed-off-by: Antonio Murdaca <[email protected]>
Co-authored-by: Miguel Martin <[email protected]>
Signed-off-by: Antonio Murdaca <[email protected]>

* fix: add body size limiter to /api endpoints

Signed-off-by: Ben Krieger <[email protected]>

* fix: limit /api/v1/to0 to GET method

Signed-off-by: Ben Krieger <[email protected]>

* fix: add owner key check on voucher import by API

Signed-off-by: Ben Krieger <[email protected]>

* fix: add check for invalid PEM in voucher import API

Signed-off-by: Ben Krieger <[email protected]>

* fix: do not fail if voucher already exists and is the same

Signed-off-by: Antonio Murdaca <[email protected]>

---------

Signed-off-by: Antonio Murdaca <[email protected]>
Signed-off-by: Ben Krieger <[email protected]>
Co-authored-by: Miguel Martin <[email protected]>
Co-authored-by: Ben Krieger <[email protected]>

Author: 3 people

README.md

@@ -149,15 +149,16 @@ Use GET and PUT requests to view and update existing owner redirect data.
 
 ## Fetch and Post Voucher
 Fetch a Voucher
+
 Fetch a voucher using curl and save it to a file named ownervoucher:
 ```
 curl --location --request GET 'http://localhost:8038/api/v1/vouchers?guid=<guid>' -o ownervoucher
 ```
-Post the Voucher to RV and Owner Server
-Post the fetched voucher to the RV and Owner server using curl:
+Post the Voucher to the Owner Server
+
+Post the fetched voucher to the Owner server using curl:
 ```
-curl -X POST 'http://localhost:8041/api/v1/owner/vouchers' -d @ownervoucher
-curl -X POST 'http://localhost:8043/api/v1/owner/vouchers' -d @ownervoucher
+curl -X POST 'http://localhost:8043/api/v1/owner/vouchers' --data-binary @ownervoucher
 ```
 ## Execute DI from the FDO GO Client.
 For Running the FDO GO Client setup, please refer to the FDO Go Client README.

api/handlers/to0.go

@@ -5,22 +5,17 @@ package handlers
 
 import (
 	"net/http"
-	"path"
 
-	"github.com/fido-device-onboard/go-fdo-server/internal/utils"
-
-	"github.com/fido-device-onboard/go-fdo-server/internal/to0"
 	"github.com/fido-device-onboard/go-fdo/protocol"
 	"github.com/fido-device-onboard/go-fdo/sqlite"
+
+	"github.com/fido-device-onboard/go-fdo-server/internal/to0"
+	"github.com/fido-device-onboard/go-fdo-server/internal/utils"
 )
 
 func To0Handler(rvInfo *[][]protocol.RvInstruction, state *sqlite.DB) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
-		to0Guid := path.Base(r.URL.Path)
-		if to0Guid == "" {
-			http.Error(w, "GUID is required", http.StatusBadRequest)
-			return
-		}
+		to0Guid := r.PathValue("guid")
 
 		if !utils.IsValidGUID(to0Guid) {
 			http.Error(w, "GUID is not a valid GUID", http.StatusBadRequest)
@@ -37,6 +32,6 @@ func To0Handler(rvInfo *[][]protocol.RvInstruction, state *sqlite.DB) http.Handl
 
 		w.Header().Set("Content-Type", "text/plain")
 		w.WriteHeader(http.StatusOK)
-		w.Write([]byte(to0Guid))
+		_, _ = w.Write([]byte(to0Guid))
 	}
 }

api/handlers/vouchers.go

@@ -4,19 +4,26 @@
 package handlers
 
 import (
+	"bytes"
+	"crypto"
+	"crypto/x509"
 	"database/sql"
 	"encoding/hex"
-	"encoding/json"
+	"encoding/pem"
 	"fmt"
+	"io"
 	"net/http"
 
+	"github.com/fido-device-onboard/go-fdo"
+
 	"github.com/fido-device-onboard/go-fdo-server/internal/utils"
 
 	"log/slog"
 
-	"github.com/fido-device-onboard/go-fdo-server/internal/db"
-	"github.com/fido-device-onboard/go-fdo-server/internal/rvinfo"
+	"github.com/fido-device-onboard/go-fdo/cbor"
 	"github.com/fido-device-onboard/go-fdo/protocol"
+
+	"github.com/fido-device-onboard/go-fdo-server/internal/db"
 )
 
 func GetVoucherHandler(w http.ResponseWriter, r *http.Request) {
@@ -49,66 +56,119 @@ func GetVoucherHandler(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	ownerKeys, err := db.FetchOwnerKeys()
-	if err != nil {
-		slog.Debug("Error querying owner_keys", "error", err)
-		http.Error(w, err.Error(), http.StatusInternalServerError)
-		return
-	}
-
-	response := struct {
-		Voucher   db.Voucher    `json:"voucher"`
-		OwnerKeys []db.OwnerKey `json:"owner_keys"`
-	}{
-		Voucher:   voucher,
-		OwnerKeys: ownerKeys,
-	}
-
-	data, err := json.Marshal(response)
-	if err != nil {
-		slog.Debug("Error marshalling JSON", "error", err)
+	w.Header().Set("Content-Type", "application/x-pem-file")
+	if err := pem.Encode(w, &pem.Block{
+		Type:  "OWNERSHIP VOUCHER",
+		Bytes: voucher.CBOR,
+	}); err != nil {
+		slog.Debug("Error encoding voucher", "error", err)
 		http.Error(w, err.Error(), http.StatusInternalServerError)
 		return
 	}
-	w.Header().Set("Content-Type", "application/json")
-	w.Write(data)
 }
 
 func InsertVoucherHandler(rvInfo *[][]protocol.RvInstruction) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
-		var request struct {
-			Voucher   db.Voucher    `json:"voucher"`
-			OwnerKeys []db.OwnerKey `json:"owner_keys"`
-		}
-
-		if err := json.NewDecoder(r.Body).Decode(&request); err != nil {
-			http.Error(w, "Invalid request payload", http.StatusBadRequest)
+		body, err := io.ReadAll(r.Body)
+		if err != nil {
+			http.Error(w, "Failure to read the request body", http.StatusInternalServerError)
 			return
 		}
 
-		guidHex := hex.EncodeToString(request.Voucher.GUID)
-		slog.Debug("Inserting voucher", "GUID", guidHex)
-
-		if err := db.InsertVoucher(request.Voucher); err != nil {
-			slog.Debug("Error inserting into database", "error", err)
-			http.Error(w, "Internal server error", http.StatusInternalServerError)
-			return
+		block, rest := pem.Decode(body)
+		for ; block != nil; block, rest = pem.Decode(rest) {
+			if block.Type != "OWNERSHIP VOUCHER" {
+				slog.Debug("Got unknown label type", "type", block.Type)
+				continue
+			}
+			var ov fdo.Voucher
+			if err := cbor.Unmarshal(block.Bytes, &ov); err != nil {
+				slog.Debug("Unable to decode cbor", "block", block.Bytes)
+				http.Error(w, "Unable to decode cbor", http.StatusBadRequest)
+				return
+			}
+
+			if dbOv, err := db.FetchVoucher(ov.Header.Val.GUID[:]); err == nil {
+				if bytes.Equal(block.Bytes, dbOv.CBOR) {
+					slog.Debug("Voucher already exists", "guid", ov.Header.Val.GUID[:])
+					continue
+				}
+				slog.Debug("Voucher guid already exists. not overwriting it", "guid", ov.Header.Val.GUID[:])
+			}
+
+			// Check that voucher owner key matches
+			expectedPubKey, err := ov.OwnerPublicKey()
+			if err != nil {
+				slog.Debug("Unable to parse owner public key of voucher", "err", err)
+				http.Error(w, "Invalid voucher", http.StatusBadRequest)
+				return
+			}
+			expectedKeyType := ov.Header.Val.ManufacturerKey.Type
+
+			ownerKeys, err := db.FetchOwnerKeys()
+			if err != nil {
+				slog.Debug("Error getting owner key", "err", err)
+				http.Error(w, "Unable to get appropriate owner key type", http.StatusInternalServerError)
+				return
+			}
+			var possibleOwnerKeys []db.OwnerKey
+			for _, ownerKey := range ownerKeys {
+				if ownerKey.Type == int(expectedKeyType) {
+					possibleOwnerKeys = append(possibleOwnerKeys, ownerKey)
+				}
+			}
+		CheckOwnerKey:
+			switch possibilities := len(possibleOwnerKeys); possibilities {
+			case 0:
+				http.Error(w, "owner key in database does not match the owner of the voucher", http.StatusBadRequest)
+				return
+
+			case 1, 2: // Can be two in the case of RSA 2048+3072 support
+				if possibilities == 2 && expectedKeyType != protocol.RsaPkcsKeyType && expectedKeyType != protocol.RsaPssKeyType {
+					slog.Error("database contains too many owner keys", "type", ov.Header.Val.ManufacturerKey.Type)
+					http.Error(w, "database contains too many owner keys of a type", http.StatusInternalServerError)
+					return
+				}
+
+				for _, possibleOwnerKey := range possibleOwnerKeys {
+					ownerKey, err := x509.ParsePKCS8PrivateKey(possibleOwnerKey.PKCS8)
+					if err != nil {
+						slog.Error("invalid owner key in DB", "type", expectedKeyType, "err", err)
+						http.Error(w, "owner key in database is malformed", http.StatusInternalServerError)
+						return
+					}
+					if ownerKey.(interface{ Public() crypto.PublicKey }).Public().(interface{ Equal(crypto.PublicKey) bool }).Equal(expectedPubKey) {
+						break CheckOwnerKey
+					}
+				}
+
+				http.Error(w, "owner key in database does not match the owner of the voucher", http.StatusBadRequest)
+				return
+
+			default:
+				slog.Error("database contains too many owner keys", "type", ov.Header.Val.ManufacturerKey.Type)
+				http.Error(w, "database contains too many owner keys of a type", http.StatusInternalServerError)
+				return
+			}
+
+			// TODO: https://github.com/fido-device-onboard/go-fdo-server/issues/18
+			slog.Debug("Inserting voucher", "GUID", ov.Header.Val.GUID)
+
+			if err := db.InsertVoucher(db.Voucher{GUID: ov.Header.Val.GUID[:], CBOR: block.Bytes}); err != nil {
+				slog.Debug("Error inserting into database", "error", err.Error())
+				http.Error(w, "Internal server error", http.StatusInternalServerError)
+				return
+			}
+
+			*rvInfo = ov.Header.Val.RvInfo
 		}
 
-		if err := db.UpdateOwnerKeys(request.OwnerKeys); err != nil {
-			slog.Debug("Error updating owner key in database", "error", err)
-			http.Error(w, "Internal server error", http.StatusInternalServerError)
+		if len(bytes.TrimSpace(rest)) > 0 {
+			http.Error(w, "Unable to decode PEM content", http.StatusBadRequest)
 			return
 		}
 
-		newRvInfo, err := rvinfo.GetRvInfoFromVoucher(request.Voucher.CBOR)
-		if err != nil {
-			fmt.Println("Error:", err)
-			return
-		}
-		*rvInfo = newRvInfo
 		w.Header().Set("Content-Type", "text/plain")
 		w.WriteHeader(http.StatusOK)
-		w.Write([]byte(guidHex))
 	}
 }

api/routes.go

@@ -4,13 +4,16 @@
 package api
 
 import (
-	"golang.org/x/time/rate"
+	"io"
 	"net/http"
 
-	"github.com/fido-device-onboard/go-fdo-server/api/handlers"
+	"golang.org/x/time/rate"
+
 	transport "github.com/fido-device-onboard/go-fdo/http"
 	"github.com/fido-device-onboard/go-fdo/protocol"
 	"github.com/fido-device-onboard/go-fdo/sqlite"
+
+	"github.com/fido-device-onboard/go-fdo-server/api/handlers"
 )
 
 // HTTPHandler handles HTTP requests
@@ -20,14 +23,27 @@ type HTTPHandler struct {
 	state   *sqlite.DB
 }
 
-func rateLimitMiddleware(limiter *rate.Limiter, next http.Handler) http.Handler {
-	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+func rateLimitMiddleware(limiter *rate.Limiter, next http.Handler) http.HandlerFunc {
+	return func(w http.ResponseWriter, r *http.Request) {
 		if !limiter.Allow() {
 			http.Error(w, "Too Many Requests", http.StatusTooManyRequests)
 			return
 		}
 		next.ServeHTTP(w, r)
-	})
+	}
+}
+
+func bodySizeMiddleware(limitBytes int64, next http.Handler) http.HandlerFunc {
+	return func(w http.ResponseWriter, r *http.Request) {
+		r.Body = struct {
+			io.Reader
+			io.Closer
+		}{
+			Reader: io.LimitReader(r.Body, limitBytes),
+			Closer: r.Body,
+		}
+		next.ServeHTTP(w, r)
+	}
 }
 
 // NewHTTPHandler creates a new HTTPHandler
@@ -37,25 +53,22 @@ func NewHTTPHandler(handler *transport.Handler, rvInfo *[][]protocol.RvInstructi
 
 // RegisterRoutes registers the routes for the HTTP server
 func (h *HTTPHandler) RegisterRoutes() *http.ServeMux {
-	handler := http.NewServeMux()
-	limiter := rate.NewLimiter(2, 10)
+	apiRouter := http.NewServeMux()
+	apiRouter.Handle("/rvinfo", handlers.RvInfoHandler(h.rvInfo))
+	apiRouter.HandleFunc("/owner/redirect", handlers.OwnerInfoHandler)
+	apiRouter.Handle("GET /to0/{guid}", handlers.To0Handler(h.rvInfo, h.state))
+	apiRouter.HandleFunc("GET /vouchers", handlers.GetVoucherHandler)
+	apiRouter.Handle("POST /owner/vouchers", handlers.InsertVoucherHandler(h.rvInfo))
+
+	apiHandler := rateLimitMiddleware(rate.NewLimiter(2, 10),
+		bodySizeMiddleware(1<<20, /* 1MB */
+			apiRouter,
+		),
+	)
 
+	handler := http.NewServeMux()
 	handler.Handle("POST /fdo/101/msg/{msg}", h.handler)
-	handler.HandleFunc("/api/v1/rvinfo", func(w http.ResponseWriter, r *http.Request) {
-		rateLimitMiddleware(limiter, http.HandlerFunc(handlers.RvInfoHandler(h.rvInfo))).ServeHTTP(w, r)
-	})
-	handler.HandleFunc("/api/v1/owner/redirect", func(w http.ResponseWriter, r *http.Request) {
-		rateLimitMiddleware(limiter, http.HandlerFunc(handlers.OwnerInfoHandler)).ServeHTTP(w, r)
-	})
-	handler.HandleFunc("/api/v1/to0/", func(w http.ResponseWriter, r *http.Request) {
-		rateLimitMiddleware(limiter, http.HandlerFunc(handlers.To0Handler(h.rvInfo, h.state))).ServeHTTP(w, r)
-	})
-	handler.HandleFunc("/api/v1/vouchers", func(w http.ResponseWriter, r *http.Request) {
-		rateLimitMiddleware(limiter, http.HandlerFunc(handlers.GetVoucherHandler)).ServeHTTP(w, r)
-	})
-	handler.HandleFunc("/api/v1/owner/vouchers", func(w http.ResponseWriter, r *http.Request) {
-		rateLimitMiddleware(limiter, http.HandlerFunc(handlers.InsertVoucherHandler(h.rvInfo))).ServeHTTP(w, r)
-	})
+	handler.Handle("/api/v1/", http.StripPrefix("/api/v1", apiHandler))
 	handler.HandleFunc("/health", handlers.HealthHandler)
 	return handler
 }

cmd/import_voucher.go

@@ -56,6 +56,7 @@ var importVoucherCmd = &cobra.Command{
 		if err != nil {
 			return fmt.Errorf("error parsing owner public key from voucher: %w", err)
 		}
+		// TODO: Allow importing vouchers with RSA 2048 manufacturer keys
 		ownerKey, _, err := state.OwnerKey(context.Background(), ov.Header.Val.ManufacturerKey.Type, 3072)
 		if err != nil {
 			return fmt.Errorf("error getting owner key: %w", err)