Skip to content

Commit

Permalink
Start refactoring for new linter rules (#39)
Browse files Browse the repository at this point in the history
refactor: refactoring to match new linter rules according to #38

Not everything is fixed, just a reference what can and needs to be improved.

* implement grantee management

* Add POST endpoint + fixes

* Save grantees as pubkey list and fix remove error; CHG: act-handler logger names

* Refactor: pass getter, putter to controller functions

* Refactor: error handling in dynamicaccess; Read cache header only for download handlers

* CHG: grantees ref is encrypted and added to history ref + tests

* Fix nil pointer dereference panic

* CHG: put actref in handlegrantees; Add: pin, tag,deferred headers

* CHG: pass loadsave to handlers; check if history address is nil

* FIX: re-init history so that it can be saved; only add publisher if histroy is zero

* make act timestamp optional

* fix revoke grantees

* Fix: Act timestamp header nil check; Uploadhandler UT

* refactor: start refactoring for now linter rules

* refactor: revert non ACT related files

* CHG: accesslogic getkeys refactor

* refactor: fix errcheck and ineffassign linter errors in most cases

* refactor: add headers, and change error handling

* refactor: add headers

---------

Co-authored-by: Kexort <[email protected]>
Co-authored-by: Bálint Ujvári <[email protected]>
  • Loading branch information
3 people authored May 16, 2024
1 parent 85c041a commit b87624d
Show file tree
Hide file tree
Showing 19 changed files with 231 additions and 174 deletions.
12 changes: 8 additions & 4 deletions pkg/api/dynamicaccess.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
// Copyright 2024 The Swarm Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

package api

import (
Expand All @@ -6,6 +10,7 @@ import (
"encoding/hex"
"encoding/json"
"errors"
"fmt"
"io"
"net/http"
"time"
Expand Down Expand Up @@ -114,7 +119,6 @@ func (s *Service) actDecryptionHandler() func(h http.Handler) http.Handler {
h.ServeHTTP(w, r.WithContext(setAddressInContext(ctx, reference)))
})
}

}

