Web UI Security Improvements (#7676)

* remove delete accounts

* check if user has not yet signed up

validator/rpc/accounts.go

@@ -89,31 +89,6 @@ func (s *Server) ListAccounts(ctx context.Context, req *pb.ListAccountsRequest)
 	}, nil
 }
 
-// DeleteAccounts deletes accounts from a user if their wallet is an imported wallet.
-func (s *Server) DeleteAccounts(
-	ctx context.Context, req *pb.DeleteAccountsRequest,
-) (*pb.DeleteAccountsResponse, error) {
-	if req.PublicKeys == nil || len(req.PublicKeys) < 1 {
-		return nil, status.Error(codes.InvalidArgument, "No public keys specified to delete")
-	}
-	if s.wallet == nil || s.keymanager == nil {
-		return nil, status.Error(codes.FailedPrecondition, "No wallet found")
-	}
-	if s.wallet.KeymanagerKind() != keymanager.Imported {
-		return nil, status.Error(codes.FailedPrecondition, "Only imported wallets can delete accounts")
-	}
-	if err := accounts.DeleteAccount(ctx, &accounts.DeleteAccountConfig{
-		Wallet:     s.wallet,
-		Keymanager: s.keymanager,
-		PublicKeys: req.PublicKeys,
-	}); err != nil {
-		return nil, status.Errorf(codes.Internal, "Could not delete public keys: %v", err)
-	}
-	return &pb.DeleteAccountsResponse{
-		DeletedKeys: req.PublicKeys,
-	}, nil
-}
-
 func createAccountWithDepositData(ctx context.Context, km accountCreator) (*pb.DepositDataResponse_DepositData, error) {
 	// Create a new validator account using the specified keymanager.
 	_, depositData, err := km.CreateAccount(ctx)

validator/rpc/accounts_test.go

@@ -162,65 +162,3 @@ func Test_createAccountWithDepositData(t *testing.T) {
 		t, rawResp.Data["fork_version"], fmt.Sprintf("%x", params.BeaconConfig().GenesisForkVersion),
 	)
 }
-
-func TestServer_DeleteAccounts_FailedPreconditions_WrongKeymanagerKind(t *testing.T) {
-	localWalletDir := setupWalletDir(t)
-	defaultWalletPath = localWalletDir
-	ctx := context.Background()
-	strongPass := "29384283xasjasd32%%&*@*#*"
-	w, err := accounts.CreateWalletWithKeymanager(ctx, &accounts.CreateWalletConfig{
-		WalletCfg: &wallet.Config{
-			WalletDir:      defaultWalletPath,
-			KeymanagerKind: keymanager.Derived,
-			WalletPassword: strongPass,
-		},
-		SkipMnemonicConfirm: true,
-	})
-	require.NoError(t, err)
-	km, err := w.InitializeKeymanager(ctx, &iface.InitializeKeymanagerConfig{
-		SkipMnemonicConfirm: true,
-	})
-	require.NoError(t, err)
-	ss := &Server{
-		wallet:     w,
-		keymanager: km,
-	}
-	_, err = ss.DeleteAccounts(ctx, &pb.DeleteAccountsRequest{
-		PublicKeys: make([][]byte, 1),
-	})
-	assert.ErrorContains(t, "Only imported wallets can delete accounts", err)
-}
-
-func TestServer_DeleteAccounts_FailedPreconditions(t *testing.T) {
-	ss := &Server{}
-	ctx := context.Background()
-	_, err := ss.DeleteAccounts(ctx, &pb.DeleteAccountsRequest{})
-	assert.ErrorContains(t, "No public keys specified", err)
-	_, err = ss.DeleteAccounts(ctx, &pb.DeleteAccountsRequest{
-		PublicKeys: make([][]byte, 1),
-	})
-	assert.ErrorContains(t, "No wallet found", err)
-}
-
-func TestServer_DeleteAccounts_OK(t *testing.T) {
-	ss, pubKeys := createImportedWalletWithAccounts(t, 3)
-	ctx := context.Background()
-	keys, err := ss.keymanager.FetchValidatingPublicKeys(ctx)
-	require.NoError(t, err)
-	require.Equal(t, len(pubKeys), len(keys))
-
-	// Next, we attempt to delete one of the keystores.
-	_, err = ss.DeleteAccounts(ctx, &pb.DeleteAccountsRequest{
-		PublicKeys: pubKeys[:1], // Delete the 0th public key
-	})
-	require.NoError(t, err)
-	ss.keymanager, err = ss.wallet.InitializeKeymanager(ctx, &iface.InitializeKeymanagerConfig{
-		SkipMnemonicConfirm: true,
-	})
-	require.NoError(t, err)
-
-	// We expect one of the keys to have been deleted.
-	keys, err = ss.keymanager.FetchValidatingPublicKeys(ctx)
-	require.NoError(t, err)
-	assert.Equal(t, len(pubKeys)-1, len(keys))
-}

