{storagenode,multinode/nodes}: use multinodeauth.Secret instead of []byte for APISecret

When enconding structs into JSON, byte slices are marshalled as base64
encoded string using the base64.StdEncoding.Encode():
ea9c3fd42d/src/encoding/json/encode.go (L833-L861)

We, however, expect API Secrets to be encoded as base64URL, so when
an marshalled secret (with byte slice type) is added to the multinode
dashboard, it fails with `illegal base64 data at input byte XX`.

This change changes the type of APISecret field in the
multinode/nodes.Nodes struct to use multinodeauth.Secret type instead
of []byte.
multinodeauth.Secret is extended with custom MarshalJSON and
UnmarshalJSON methods which implement the json.Marshaler and
json.Unmarshaler interfaces, respectively.

Resolves https://github.com/storj/storj/issues/4949

Change-Id: Ib14b5f49ceaac109620c25d7ff83be865c698343
This commit is contained in:
Clement Sam 2022-08-12 19:50:30 +00:00
parent a4192acabb
commit 7e5025cac0
14 changed files with 125 additions and 78 deletions

View File

@ -154,13 +154,6 @@ func cmdRun(cmd *cobra.Command, args []string) (err error) {
return errs.Combine(runError, closeError)
}
type nodeInfo struct {
NodeID storj.NodeID `json:"id"`
PublicAddress string `json:"publicAddress"`
APISecret string `json:"apiSecret"`
Name string `json:"name"`
}
func cmdAdd(cmd *cobra.Command, args []string) (err error) {
ctx, _ := process.Ctx(cmd)
log := zap.L()
@ -187,7 +180,7 @@ func cmdAdd(cmd *cobra.Command, args []string) (err error) {
dialer := rpc.NewDefaultDialer(tlsOptions)
var nodeList []nodeInfo
var nodeList []nodes.Node
hasRequiredFlags := addCfg.NodeID != "" && addCfg.APISecret != "" && addCfg.PublicAddress != ""
@ -200,54 +193,48 @@ func cmdAdd(cmd *cobra.Command, args []string) (err error) {
if err != nil {
return err
}
nodeList = []nodeInfo{
apiSecret, err := multinodeauth.SecretFromBase64(addCfg.APISecret)
if err != nil {
return err
}
nodeList = []nodes.Node{
{
NodeID: nodeID,
ID: nodeID,
PublicAddress: addCfg.PublicAddress,
APISecret: addCfg.APISecret,
APISecret: apiSecret,
Name: addCfg.Name,
},
}
} else {
path := args[0]
var nodesData []byte
var nodesJSONData []byte
if path == "-" {
stdin := cmd.InOrStdin()
data, err := ioutil.ReadAll(stdin)
if err != nil {
return err
}
nodesData = data
nodesJSONData = data
} else {
nodesData, err = os.ReadFile(path)
nodesJSONData, err = os.ReadFile(path)
if err != nil {
return err
}
}
nodeList, err = unmarshalJSONNodes(nodesData)
nodeList, err = unmarshalJSONNodes(nodesJSONData)
if err != nil {
return err
}
}
for _, node := range nodeList {
if _, err := db.Nodes().Get(ctx, node.NodeID); err == nil {
return errs.New("Node with ID %s is already added to the multinode dashboard", node.NodeID)
}
apiSecret, err := multinodeauth.SecretFromBase64(node.APISecret)
if err != nil {
return err
if _, err := db.Nodes().Get(ctx, node.ID); err == nil {
return errs.New("Node with ID %s is already added to the multinode dashboard", node.ID)
}
service := nodes.NewService(log, dialer, db.Nodes())
err = service.Add(ctx, nodes.Node{
ID: node.NodeID,
APISecret: apiSecret[:],
PublicAddress: node.PublicAddress,
Name: node.Name,
})
err = service.Add(ctx, node)
if err != nil {
return err
}
@ -256,26 +243,26 @@ func cmdAdd(cmd *cobra.Command, args []string) (err error) {
return nil
}
func unmarshalJSONNodes(nodesData []byte) ([]nodeInfo, error) {
var nodes []nodeInfo
nodesData = bytes.TrimLeft(nodesData, " \t\r\n")
func unmarshalJSONNodes(nodesJSONData []byte) ([]nodes.Node, error) {
var nodesInfo []nodes.Node
nodesJSONData = bytes.TrimLeft(nodesJSONData, " \t\r\n")
switch {
case len(nodesData) > 0 && nodesData[0] == '[': // data is json array
err := json.Unmarshal(nodesData, &nodes)
case len(nodesJSONData) > 0 && nodesJSONData[0] == '[': // data is json array
err := json.Unmarshal(nodesJSONData, &nodesInfo)
if err != nil {
return nil, err
}
case len(nodesData) > 0 && nodesData[0] == '{': // data is json object
var singleNode nodeInfo
err := json.Unmarshal(nodesData, &singleNode)
case len(nodesJSONData) > 0 && nodesJSONData[0] == '{': // data is json object
var singleNode nodes.Node
err := json.Unmarshal(nodesJSONData, &singleNode)
if err != nil {
return nil, err
}
nodes = []nodeInfo{singleNode}
nodesInfo = []nodes.Node{singleNode}
default:
return nil, errs.New("invalid JSON format")
}
return nodes, nil
return nodesInfo, nil
}

View File

@ -4,17 +4,23 @@
package main
import (
"encoding/base64"
"testing"
"github.com/stretchr/testify/require"
"storj.io/common/storj"
"storj.io/storj/multinode/nodes"
"storj.io/storj/private/multinodeauth"
)
func Test_unmarshalJSONNodes(t *testing.T) {
nodeID, err := storj.NodeIDFromString("1MJ7R1cqGrFnELPY3YKd62TBJ6vE8x9yPKPwUFHUx6G8oypezR")
require.NoError(t, err)
apiSecret, err := multinodeauth.SecretFromBase64("b_yeI0OBKBusBVN4_dHxpxlwdTyoFPwtEuHv9ACl9jI=")
require.NoError(t, err)
t.Run("valid json object", func(t *testing.T) {
nodesJSONData := `
{
@ -24,11 +30,11 @@ func Test_unmarshalJSONNodes(t *testing.T) {
"apiSecret": "b_yeI0OBKBusBVN4_dHxpxlwdTyoFPwtEuHv9ACl9jI="
}
`
expectedNodeInfo := []nodeInfo{
expectedNodeInfo := []nodes.Node{
{
NodeID: nodeID,
ID: nodeID,
PublicAddress: "awn7k09ts6mxbgau.myfritz.net:13010",
APISecret: "b_yeI0OBKBusBVN4_dHxpxlwdTyoFPwtEuHv9ACl9jI=",
APISecret: apiSecret,
Name: "Storagenode 1",
},
}
@ -50,11 +56,11 @@ func Test_unmarshalJSONNodes(t *testing.T) {
}
]
`
expectedNodeInfo := []nodeInfo{
expectedNodeInfo := []nodes.Node{
{
NodeID: nodeID,
ID: nodeID,
PublicAddress: "awn7k09ts6mxbgau.myfritz.net:13010",
APISecret: "b_yeI0OBKBusBVN4_dHxpxlwdTyoFPwtEuHv9ACl9jI=",
APISecret: apiSecret,
Name: "Storagenode 1",
},
}
@ -64,4 +70,34 @@ func Test_unmarshalJSONNodes(t *testing.T) {
require.Equal(t, expectedNodeInfo, got)
})
t.Run("invalid base64 input, expects base64url", func(t *testing.T) {
nodesJSONData := `
{
"name": "Storagenode 1",
"id":"1MJ7R1cqGrFnELPY3YKd62TBJ6vE8x9yPKPwUFHUx6G8oypezR",
"publicAddress": "awn7k09ts6mxbgau.myfritz.net:13010",
"apiSecret": "b/yeI0OBKBusBVN4/dHxpxlwdTyoFPwtEuHv9ACl9jI="
}
`
got, err := unmarshalJSONNodes([]byte(nodesJSONData))
require.Error(t, err)
require.ErrorIs(t, err, base64.CorruptInputError(1))
require.Nil(t, got)
})
t.Run("invalid secret", func(t *testing.T) {
nodesJSONData := `
{
"name": "Storagenode 1",
"id":"1MJ7R1cqGrFnELPY3YKd62TBJ6vE8x9yPKPwUFHUx6G8oypezR",
"publicAddress": "awn7k09ts6mxbgau.myfritz.net:13010",
"apiSecret": "b_yeI0OBKBusBVN4_dHxpxlwdTyoFPwtEuHv9ACl9jI-"
}
`
got, err := unmarshalJSONNodes([]byte(nodesJSONData))
require.Error(t, err)
require.Equal(t, "invalid secret", err.Error())
require.Nil(t, got)
})
}

View File

@ -385,10 +385,9 @@ func cmdInfo(cmd *cobra.Command, args []string) (err error) {
}
if nodeInfoCfg.JSON {
node := nodes.Node{
ID: identity.ID,
APISecret: apiKey.Secret[:],
APISecret: apiKey.Secret,
PublicAddress: nodeInfoCfg.Contact.ExternalAddress,
}

View File

@ -162,7 +162,7 @@ func (service *Service) getMonthlySatellite(ctx context.Context, node nodes.Node
bandwidthClient := multinodepb.NewDRPCBandwidthClient(conn)
header := &multinodepb.RequestHeader{
ApiKey: node.APISecret,
ApiKey: node.APISecret[:],
}
ingress, err := bandwidthClient.IngressSummarySatellite(ctx, &multinodepb.IngressSummarySatelliteRequest{
@ -240,7 +240,7 @@ func (service *Service) getMonthly(ctx context.Context, node nodes.Node) (_ Mont
bandwidthClient := multinodepb.NewDRPCBandwidthClient(conn)
header := &multinodepb.RequestHeader{
ApiKey: node.APISecret,
ApiKey: node.APISecret[:],
}
ingress, err := bandwidthClient.IngressSummary(ctx, &multinodepb.IngressSummaryRequest{

View File

@ -66,7 +66,7 @@ func (controller *Nodes) Add(w http.ResponseWriter, r *http.Request) {
return
}
if err = controller.service.Add(ctx, nodes.Node{ID: id, APISecret: apiSecret[:], PublicAddress: payload.PublicAddress}); err != nil {
if err = controller.service.Add(ctx, nodes.Node{ID: id, APISecret: apiSecret, PublicAddress: payload.PublicAddress}); err != nil {
switch {
case nodes.ErrNodeNotReachable.Has(err):
controller.serveError(w, http.StatusNotFound, ErrNodes.Wrap(err))

View File

@ -13,6 +13,7 @@ import (
"storj.io/common/storj"
"storj.io/storj/multinode/multinodedb/dbx"
"storj.io/storj/multinode/nodes"
"storj.io/storj/private/multinodeauth"
)
// ErrNodesDB indicates about internal NodesDB error.
@ -110,7 +111,7 @@ func (n *nodesdb) Add(ctx context.Context, node nodes.Node) (err error) {
dbx.Node_Id(node.ID.Bytes()),
dbx.Node_Name(node.Name),
dbx.Node_PublicAddress(node.PublicAddress),
dbx.Node_ApiSecret(node.APISecret),
dbx.Node_ApiSecret(node.APISecret[:]),
)
return ErrNodesDB.Wrap(err)
@ -145,9 +146,14 @@ func fromDBXNode(ctx context.Context, node *dbx.Node) (_ nodes.Node, err error)
return nodes.Node{}, err
}
secret, err := multinodeauth.SecretFromBytes(node.ApiSecret)
if err != nil {
return nodes.Node{}, err
}
result := nodes.Node{
ID: id,
APISecret: node.ApiSecret,
APISecret: secret,
Name: node.Name,
PublicAddress: node.PublicAddress,
}

View File

@ -10,6 +10,7 @@ import (
"github.com/zeebo/errs"
"storj.io/common/storj"
"storj.io/storj/private/multinodeauth"
)
// DB exposes needed by MND NodesDB functionality.
@ -41,9 +42,9 @@ var (
type Node struct {
ID storj.NodeID `json:"id"`
// APISecret is a secret issued by storagenode, that will be main auth mechanism in MND <-> SNO api.
APISecret []byte `json:"apiSecret"`
PublicAddress string `json:"publicAddress"`
Name string `json:"name"`
APISecret multinodeauth.Secret `json:"apiSecret"`
PublicAddress string `json:"publicAddress"`
Name string `json:"name"`
}
// Status represents node online status.

View File

@ -16,6 +16,7 @@ import (
"storj.io/storj/multinode"
"storj.io/storj/multinode/multinodedb/multinodedbtest"
"storj.io/storj/multinode/nodes"
"storj.io/storj/private/multinodeauth"
)
func TestNodesDB(t *testing.T) {
@ -23,7 +24,7 @@ func TestNodesDB(t *testing.T) {
nodesRepository := db.Nodes()
nodeID := testrand.NodeID()
apiSecret := []byte("secret")
apiSecret := multinodeauth.Secret{uint8(0)}
publicAddress := "228.13.38.1:8081"
err := nodesRepository.Add(ctx, nodes.Node{ID: nodeID, APISecret: apiSecret, PublicAddress: publicAddress})
@ -67,7 +68,7 @@ func TestNodesDB(t *testing.T) {
for i := 0; i < nodesAmount; i++ {
node := nodes.Node{
ID: testrand.NodeID(),
APISecret: []byte{uint8(i)},
APISecret: multinodeauth.Secret{uint8(i)},
PublicAddress: fmt.Sprintf("%d", i),
Name: fmt.Sprintf("%d", i),
}

View File

@ -65,7 +65,7 @@ func (service *Service) Add(ctx context.Context, node Node) (err error) {
nodeClient := multinodepb.NewDRPCNodeClient(conn)
header := &multinodepb.RequestHeader{
ApiKey: node.APISecret,
ApiKey: node.APISecret[:],
}
// making test request to check node api key.
@ -154,7 +154,7 @@ func (service *Service) ListInfos(ctx context.Context) (_ []NodeInfo, err error)
payoutClient := multinodepb.NewDRPCPayoutClient(conn)
header := &multinodepb.RequestHeader{
ApiKey: node.APISecret,
ApiKey: node.APISecret[:],
}
nodeVersion, err := nodeClient.Version(ctx, &multinodepb.VersionRequest{Header: header})
@ -250,7 +250,7 @@ func (service *Service) ListInfosSatellite(ctx context.Context, satelliteID stor
payoutClient := multinodepb.NewDRPCPayoutClient(conn)
header := &multinodepb.RequestHeader{
ApiKey: node.APISecret,
ApiKey: node.APISecret[:],
}
nodeVersion, err := nodeClient.Version(ctx, &multinodepb.VersionRequest{Header: header})
@ -349,7 +349,7 @@ func (service *Service) trustedSatellites(ctx context.Context, node Node) (_ sto
nodeClient := multinodepb.NewDRPCNodeClient(conn)
header := &multinodepb.RequestHeader{
ApiKey: node.APISecret,
ApiKey: node.APISecret[:],
}
resp, err := nodeClient.TrustedSatellites(ctx, &multinodepb.TrustedSatellitesRequest{Header: header})

View File

@ -104,7 +104,7 @@ func (service *Service) GetOperator(ctx context.Context, node nodes.Node) (_ Ope
nodeClient := multinodepb.NewDRPCNodeClient(conn)
payoutClient := multinodepb.NewDRPCPayoutClient(conn)
header := &multinodepb.RequestHeader{
ApiKey: node.APISecret,
ApiKey: node.APISecret[:],
}
operatorResponse, err := nodeClient.Operator(ctx, &multinodepb.OperatorRequest{Header: header})

View File

@ -241,7 +241,7 @@ func (service *Service) summarySatellite(ctx context.Context, node nodes.Node, s
payoutClient := multinodepb.NewDRPCPayoutClient(conn)
header := &multinodepb.RequestHeader{
ApiKey: node.APISecret,
ApiKey: node.APISecret[:],
}
response, err := payoutClient.SatelliteSummary(ctx, &multinodepb.SatelliteSummaryRequest{Header: header, SatelliteId: satelliteID})
@ -268,7 +268,7 @@ func (service *Service) summarySatellitePeriod(ctx context.Context, node nodes.N
payoutClient := multinodepb.NewDRPCPayoutClient(conn)
header := &multinodepb.RequestHeader{
ApiKey: node.APISecret,
ApiKey: node.APISecret[:],
}
response, err := payoutClient.SatellitePeriodSummary(ctx, &multinodepb.SatellitePeriodSummaryRequest{Header: header, SatelliteId: satelliteID, Period: period})
@ -295,7 +295,7 @@ func (service *Service) summaryPeriod(ctx context.Context, node nodes.Node, peri
payoutClient := multinodepb.NewDRPCPayoutClient(conn)
header := &multinodepb.RequestHeader{
ApiKey: node.APISecret,
ApiKey: node.APISecret[:],
}
response, err := payoutClient.AllSatellitesPeriodSummary(ctx, &multinodepb.AllSatellitesPeriodSummaryRequest{Header: header, Period: period})
@ -322,7 +322,7 @@ func (service *Service) summary(ctx context.Context, node nodes.Node) (info *mul
payoutClient := multinodepb.NewDRPCPayoutClient(conn)
header := &multinodepb.RequestHeader{
ApiKey: node.APISecret,
ApiKey: node.APISecret[:],
}
response, err := payoutClient.AllSatellitesSummary(ctx, &multinodepb.AllSatellitesSummaryRequest{Header: header})
@ -401,7 +401,7 @@ func (service *Service) HeldAmountSummary(ctx context.Context, nodeID storj.Node
nodeClient := multinodepb.NewDRPCNodeClient(conn)
header := &multinodepb.RequestHeader{
ApiKey: node.APISecret,
ApiKey: node.APISecret[:],
}
trusted, err := nodeClient.TrustedSatellites(ctx, &multinodepb.TrustedSatellitesRequest{
Header: header,
@ -459,7 +459,7 @@ func (service *Service) heldAmountHistory(ctx context.Context, node nodes.Node,
payoutClient := multinodepb.NewDRPCPayoutsClient(conn)
header := &multinodepb.RequestHeader{
ApiKey: node.APISecret,
ApiKey: node.APISecret[:],
}
resp, err := payoutClient.HeldAmountHistory(ctx, &multinodepb.HeldAmountHistoryRequest{
Header: header,
@ -504,7 +504,7 @@ func (service *Service) nodeExpectations(ctx context.Context, node nodes.Node) (
payoutClient := multinodepb.NewDRPCPayoutClient(conn)
header := &multinodepb.RequestHeader{
ApiKey: node.APISecret,
ApiKey: node.APISecret[:],
}
estimated, err := payoutClient.EstimatedPayoutTotal(ctx, &multinodepb.EstimatedPayoutTotalRequest{Header: header})
@ -536,7 +536,7 @@ func (service *Service) earned(ctx context.Context, node nodes.Node) (_ int64, e
payoutClient := multinodepb.NewDRPCPayoutClient(conn)
header := &multinodepb.RequestHeader{
ApiKey: node.APISecret,
ApiKey: node.APISecret[:],
}
amount, err := payoutClient.Earned(ctx, &multinodepb.EarnedRequest{Header: header})
@ -563,7 +563,7 @@ func (service *Service) earnedSatellite(ctx context.Context, node nodes.Node) (_
payoutClient := multinodepb.NewDRPCPayoutClient(conn)
header := &multinodepb.RequestHeader{
ApiKey: node.APISecret,
ApiKey: node.APISecret[:],
}
response, err := payoutClient.EarnedPerSatellite(ctx, &multinodepb.EarnedPerSatelliteRequest{Header: header})
@ -597,7 +597,7 @@ func (service *Service) PaystubSatellitePeriod(ctx context.Context, period strin
payoutClient := multinodepb.NewDRPCPayoutClient(conn)
header := &multinodepb.RequestHeader{
ApiKey: node.APISecret,
ApiKey: node.APISecret[:],
}
response, err := payoutClient.SatellitePeriodPaystub(ctx, &multinodepb.SatellitePeriodPaystubRequest{
@ -648,7 +648,7 @@ func (service *Service) PaystubPeriod(ctx context.Context, period string, nodeID
payoutClient := multinodepb.NewDRPCPayoutClient(conn)
header := &multinodepb.RequestHeader{
ApiKey: node.APISecret,
ApiKey: node.APISecret[:],
}
response, err := payoutClient.PeriodPaystub(ctx, &multinodepb.PeriodPaystubRequest{
@ -698,7 +698,7 @@ func (service *Service) PaystubSatellite(ctx context.Context, nodeID, satelliteI
payoutClient := multinodepb.NewDRPCPayoutClient(conn)
header := &multinodepb.RequestHeader{
ApiKey: node.APISecret,
ApiKey: node.APISecret[:],
}
response, err := payoutClient.SatellitePaystub(ctx, &multinodepb.SatellitePaystubRequest{
@ -748,7 +748,7 @@ func (service *Service) Paystub(ctx context.Context, nodeID storj.NodeID) (_ Pay
payoutClient := multinodepb.NewDRPCPayoutClient(conn)
header := &multinodepb.RequestHeader{
ApiKey: node.APISecret,
ApiKey: node.APISecret[:],
}
response, err := payoutClient.Paystub(ctx, &multinodepb.PaystubRequest{

View File

@ -92,7 +92,7 @@ func (service *Service) dialStats(ctx context.Context, node nodes.Node, satellit
req := &multinodepb.ReputationRequest{
Header: &multinodepb.RequestHeader{
ApiKey: node.APISecret,
ApiKey: node.APISecret[:],
},
SatelliteId: satelliteID,
}

View File

@ -197,7 +197,7 @@ func (service *Service) dialDiskSpace(ctx context.Context, node nodes.Node) (dis
diskSpaceResponse, err := storageClient.DiskSpace(ctx, &multinodepb.DiskSpaceRequest{
Header: &multinodepb.RequestHeader{
ApiKey: node.APISecret,
ApiKey: node.APISecret[:],
},
})
if err != nil {
@ -233,7 +233,7 @@ func (service *Service) dialUsage(ctx context.Context, node nodes.Node, from, to
req := &multinodepb.StorageUsageRequest{
Header: &multinodepb.RequestHeader{
ApiKey: node.APISecret,
ApiKey: node.APISecret[:],
},
From: from,
To: to,
@ -276,7 +276,7 @@ func (service *Service) dialUsageSatellite(ctx context.Context, node nodes.Node,
req := &multinodepb.StorageUsageSatelliteRequest{
Header: &multinodepb.RequestHeader{
ApiKey: node.APISecret,
ApiKey: node.APISecret[:],
},
SatelliteId: satelliteID,
From: from,

View File

@ -7,6 +7,7 @@ import (
"bytes"
"crypto/rand"
"encoding/base64"
"encoding/json"
"github.com/zeebo/errs"
)
@ -39,6 +40,22 @@ func (secret Secret) IsZero() bool {
return bytes.Equal(secret[:], zero[:])
}
// MarshalJSON implements json.Marshaler Interface.
func (secret Secret) MarshalJSON() ([]byte, error) {
return json.Marshal(secret.String())
}
// UnmarshalJSON implements json.Unmarshaler Interface.
func (secret *Secret) UnmarshalJSON(data []byte) error {
var err error
var s string
if err := json.Unmarshal(data, &s); err != nil {
return err
}
*secret, err = SecretFromBase64(s)
return err
}
// SecretFromBase64 creates new secret from base64 string.
func SecretFromBase64(s string) (Secret, error) {
b, err := base64.URLEncoding.DecodeString(s)