// actEncryptionHandler is a middleware that encrypts the given address using the publisher's public key
Expand All @@ -133,7 +137,7 @@ func (s *Service) actEncryptionHandler(
if err != nil {
logger.Debug("act failed to encrypt reference", "error", err)
logger.Error(nil, "act failed to encrypt reference")
return swarm.ZeroAddress, err
return swarm.ZeroAddress, fmt.Errorf("act failed to encrypt reference: %w", err)
}
// only need to upload history and kvs if a new history is created,
// meaning that the publsher uploaded to the history for the first time
Expand All @@ -142,13 +146,13 @@ func (s *Service) actEncryptionHandler(
if err != nil {
logger.Debug("done split keyvaluestore failed", "error", err)
logger.Error(nil, "done split keyvaluestore failed")
return swarm.ZeroAddress, err
return swarm.ZeroAddress, fmt.Errorf("done split keyvaluestore failed: %w", err)
}
err = putter.Done(historyReference)
if err != nil {
logger.Debug("done split history failed", "error", err)
logger.Error(nil, "done split history failed")
return swarm.ZeroAddress, err
return swarm.ZeroAddress, fmt.Errorf("done split history failed: %w", err)
}
}

Expand Down
48 changes: 21 additions & 27 deletions pkg/api/dynamicaccess_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,23 +40,23 @@ func prepareHistoryFixture(storer api.Storer) (dynamicaccess.History, swarm.Addr

testActRef1 := swarm.NewAddress([]byte("39a5ea87b141fe44aa609c3327ecd891"))
firstTime := time.Date(1994, time.April, 1, 0, 0, 0, 0, time.UTC).Unix()
h.Add(ctx, testActRef1, &firstTime, nil)
_ = h.Add(ctx, testActRef1, &firstTime, nil)

testActRef2 := swarm.NewAddress([]byte("39a5ea87b141fe44aa609c3327ecd892"))
secondTime := time.Date(2000, time.April, 1, 0, 0, 0, 0, time.UTC).Unix()
h.Add(ctx, testActRef2, &secondTime, nil)
_ = h.Add(ctx, testActRef2, &secondTime, nil)

testActRef3 := swarm.NewAddress([]byte("39a5ea87b141fe44aa609c3327ecd893"))
thirdTime := time.Date(2015, time.April, 1, 0, 0, 0, 0, time.UTC).Unix()
h.Add(ctx, testActRef3, &thirdTime, nil)
_ = h.Add(ctx, testActRef3, &thirdTime, nil)

testActRef4 := swarm.NewAddress([]byte("39a5ea87b141fe44aa609c3327ecd894"))
fourthTime := time.Date(2020, time.April, 1, 0, 0, 0, 0, time.UTC).Unix()
h.Add(ctx, testActRef4, &fourthTime, nil)
_ = h.Add(ctx, testActRef4, &fourthTime, nil)

testActRef5 := swarm.NewAddress([]byte("39a5ea87b141fe44aa609c3327ecd895"))
fifthTime := time.Date(2030, time.April, 1, 0, 0, 0, 0, time.UTC).Unix()
h.Add(ctx, testActRef5, &fifthTime, nil)
_ = h.Add(ctx, testActRef5, &fifthTime, nil)

ref, _ := h.Store(ctx)
return h, ref
Expand Down Expand Up @@ -221,10 +221,11 @@ func TestDacEachEndpointWithAct(t *testing.T) {
}
}

// nolint:paralleltest,tparallel
// TestDacWithoutActHeader [negative tests]:
// 1. upload w/ "Swarm-Act" header then try to dowload w/o the header.
// 2. upload w/o "Swarm-Act" header then try to dowload w/ the header.
//
//nolint:paralleltest,tparallel
func TestDacWithoutAct(t *testing.T) {
t.Parallel()
var (
Expand Down Expand Up @@ -321,8 +322,9 @@ func TestDacWithoutAct(t *testing.T) {
})
}

// nolint:paralleltest,tparallel
// TestDacInvalidPath [negative test]: Expect Bad request when the path address is invalid.
//
//nolint:paralleltest,tparallel
func TestDacInvalidPath(t *testing.T) {
t.Parallel()
var (
Expand All @@ -345,9 +347,7 @@ func TestDacInvalidPath(t *testing.T) {
PublicKey: pk.PublicKey,
Dac: mockdac.New(),
})
var (
encryptedRef = "asd"
)
encryptedRef := "asd"

jsonhttptest.Request(t, client, http.MethodGet, fileDownloadResource(encryptedRef), http.StatusBadRequest,
jsonhttptest.WithRequestHeader(api.SwarmActTimestampHeader, strconv.FormatInt(now, 10)),
Expand All @@ -361,7 +361,8 @@ func TestDacInvalidPath(t *testing.T) {
Field: "address",
Error: api.HexInvalidByteError('s').Error(),
},
}}),
},
}),
jsonhttptest.WithRequestHeader(api.ContentTypeHeader, "text/html; charset=utf-8"),
)
})
Expand All @@ -388,7 +389,6 @@ func TestDacHistory(t *testing.T) {
fileName = "sample.html"
now = time.Now().Unix()
)
fmt.Printf("bagoy now: %d\n", now)

t.Run("empty-history-upload-then-download-and-check-data", func(t *testing.T) {
client, _, _, _ := newTestServer(t, testServerOptions{
Expand Down Expand Up @@ -516,9 +516,7 @@ func TestDacHistory(t *testing.T) {
PublicKey: pk.PublicKey,
Dac: mockdac.New(),
})
var (
testfile = "testfile1"
)
testfile := "testfile1"

jsonhttptest.Request(t, client, http.MethodPost, fileUploadResource+"?name="+fileName, http.StatusInternalServerError,
jsonhttptest.WithRequestHeader(api.SwarmActHeader, "true"),
Expand All @@ -541,9 +539,7 @@ func TestDacHistory(t *testing.T) {
PublicKey: pk.PublicKey,
Dac: mockdac.New(mockdac.WithHistory(h, fixtureHref.String())),
})
var (
encryptedRef = "a5df670544eaea29e61b19d8739faa4573b19e4426e58a173e51ed0b5e7e2ade"
)
encryptedRef := "a5df670544eaea29e61b19d8739faa4573b19e4426e58a173e51ed0b5e7e2ade"

jsonhttptest.Request(t, client, http.MethodGet, fileDownloadResource(encryptedRef), http.StatusNotFound,
jsonhttptest.WithRequestHeader(api.SwarmActTimestampHeader, strconv.FormatInt(now, 10)),
Expand Down Expand Up @@ -619,9 +615,7 @@ func TestDacTimestamp(t *testing.T) {
})

t.Run("download-w/o-timestamp", func(t *testing.T) {
var (
encryptedRef = "a5df670544eaea29e61b19d8739faa4573b19e4426e58a173e51ed0b5e7e2ade"
)
encryptedRef := "a5df670544eaea29e61b19d8739faa4573b19e4426e58a173e51ed0b5e7e2ade"
client, _, _, _ := newTestServer(t, testServerOptions{
Storer: storerMock,
Logger: logger,
Expand Down Expand Up @@ -754,7 +748,8 @@ func TestDacPublisher(t *testing.T) {
Field: "Swarm-Act-Publisher",
Error: "malformed public key: invalid length: 32",
},
}}),
},
}),
jsonhttptest.WithRequestHeader(api.ContentTypeHeader, "text/html; charset=utf-8"),
)
})
Expand Down Expand Up @@ -785,9 +780,7 @@ func TestDacPublisher(t *testing.T) {
})

