-
Notifications
You must be signed in to change notification settings - Fork 176
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Relay authentication #911
Relay authentication #911
Changes from 28 commits
02e9069
dc39c21
d99fd39
e39eaf9
876ec69
9500f4f
ffe18c6
79d9614
c1d83e6
ed4daca
3438b92
b5dc37c
b420cca
517b78e
7982aec
366fdf7
f2c10e4
0f95ce4
c2bb9c3
151305b
29fd940
3993af4
d094b80
d8691e1
8b9512b
fb7ec51
6325f0c
5f853e0
f22544c
7a26c27
d21e0bc
98a8469
5948e73
cc62eac
0372ee9
97cc318
fea251a
fe82924
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,7 +34,7 @@ message GetChunksRequest { | |
|
||
// If this is an authenticated request, this should hold the ID of the requester. If this | ||
// is an unauthenticated request, this field should be empty. | ||
uint64 requester_id = 2; | ||
bytes requester_id = 2; | ||
|
||
// If this is an authenticated request, this field will hold a signature by the requester | ||
// on the chunks being requested. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Document how this signature should be computed There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,159 @@ | ||
package auth | ||
|
||
import ( | ||
"context" | ||
"errors" | ||
"fmt" | ||
pb "github.com/Layr-Labs/eigenda/api/grpc/relay" | ||
"github.com/Layr-Labs/eigenda/core" | ||
"sync" | ||
"time" | ||
) | ||
|
||
// RequestAuthenticator authenticates requests to the relay service. This object is thread safe. | ||
type RequestAuthenticator interface { | ||
// AuthenticateGetChunksRequest authenticates a GetChunksRequest, returning an error if the request is invalid. | ||
// The address is the address of the peer that sent the request. This may be used to cache auth results | ||
// in order to save server resources. | ||
AuthenticateGetChunksRequest( | ||
address string, | ||
request *pb.GetChunksRequest, | ||
now time.Time) error | ||
} | ||
|
||
// authenticationTimeout is used to track the expiration of an auth. | ||
type authenticationTimeout struct { | ||
clientID string | ||
expiration time.Time | ||
} | ||
|
||
var _ RequestAuthenticator = &requestAuthenticator{} | ||
|
||
type requestAuthenticator struct { | ||
ics core.IndexedChainState | ||
|
||
// authenticatedClients is a set of client IDs that have been recently authenticated. | ||
authenticatedClients map[string]struct{} | ||
|
||
// authenticationTimeouts is a list of authentications that have been performed, along with their expiration times. | ||
authenticationTimeouts []*authenticationTimeout | ||
|
||
// authenticationTimeoutDuration is the duration for which an auth is valid. | ||
// If this is zero, then auth saving is disabled, and each request will be authenticated independently. | ||
authenticationTimeoutDuration time.Duration | ||
|
||
// savedAuthLock is used for thread safe atomic modification of the authenticatedClients map and the | ||
// authenticationTimeouts queue. | ||
savedAuthLock sync.Mutex | ||
} | ||
|
||
// NewRequestAuthenticator creates a new RequestAuthenticator. | ||
func NewRequestAuthenticator( | ||
ics core.IndexedChainState, | ||
authenticationTimeoutDuration time.Duration) RequestAuthenticator { | ||
|
||
return &requestAuthenticator{ | ||
ics: ics, | ||
authenticatedClients: make(map[string]struct{}), | ||
authenticationTimeouts: make([]*authenticationTimeout, 0), | ||
authenticationTimeoutDuration: authenticationTimeoutDuration, | ||
} | ||
} | ||
|
||
func (a *requestAuthenticator) AuthenticateGetChunksRequest( | ||
address string, | ||
ian-shim marked this conversation as resolved.
Show resolved
Hide resolved
|
||
request *pb.GetChunksRequest, | ||
now time.Time) error { | ||
|
||
if a == nil { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How does this compare to check the authenticator from outside? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The way I have it here is wrong (it didn't pass unit tests). I removed this check and instead now make it in the outside context. |
||
// do not enforce auth if the authenticator is nil | ||
return nil | ||
} | ||
|
||
if a.isAuthenticationStillValid(now, address) { | ||
// We've recently authenticated this client. Do not authenticate again for a while. | ||
return nil | ||
} | ||
|
||
blockNumber, err := a.ics.GetCurrentBlockNumber() | ||
if err != nil { | ||
return fmt.Errorf("failed to get current block number: %w", err) | ||
} | ||
operators, err := a.ics.GetIndexedOperators(context.Background(), blockNumber) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are 2 RPC calls here, it'd be helpful to cache the operator state There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. caching added |
||
if err != nil { | ||
return fmt.Errorf("failed to get operators: %w", err) | ||
} | ||
|
||
operatorID := core.OperatorID(request.RequesterId) | ||
operator, ok := operators[operatorID] | ||
if !ok { | ||
return errors.New("operator not found") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It'll be helpful to include the block number in error message There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. added |
||
} | ||
key := operator.PubkeyG2 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd cache There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. This will break if we ever allow operators to change keys, but that's a bridge we can cross in the future. |
||
|
||
g1Point, err := (&core.G1Point{}).Deserialize(request.RequesterSignature) | ||
if err != nil { | ||
return fmt.Errorf("failed to deserialize signature: %w", err) | ||
} | ||
|
||
signature := core.Signature{ | ||
G1Point: g1Point, | ||
} | ||
|
||
hash := HashGetChunksRequest(request) | ||
isValid := signature.Verify(key, ([32]byte)(hash)) | ||
|
||
if !isValid { | ||
return errors.New("signature verification failed") | ||
} | ||
|
||
a.saveAuthenticationResult(now, address) | ||
return nil | ||
} | ||
|
||
// saveAuthenticationResult saves the result of an auth. | ||
func (a *requestAuthenticator) saveAuthenticationResult(now time.Time, address string) { | ||
if a.authenticationTimeoutDuration == 0 { | ||
// Authentication saving is disabled. | ||
return | ||
} | ||
|
||
a.savedAuthLock.Lock() | ||
defer a.savedAuthLock.Unlock() | ||
|
||
a.authenticatedClients[address] = struct{}{} | ||
a.authenticationTimeouts = append(a.authenticationTimeouts, | ||
&authenticationTimeout{ | ||
clientID: address, | ||
expiration: now.Add(a.authenticationTimeoutDuration), | ||
}) | ||
} | ||
|
||
// isAuthenticationStillValid returns true if the client at the given address has been authenticated recently. | ||
func (a *requestAuthenticator) isAuthenticationStillValid(now time.Time, address string) bool { | ||
if a.authenticationTimeoutDuration == 0 { | ||
// Authentication saving is disabled. | ||
return false | ||
} | ||
|
||
a.savedAuthLock.Lock() | ||
defer a.savedAuthLock.Unlock() | ||
|
||
a.removeOldAuthentications(now) | ||
_, ok := a.authenticatedClients[address] | ||
return ok | ||
} | ||
|
||
// removeOldAuthentications removes any authentications that have expired. | ||
ian-shim marked this conversation as resolved.
Show resolved
Hide resolved
|
||
func (a *requestAuthenticator) removeOldAuthentications(now time.Time) { | ||
index := 0 | ||
for ; index < len(a.authenticationTimeouts); index++ { | ||
if a.authenticationTimeouts[index].expiration.After(now) { | ||
break | ||
} | ||
delete(a.authenticatedClients, a.authenticationTimeouts[index].clientID) | ||
} | ||
if index > 0 { | ||
a.authenticationTimeouts = a.authenticationTimeouts[index:] | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From the code below, this is interpreted as operator ID. It'll be helpful to document it here so the users know how to set it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've renamed this field to
operator_id
. Is that sufficient, or do you think I should add additional documentation?