Fixed a client bug that prevented server for rotating keys.

api/proxy.go

@@ -20,12 +20,12 @@ package api
 import (
 	"crypto/tls"
 	"crypto/x509"
-	"errors"
 	"net/http"
 	"net/http/httputil"
 	"net/url"
 
 	"github.com/grpc-ecosystem/grpc-gateway/v2/runtime"
+	errors "github.com/pkg/errors"
 	"golang.org/x/net/context"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/credentials"
@@ -261,7 +261,7 @@ func GetAPIHandler(
 
 	_, err = gw_cert.Verify(x509.VerifyOptions{Roots: CA_Pool})
 	if err != nil {
-		return nil, err
+		return nil, errors.WithStack(err)
 	}
 
 	gw_name := crypto_utils.GetSubjectName(gw_cert)

artifacts/definitions/Reporting/Default.yaml

@@ -253,7 +253,7 @@ reports:
                  {{ $data := Query "SELECT timestamp(epoch=now()).UTC.String AS Time, OS, Fqdn FROM info()" | Expand }}
                  {{ Get $hostinfo "0.Fqdn" }} Artifact Collection
                </div>
-               <div class="col">{{- Get $data "0.Time" -}}</div>
+               <div class="col">{{- Get $data "0" -}}</div>
              </div>
 
              {{ range .parts }}

artifacts/definitions/Windows/EventLogs/Modifications.yaml

@@ -38,8 +38,8 @@ sources:
              OwningPublisher, Enabled
       FROM read_reg_key(globs=Key)
       WHERE ChannelName =~ ProviderRegex
-        AND Mtime.Time > DateAfterTime
-        AND Mtime.Time < DateBeforeTime
+        AND Mtime > DateAfterTime
+        AND Mtime < DateBeforeTime
 
   - name: Providers
     description: Inspect the state of each provider
@@ -69,6 +69,6 @@ sources:
          Enabled, Content
       FROM X
       WHERE ProviderName =~ ProviderRegex
-        AND Mtime.Time > DateAfterTime
-        AND Mtime.Time < DateBeforeTime
+        AND Mtime > DateAfterTime
+        AND Mtime < DateBeforeTime
       ORDER BY ProviderName

artifacts/testdata/windows/users.out.yaml

@@ -3,7 +3,7 @@ Select Name, Uid, Gid, Directory, Type  from Artifact.Windows.Sys.Users() WHERE
   "Name": "Guest",
   "Uid": 501,
   "Gid": 513,
-  "Directory": [],
+  "Directory": null,
   "Type": "local"
  },
  {

crypto/client/client.go

@@ -42,6 +42,10 @@ func (self *ClientCryptoManager) AddCertificate(certificate_pem []byte) (string,
 		return "", err
 	}
 
+	// Remove the cached key for this server. This is essential to
+	// ensure servers can rotate their keys.
+	self.cipher_lru.Delete(server_name)
+
 	return server_name, nil
 }
 

go.mod

@@ -119,7 +119,7 @@ require (
 	www.velocidex.com/golang/go-prefetch v0.0.0-20200722101157-37e4751dd5ca
 	www.velocidex.com/golang/oleparse v0.0.0-20190327031422-34195d413196
 	www.velocidex.com/golang/regparser v0.0.0-20190625082115-b02dc43c2500
-	www.velocidex.com/golang/vfilter v0.0.0-20210406162709-d40800885aff
+	www.velocidex.com/golang/vfilter v0.0.0-20210424234412-ae5b320f6e90
 	www.velocidex.com/golang/vtypes v0.0.0-20210323032031-b61f37170666
 )
 

go.sum

@@ -919,7 +919,7 @@ www.velocidex.com/golang/oleparse v0.0.0-20190327031422-34195d413196/go.mod h1:i
 www.velocidex.com/golang/regparser v0.0.0-20190625082115-b02dc43c2500 h1:XqZddiAbjPIsTZcEPbqqqABS/ZV5SB7j33eczNsqD60=
 www.velocidex.com/golang/regparser v0.0.0-20190625082115-b02dc43c2500/go.mod h1:DVzloLH8L+oF3zma1Jisaat5bGF+4VLggDcYlIp00ns=
 www.velocidex.com/golang/vfilter v0.0.0-20210108051106-c18c13c24eff/go.mod h1:EdP5LDT3l9khZVjDVD2YKoDvECi3AW0e0074quDPNpA=
-www.velocidex.com/golang/vfilter v0.0.0-20210406162709-d40800885aff h1:O+DyRkA83hZMKBzX8FSLo/BzRmvqoMj9W2fYLkg8BaM=
-www.velocidex.com/golang/vfilter v0.0.0-20210406162709-d40800885aff/go.mod h1:KB724xBNYh4lgipyGwsvx0/5hXRqsKjmrMrkSjGESvU=
+www.velocidex.com/golang/vfilter v0.0.0-20210424234412-ae5b320f6e90 h1:cXbY5P/mXb0p904p981LvWek8bzsgQYnjpDPnkn+2Fk=
+www.velocidex.com/golang/vfilter v0.0.0-20210424234412-ae5b320f6e90/go.mod h1:KB724xBNYh4lgipyGwsvx0/5hXRqsKjmrMrkSjGESvU=
 www.velocidex.com/golang/vtypes v0.0.0-20210323032031-b61f37170666 h1:4VjIpQYv3WWXizcMMwAHi8hp5B6pbWPD1jsNxygWhRE=
 www.velocidex.com/golang/vtypes v0.0.0-20210323032031-b61f37170666/go.mod h1:34AZRfhNvJ1QAwPpYrDxjCyOFys+NbSmH6LLVSjsAEg=

http_comms/comms.go

@@ -799,6 +799,7 @@ func NewHTTPCommunicator(
 			manager:    manager,
 			executor:   executor,
 			logger:     logger,
+			clock:      clock,
 		},
 		on_exit: on_exit,
 		sender:  sender,

http_comms/comms_test.go

@@ -237,8 +237,6 @@ func (self *CommsTestSuite) TestEnrollment() {
 
 	communicator.receiver.sendMessageList(context.Background(), nil, false)
 
-	utils.Debug(self.frontend1.events)
-
 	checkResponses(self.T(), self.frontend1.events, []string{
 		// First request looks for server.pem but fails on frontend1
 		"0 request: /server.pem",

http_comms/e2e_test.go

@@ -0,0 +1,234 @@
+package http_comms
+
+import (
+	"context"
+	"net/http"
+	"sync"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"github.com/stretchr/testify/suite"
+	actions_proto "www.velocidex.com/golang/velociraptor/actions/proto"
+	"www.velocidex.com/golang/velociraptor/api"
+	"www.velocidex.com/golang/velociraptor/config"
+	config_proto "www.velocidex.com/golang/velociraptor/config/proto"
+	"www.velocidex.com/golang/velociraptor/crypto"
+	crypto_client "www.velocidex.com/golang/velociraptor/crypto/client"
+	crypto_proto "www.velocidex.com/golang/velociraptor/crypto/proto"
+	crypto_utils "www.velocidex.com/golang/velociraptor/crypto/utils"
+	"www.velocidex.com/golang/velociraptor/datastore"
+	"www.velocidex.com/golang/velociraptor/executor"
+	"www.velocidex.com/golang/velociraptor/file_store/test_utils"
+	"www.velocidex.com/golang/velociraptor/logging"
+	"www.velocidex.com/golang/velociraptor/paths"
+	"www.velocidex.com/golang/velociraptor/server"
+	"www.velocidex.com/golang/velociraptor/services"
+	"www.velocidex.com/golang/velociraptor/services/client_info"
+	"www.velocidex.com/golang/velociraptor/services/interrogation"
+	"www.velocidex.com/golang/velociraptor/services/journal"
+	"www.velocidex.com/golang/velociraptor/services/launcher"
+	"www.velocidex.com/golang/velociraptor/services/notifications"
+	"www.velocidex.com/golang/velociraptor/services/repository"
+	"www.velocidex.com/golang/velociraptor/utils"
+	"www.velocidex.com/golang/velociraptor/vtesting"
+)
+
+type TestSuite struct {
+	suite.Suite
+	config_obj *config_proto.Config
+	client_id  string
+	sm         *services.Service
+}
+
+func (self *TestSuite) SetupTest() {
+	t := self.T()
+
+	config_obj, err := new(config.Loader).WithFileLoader(
+		"../http_comms/test_data/server.config.yaml").
+		WithRequiredClient().WithWriteback().LoadAndValidate()
+	assert.NoError(t, err)
+
+	self.config_obj = config_obj
+
+	ctx, _ := context.WithTimeout(context.Background(), time.Second*60)
+	self.sm = services.NewServiceManager(ctx, self.config_obj)
+
+	self.config_obj.Frontend.IsMaster = true
+
+	// Start the journaling service manually for tests.
+	require.NoError(t, self.sm.Start(journal.StartJournalService))
+	require.NoError(t, self.sm.Start(notifications.StartNotificationService))
+	require.NoError(t, self.sm.Start(interrogation.StartInterrogationService))
+	require.NoError(t, self.sm.Start(repository.StartRepositoryManager))
+	require.NoError(t, self.sm.Start(launcher.StartLauncherService))
+	require.NoError(t, self.sm.Start(client_info.StartClientInfoService))
+
+	self.EnrolClient()
+
+	self.config_obj.Client.MaxPoll = 1
+	self.config_obj.Client.MaxPollStd = 1
+}
+
+func (self *TestSuite) TearDownTest() {
+	self.sm.Close()
+
+	test_utils.GetMemoryDataStore(self.T(), self.config_obj).Clear()
+	test_utils.GetMemoryFileStore(self.T(), self.config_obj).Clear()
+}
+
+// Create a client record so server and client can talk.
+func (self *TestSuite) EnrolClient() {
+	private_key, err := crypto_utils.ParseRsaPrivateKeyFromPemStr(
+		[]byte(self.config_obj.Writeback.PrivateKey))
+	assert.NoError(self.T(), err)
+
+	pem := &crypto_proto.PublicKey{
+		Pem:        crypto_utils.PublicKeyToPem(&private_key.PublicKey),
+		EnrollTime: 1000,
+	}
+
+	self.client_id = crypto_utils.ClientIDFromPublicKey(&private_key.PublicKey)
+	client_path_manager := paths.NewClientPathManager(self.client_id)
+	db, _ := datastore.GetDB(self.config_obj)
+
+	// Write a client record.
+	client_info_obj := &actions_proto.ClientInfo{
+		ClientId: self.client_id,
+	}
+	err = db.SetSubject(self.config_obj, client_path_manager.Path(), client_info_obj)
+	assert.NoError(self.T(), err)
+
+	err = db.SetSubject(self.config_obj, client_path_manager.Key().Path(), pem)
+	assert.NoError(self.T(), err)
+}
+
+// Create a server
+func (self *TestSuite) makeServer(
+	server_ctx context.Context,
+	server_wg *sync.WaitGroup) {
+
+	// Create a new server
+	server_obj, err := server.NewServer(server_ctx, self.config_obj, server_wg)
+	assert.NoError(self.T(), err)
+
+	mux := http.NewServeMux()
+	server.PrepareFrontendMux(self.config_obj, server_obj, mux)
+
+	err = api.StartFrontendPlainHttp(server_ctx, server_wg, self.config_obj, server_obj, mux)
+	assert.NoError(self.T(), err)
+
+	// Wait for it to come up
+	vtesting.WaitUntil(2*time.Second, self.T(), func() bool {
+		req, err := http.Get("http://localhost:8000/server.pem")
+		if err != nil || req.StatusCode != http.StatusOK {
+			return false
+		}
+		defer req.Body.Close()
+
+		return true
+	})
+}
+
+// Create a client
+func (self *TestSuite) makeClient(
+	client_ctx context.Context,
+	client_wg *sync.WaitGroup) *HTTPCommunicator {
+	manager, err := crypto_client.NewClientCryptoManager(
+		self.config_obj, []byte(self.config_obj.Writeback.PrivateKey))
+	assert.NoError(self.T(), err)
+
+	exe, err := executor.NewClientExecutor(client_ctx, self.config_obj)
+	assert.NoError(self.T(), err)
+
+	on_error := func() {}
+	comm, err := NewHTTPCommunicator(
+		self.config_obj,
+		manager,
+		exe,
+		[]string{"http://localhost:8000/"},
+		on_error, utils.RealClock{},
+	)
+	assert.NoError(self.T(), err)
+
+	client_wg.Add(1)
+	go func() {
+		defer client_wg.Done()
+
+		comm.Run(client_ctx)
+	}()
+
+	return comm
+}
+
+func (self *TestSuite) TestServerRotateKeyE2E() {
+	server_ctx, server_cancel := context.WithCancel(self.sm.Ctx)
+	server_wg := &sync.WaitGroup{}
+
+	self.makeServer(server_ctx, server_wg)
+
+	client_ctx, client_cancel := context.WithCancel(self.sm.Ctx)
+	client_wg := &sync.WaitGroup{}
+
+	comm := self.makeClient(client_ctx, client_wg)
+	err := comm.sender.sendToURL(client_ctx, [][]byte{}, false)
+	assert.NoError(self.T(), err)
+
+	// Make sure the client is properly enrolled
+	vtesting.WaitUntil(2*time.Second, self.T(), func() bool {
+		// json.Dump(logging.GetMemoryLogs())
+		return vtesting.ContainsString("response with status: 200", logging.GetMemoryLogs())
+	})
+	//	json.Dump(logging.GetMemoryLogs())
+	logging.ClearMemoryLogs()
+
+	// Now rotate the server keys: First shut down the old server.
+	server_cancel()
+	server_wg.Wait()
+
+	// Now rekey the server
+	frontend_cert, err := crypto.GenerateServerCert(
+		self.config_obj, self.config_obj.Client.PinnedServerName)
+	assert.NoError(self.T(), err)
+
+	self.config_obj.Frontend.Certificate = frontend_cert.Cert
+	self.config_obj.Frontend.PrivateKey = frontend_cert.PrivateKey
+
+	// Now bring up the new server.
+	server_ctx, server_cancel = context.WithCancel(self.sm.Ctx)
+	server_wg = &sync.WaitGroup{}
+
+	self.makeServer(server_ctx, server_wg)
+
+	//	json.Dump(logging.GetMemoryLogs())
+
+	logging.ClearMemoryLogs()
+
+	// Sending another one will produce an error.
+	err = comm.sender.sendToURL(client_ctx, [][]byte{}, false)
+	assert.Error(self.T(), err)
+
+	// Make sure the client properly rekeys and continues to talk to the server
+	vtesting.WaitUntil(5*time.Second, self.T(), func() bool {
+		// json.Dump(logging.GetMemoryLogs())
+		return vtesting.ContainsString("response with status: 200", logging.GetMemoryLogs())
+	})
+
+	// Done
+	server_cancel()
+	client_cancel()
+
+	// Wait for the server to quit.
+	client_wg.Wait()
+	server_wg.Wait()
+}
+
+func TestClientServerComms(t *testing.T) {
+	config_obj := config.GetDefaultConfig()
+	config_obj.Datastore.Implementation = "Test"
+
+	suite.Run(t, &TestSuite{
+		config_obj: config_obj,
+	})
+}

logging/logging.go

@@ -100,6 +100,10 @@ func InitLogging(config_obj *config_proto.Config) error {
 	return nil
 }
 
+func ClearMemoryLogs() {
+	memory_logs = nil
+}
+
 func GetMemoryLogs() []string {
 	mu.Lock()
 	defer mu.Unlock()

paths/artifacts/paths.go

@@ -264,10 +264,10 @@ func GetArtifactMode(config_obj *config_proto.Config, artifact_name string) (int
 
 	repository, _ := manager.GetGlobalRepository(config_obj)
 
-	artifact, pres := repository.Get(config_obj, artifact_name)
-	if !pres {
-		return 0, fmt.Errorf("Artifact %s not known", artifact_name)
+	artifact_type, err := repository.GetArtifactType(config_obj, artifact_name)
+	if err != nil {
+		return 0, err
 	}
 
-	return paths.ModeNameToMode(artifact.Type), nil
+	return paths.ModeNameToMode(artifact_type), nil
 }

services/repository.go

@@ -92,6 +92,9 @@ type Repository interface {
 	Get(config_obj *config_proto.Config,
 		name string) (*artifacts_proto.Artifact, bool)
 
+	// An optimization that avoids copying the entire artifact definition
+	GetArtifactType(config_obj *config_proto.Config, artifact_name string) (string, error)
+
 	// Remove a named artifact from the repository.
 	Del(name string)
 

services/repository/repository.go

@@ -277,6 +277,19 @@ func (self *Repository) LoadProto(artifact *artifacts_proto.Artifact, validate b
 	return artifact, nil
 }
 
+func (self *Repository) GetArtifactType(
+	config_obj *config_proto.Config, artifact_name string) (string, error) {
+	self.mu.Lock()
+	defer self.mu.Unlock()
+
+	artifact, pres := self.get(artifact_name)
+	if !pres {
+		return "", fmt.Errorf("Artifact %s not known", artifact_name)
+	}
+
+	return artifact.Type, nil
+}
+
 func (self *Repository) Get(
 	config_obj *config_proto.Config, name string) (*artifacts_proto.Artifact, bool) {
 	self.mu.Lock()