t.Run("download-w/o-publisher", func(t *testing.T) {
var (
encryptedRef = "a5df670544eaea29e61b19d8739faa4573b19e4426e58a173e51ed0b5e7e2ade"
)
encryptedRef := "a5df670544eaea29e61b19d8739faa4573b19e4426e58a173e51ed0b5e7e2ade"
client, _, _, _ := newTestServer(t, testServerOptions{
Storer: storerMock,
Logger: logger,
Expand Down Expand Up @@ -862,7 +855,8 @@ func TestDacGrantees(t *testing.T) {
Field: "address",
Error: api.HexInvalidByteError('s').Error(),
},
}}),
},
}),
)
})
t.Run("add-revoke-grantees", func(t *testing.T) {
Expand Down Expand Up @@ -904,8 +898,8 @@ func TestDacGrantees(t *testing.T) {
jsonhttptest.WithRequestHeader(api.SwarmPostageBatchIdHeader, batchOkStr),
jsonhttptest.WithJSONRequestBody(body),
)

})

t.Run("create-granteelist", func(t *testing.T) {
body := api.GranteesPostRequest{
GranteeList: []string{
Expand Down
65 changes: 36 additions & 29 deletions pkg/dynamicaccess/accesslogic.go
Original file line number Diff line number Diff line change
@@ -1,34 +1,42 @@
// Copyright 2024 The Swarm Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

package dynamicaccess

import (
"context"
"crypto/ecdsa"
"fmt"

encryption "github.com/ethersphere/bee/v2/pkg/encryption"
"github.com/ethersphere/bee/v2/pkg/kvs"
"github.com/ethersphere/bee/v2/pkg/swarm"
"golang.org/x/crypto/sha3"
)

var hashFunc = sha3.NewLegacyKeccak256
var oneByteArray = []byte{1}
var zeroByteArray = []byte{0}
//nolint:gochecknoglobals
var (
hashFunc = sha3.NewLegacyKeccak256
oneByteArray = []byte{1}
zeroByteArray = []byte{0}
)

// Read-only interface for the ACT
// Decryptor is a read-only interface for the ACT.
type Decryptor interface {
// DecryptRef will return a decrypted reference, for given encrypted reference and grantee
DecryptRef(ctx context.Context, storage kvs.KeyValueStore, encryptedRef swarm.Address, publisher *ecdsa.PublicKey) (swarm.Address, error)
// Embedding the Session interface
Session
}

// Control interface for the ACT (does write operations)
// Control interface for the ACT (does write operations).
type Control interface {
// Embedding the Decryptor interface
Decryptor
// Adds a new grantee to the ACT
// AddGrantee adds a new grantee to the ACT
AddGrantee(ctx context.Context, storage kvs.KeyValueStore, publisherPubKey, granteePubKey *ecdsa.PublicKey, accessKey *encryption.Key) error
// Encrypts a Swarm reference for a given grantee
// EncryptRef encrypts a Swarm reference for a given grantee
EncryptRef(ctx context.Context, storage kvs.KeyValueStore, grantee *ecdsa.PublicKey, ref swarm.Address) (swarm.Address, error)
}

Expand All @@ -38,14 +46,14 @@ type ActLogic struct {

var _ Control = (*ActLogic)(nil)

// Adds a new publisher to an empty act
// AddPublisher adds a new publisher to an empty act.
func (al ActLogic) AddPublisher(ctx context.Context, storage kvs.KeyValueStore, publisher *ecdsa.PublicKey) error {
accessKey := encryption.GenerateRandomKey(encryption.KeyLength)

return al.AddGrantee(ctx, storage, publisher, publisher, &accessKey)
}

// Encrypts a SWARM reference for a publisher
// EncryptRef encrypts a SWARM reference for a publisher.
func (al ActLogic) EncryptRef(ctx context.Context, storage kvs.KeyValueStore, publisherPubKey *ecdsa.PublicKey, ref swarm.Address) (swarm.Address, error) {
accessKey, err := al.getAccessKey(ctx, storage, publisherPubKey)
if err != nil {
Expand All @@ -54,13 +62,13 @@ func (al ActLogic) EncryptRef(ctx context.Context, storage kvs.KeyValueStore, pu
refCipher := encryption.New(accessKey, 0, uint32(0), hashFunc)
encryptedRef, err := refCipher.Encrypt(ref.Bytes())
if err != nil {
return swarm.ZeroAddress, err
return swarm.ZeroAddress, fmt.Errorf("failed to encrypt reference: %w", err)
}

return swarm.NewAddress(encryptedRef), nil
}

// Adds a new grantee to the ACT
// AddGrantee adds a new grantee to the ACT.
func (al ActLogic) AddGrantee(ctx context.Context, storage kvs.KeyValueStore, publisherPubKey, granteePubKey *ecdsa.PublicKey, accessKeyPointer *encryption.Key) error {
var (
accessKey encryption.Key
Expand All @@ -79,61 +87,60 @@ func (al ActLogic) AddGrantee(ctx context.Context, storage kvs.KeyValueStore, pu
}

// Encrypt the access key for the new Grantee
keys, err := al.getKeys(granteePubKey)
lookupKey, accessKeyDecryptionKey, err := al.getKeys(granteePubKey)
if err != nil {
return err
}
lookupKey := keys[0]
// accessKeyDecryptionKey is used for encryption of the access key
accessKeyDecryptionKey := keys[1]

// Encrypt the access key for the new Grantee
cipher := encryption.New(encryption.Key(accessKeyDecryptionKey), 0, uint32(0), hashFunc)
granteeEncryptedAccessKey, err := cipher.Encrypt(accessKey)
if err != nil {
return err
return fmt.Errorf("failed to encrypt access key: %w", err)
}

// Add the new encrypted access key for the Act
// Add the new encrypted access key to the Act
return storage.Put(ctx, lookupKey, granteeEncryptedAccessKey)
}

// Will return the access key for a publisher (public key)
// Will return the access key for a publisher (public key).
func (al *ActLogic) getAccessKey(ctx context.Context, storage kvs.KeyValueStore, publisherPubKey *ecdsa.PublicKey) ([]byte, error) {
keys, err := al.getKeys(publisherPubKey)
publisherLookupKey, publisherAKDecryptionKey, err := al.getKeys(publisherPubKey)
if err != nil {
return nil, err
}
publisherLookupKey := keys[0]
publisherAKDecryptionKey := keys[1]
// no need to constructor call if value not found in act
// no need for constructor call if value not found in act
accessKeyDecryptionCipher := encryption.New(encryption.Key(publisherAKDecryptionKey), 0, uint32(0), hashFunc)
encryptedAK, err := storage.Get(ctx, publisherLookupKey)
if err != nil {
return nil, err
return nil, fmt.Errorf("failed go get value from KVS: %w", err)
}

return accessKeyDecryptionCipher.Decrypt(encryptedAK)
}

// Generate lookup key and access key decryption key for a given public key
func (al *ActLogic) getKeys(publicKey *ecdsa.PublicKey) ([][]byte, error) {
return al.Session.Key(publicKey, [][]byte{zeroByteArray, oneByteArray})
func (al *ActLogic) getKeys(publicKey *ecdsa.PublicKey) ([]byte, []byte, error) {
nonces := [][]byte{zeroByteArray, oneByteArray}
// keys := make([][]byte, 0, len(nonces))
keys, err := al.Session.Key(publicKey, nonces)
if keys == nil {
return nil, nil, err
}
return keys[0], keys[1], err
}

// DecryptRef will return a decrypted reference, for given encrypted reference and publisher
func (al ActLogic) DecryptRef(ctx context.Context, storage kvs.KeyValueStore, encryptedRef swarm.Address, publisher *ecdsa.PublicKey) (swarm.Address, error) {
keys, err := al.getKeys(publisher)
lookupKey, accessKeyDecryptionKey, err := al.getKeys(publisher)
if err != nil {
return swarm.ZeroAddress, err
}
lookupKey := keys[0]
accessKeyDecryptionKey := keys[1]

// Lookup encrypted access key from the ACT manifest
encryptedAccessKey, err := storage.Get(ctx, lookupKey)
if err != nil {
return swarm.ZeroAddress, err
return swarm.ZeroAddress, fmt.Errorf("failed to get access key from KVS: %w", err)
}

// Decrypt access key
Expand Down
Loading

0 comments on commit b87624d

Please sign in to comment.