-
Notifications
You must be signed in to change notification settings - Fork 202
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
Add basic authentication classes #166
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
package core | ||
|
||
type BlobRequestAuthenticator interface { | ||
AuthenticateBlobRequest(header BlobAuthHeader) error | ||
} | ||
|
||
type BlobRequestSigner interface { | ||
SignBlobRequest(header BlobAuthHeader) ([]byte, error) | ||
GetAccountID() string | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,67 @@ | ||
package auth_test | ||
|
||
import ( | ||
"math/rand" | ||
"testing" | ||
|
||
"github.com/Layr-Labs/eigenda/core" | ||
"github.com/Layr-Labs/eigenda/core/auth" | ||
"github.com/stretchr/testify/assert" | ||
) | ||
|
||
func TestAuthentication(t *testing.T) { | ||
|
||
// Make the authenticator | ||
authenticator := auth.NewAuthenticator(auth.AuthConfig{}) | ||
|
||
// Make the signer | ||
privateKeyHex := "0x0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" | ||
signer := auth.NewSigner(privateKeyHex) | ||
|
||
testHeader := core.BlobAuthHeader{ | ||
BlobCommitments: core.BlobCommitments{}, | ||
AccountID: signer.GetAccountID(), | ||
Nonce: rand.Uint32(), | ||
AuthenticationData: []byte{}, | ||
} | ||
|
||
// Sign the header | ||
signature, err := signer.SignBlobRequest(testHeader) | ||
assert.NoError(t, err) | ||
|
||
testHeader.AuthenticationData = signature | ||
|
||
err = authenticator.AuthenticateBlobRequest(testHeader) | ||
assert.NoError(t, err) | ||
|
||
} | ||
|
||
func TestAuthenticationFail(t *testing.T) { | ||
|
||
// Make the authenticator | ||
authenticator := auth.NewAuthenticator(auth.AuthConfig{}) | ||
|
||
// Make the signer | ||
privateKeyHex := "0x0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" | ||
signer := auth.NewSigner(privateKeyHex) | ||
|
||
testHeader := core.BlobAuthHeader{ | ||
BlobCommitments: core.BlobCommitments{}, | ||
AccountID: signer.GetAccountID(), | ||
Nonce: rand.Uint32(), | ||
AuthenticationData: []byte{}, | ||
} | ||
|
||
privateKeyHex = "0x0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcded" | ||
signer = auth.NewSigner(privateKeyHex) | ||
|
||
// Sign the header | ||
signature, err := signer.SignBlobRequest(testHeader) | ||
assert.NoError(t, err) | ||
|
||
testHeader.AuthenticationData = signature | ||
|
||
err = authenticator.AuthenticateBlobRequest(testHeader) | ||
assert.Error(t, err) | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,64 @@ | ||
package auth | ||
|
||
import ( | ||
"bytes" | ||
"encoding/binary" | ||
"fmt" | ||
|
||
"github.com/Layr-Labs/eigenda/core" | ||
|
||
"github.com/ethereum/go-ethereum/common/hexutil" | ||
"github.com/ethereum/go-ethereum/crypto" | ||
) | ||
|
||
type AuthConfig struct { | ||
} | ||
|
||
type authenticator struct { | ||
config AuthConfig | ||
} | ||
|
||
func NewAuthenticator(config AuthConfig) core.BlobRequestAuthenticator { | ||
|
||
return &authenticator{ | ||
config: config, | ||
} | ||
} | ||
|
||
func (*authenticator) AuthenticateBlobRequest(header core.BlobAuthHeader) error { | ||
|
||
buf := make([]byte, 4) | ||
binary.BigEndian.PutUint32(buf, header.Nonce) | ||
hash := crypto.Keccak256(buf) | ||
|
||
publicKeyBytes, err := hexutil.Decode(header.AccountID) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
sig := header.AuthenticationData | ||
|
||
// Decode public key | ||
pubKey, err := crypto.UnmarshalPubkey(publicKeyBytes) | ||
if err != nil { | ||
return fmt.Errorf("failed to decode public key: %v", err) | ||
} | ||
|
||
// Ensure the signature is 65 bytes (Recovery ID is the last byte) | ||
if len(sig) != 65 { | ||
mooselumph marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return fmt.Errorf("signature length is unexpected: %d", len(sig)) | ||
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. probably log accountId to help debug or whatever other identifier to uniquely identify this request |
||
} | ||
|
||
// Verify the signature | ||
sigPublicKeyECDSA, err := crypto.SigToPub(hash, sig) | ||
if err != nil { | ||
return fmt.Errorf("failed to recover public key from signature: %v", err) | ||
} | ||
|
||
if !bytes.Equal(pubKey.X.Bytes(), sigPublicKeyECDSA.X.Bytes()) || !bytes.Equal(pubKey.Y.Bytes(), sigPublicKeyECDSA.Y.Bytes()) { | ||
return fmt.Errorf("signature doesn't match with provided public key") | ||
} | ||
|
||
return nil | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,53 @@ | ||
package auth | ||
|
||
import ( | ||
"crypto/ecdsa" | ||
"encoding/binary" | ||
"fmt" | ||
"log" | ||
|
||
"github.com/Layr-Labs/eigenda/core" | ||
"github.com/ethereum/go-ethereum/common" | ||
"github.com/ethereum/go-ethereum/common/hexutil" | ||
"github.com/ethereum/go-ethereum/crypto" | ||
) | ||
|
||
type signer struct { | ||
PrivateKey *ecdsa.PrivateKey | ||
} | ||
|
||
func NewSigner(privateKeyHex string) core.BlobRequestSigner { | ||
|
||
privateKeyBytes := common.FromHex(privateKeyHex) | ||
privateKey, err := crypto.ToECDSA(privateKeyBytes) | ||
if err != nil { | ||
log.Fatalf("Failed to parse private key: %v", err) | ||
} | ||
|
||
return &signer{ | ||
PrivateKey: privateKey, | ||
} | ||
} | ||
|
||
func (s *signer) SignBlobRequest(header core.BlobAuthHeader) ([]byte, error) { | ||
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. should it also sign 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. Authentication will have two stages:
If we add signing the kzg now, it implies additional work, such as actually verifying the kzg commitment. We can add this quickly, but I'm trying to minimize scope for now. |
||
|
||
// Message you want to sign | ||
buf := make([]byte, 4) | ||
binary.BigEndian.PutUint32(buf, header.Nonce) | ||
hash := crypto.Keccak256(buf) | ||
|
||
// Sign the hash using the private key | ||
sig, err := crypto.Sign(hash, s.PrivateKey) | ||
if err != nil { | ||
return nil, fmt.Errorf("failed to sign hash: %v", err) | ||
} | ||
|
||
return sig, nil | ||
} | ||
|
||
func (s *signer) GetAccountID() string { | ||
|
||
publicKeyBytes := crypto.FromECDSAPub(&s.PrivateKey.PublicKey) | ||
return hexutil.Encode(publicKeyBytes) | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -40,6 +40,17 @@ type Blob struct { | |
Data []byte | ||
} | ||
|
||
type BlobAuthHeader struct { | ||
// Commitments | ||
BlobCommitments `json:"commitments"` | ||
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. can you comment on what this means and why we need this field? 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 a comment to the |
||
// AccountID is the account that is paying for the blob to be stored | ||
AccountID AccountID `json:"account_id"` | ||
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. Is this decided already (to be the eth address)? 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's some uncertainty about how this will marry with the payments design, but I think it's okay to move forward with supporting a simple ECDSA-based authentication. We can swap this out pretty easily, imo. Currently, the AccountID shoul be the ECDSA public key. I'll update the comment. |
||
// Nonce | ||
Nonce uint32 `json:"nonce"` | ||
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. does it need to be incremented for every call? looks like 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. Eventually this will be an incremental nonce (https://docs.google.com/document/d/1w2H38txbe21ex8bwxNMAcjRq4K5WHq0NCv6jYy8yo54/edit#heading=h.2ty1ce44eg7b). However, since we don't yet support payments we can simplify things. I think that the random challenge approach is the quickest way to get us the minimal capability that we want. |
||
// AuthenticationData is the signature of the blob header by the account ID | ||
AuthenticationData []byte `json:"authentication_data"` | ||
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. why don't we call it 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's possible this could be more generic in the future... For instance, this could end up being data which gets passed along to the rollup account smart contract, whose semantics the EigenDA protocol is agnostic to. But I'm fine with calling it signature for now. |
||
} | ||
|
||
// BlobRequestHeader contains the original data size of a blob and the security required | ||
type BlobRequestHeader struct { | ||
// Commitments | ||
|
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.
nit: does accountId need any validation before decoding?
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.
Shouldn't, if the string is invalid hex then the Decode method will fail.