validator/rpc/auth.go

@@ -28,7 +28,8 @@ var (
 
 const (
 	// HashedRPCPassword for the validator RPC access.
-	HashedRPCPassword = "rpc-password-hash"
+	HashedRPCPassword       = "rpc-password-hash"
+	checkUserSignupInterval = time.Second * 30
 )
 
 // Signup to authenticate access to the validator RPC API using bcrypt and
@@ -143,6 +144,40 @@ func (s *Server) ChangePassword(ctx context.Context, req *pb.ChangePasswordReque
 	return &ptypes.Empty{}, nil
 }
 
+// SaveHashedPassword to disk for the validator RPC.
+func (s *Server) SaveHashedPassword(password string) error {
+	hashedPassword, err := bcrypt.GenerateFromPassword([]byte(password), hashCost)
+	if err != nil {
+		return errors.Wrap(err, "could not generate hashed password")
+	}
+	hashFilePath := filepath.Join(s.walletDir, HashedRPCPassword)
+	return ioutil.WriteFile(hashFilePath, hashedPassword, params.BeaconIoConfig().ReadWritePermissions)
+}
+
+// Interval in which we should check if a user has not yet used the RPC Signup endpoint
+// which means they are using the --web flag and someone could come in and signup for them
+// if they have their web host:port exposed to the Internet.
+func (s *Server) checkUserSignup(ctx context.Context) {
+	ticker := time.NewTicker(checkUserSignupInterval)
+	defer ticker.Stop()
+	for {
+		select {
+		case <-ticker.C:
+			hashedPasswordPath := filepath.Join(s.walletDir, HashedRPCPassword)
+			if fileutil.FileExists(hashedPasswordPath) {
+				return
+			}
+			log.Warn(
+				"You are using the --web option but have not yet signed via a browser. " +
+					"If your web host and port are exposed to the Internet, someone else can attempt to sign up " +
+					"for you!",
+			)
+		case <-s.ctx.Done():
+			return
+		}
+	}
+}
+
 // Creates a JWT token string using the JWT key with an expiration timestamp.
 func (s *Server) createTokenString() (string, uint64, error) {
 	// Create a new token object, specifying signing method and the claims
@@ -158,13 +193,3 @@ func (s *Server) createTokenString() (string, uint64, error) {
 	}
 	return tokenString, uint64(expirationTime.Unix()), nil
 }
-
-// SaveHashedPassword to disk for the validator RPC.
-func (s *Server) SaveHashedPassword(password string) error {
-	hashedPassword, err := bcrypt.GenerateFromPassword([]byte(password), hashCost)
-	if err != nil {
-		return errors.Wrap(err, "could not generate hashed password")
-	}
-	hashFilePath := filepath.Join(s.walletDir, HashedRPCPassword)
-	return ioutil.WriteFile(hashFilePath, hashedPassword, params.BeaconIoConfig().ReadWritePermissions)
-}

validator/rpc/server.go

@@ -154,6 +154,7 @@ func (s *Server) Start() {
 			}
 		}
 	}()
+	go s.checkUserSignup(s.ctx)
 	log.WithField("address", address).Info("gRPC server listening on address")
 }