Repository: open-telemetry/opamp-go (go)

Issue:
PackageSyncer should verify the signature somehow
See todo here:
https://github.com/open-telemetry/opamp-go/blob/b33ab76c401d0f5c3f179c5905409c1502176f97/client/internal/packagessyncer.go#L293

The packagesSyncer does not have a way to verify the signature. I think passing the signature into UpdateContent would make sense here.

Pull request:
Add signature to UpdateContent
Passes the signature field to UpdateContent, so the implementation may verify the signature of the file.

Closes #300 

Patch:
diff --git a/client/internal/inmempackagestore.go b/client/internal/inmempackagestore.go
index a6abc1f..dc719e7 100644
--- a/client/internal/inmempackagestore.go
+++ b/client/internal/inmempackagestore.go
@@ -13,6 +13,7 @@ type InMemPackagesStore struct {
 	allPackagesHash      []byte
 	pkgState             map[string]types.PackageState
 	fileContents         map[string][]byte
+	fileSignatures       map[string][]byte
 	fileHashes           map[string][]byte
 	lastReportedStatuses *protobufs.PackageStatuses
 
@@ -23,9 +24,10 @@ var _ types.PackagesStateProvider = (*InMemPackagesStore)(nil)
 
 func NewInMemPackagesStore() *InMemPackagesStore {
 	return &InMemPackagesStore{
-		fileContents: map[string][]byte{},
-		fileHashes:   map[string][]byte{},
-		pkgState:     map[string]types.PackageState{},
+		fileContents:   map[string][]byte{},
+		fileSignatures: map[string][]byte{},
+		fileHashes:     map[string][]byte{},
+		pkgState:       map[string]types.PackageState{},
 	}
 }
 
@@ -63,12 +65,13 @@ func (l *InMemPackagesStore) FileContentHash(packageName string) ([]byte, error)
 	return l.fileHashes[packageName], nil
 }
 
-func (l *InMemPackagesStore) UpdateContent(_ context.Context, packageName string, data io.Reader, contentHash []byte) error {
+func (l *InMemPackagesStore) UpdateContent(_ context.Context, packageName string, data io.Reader, contentHash, signature []byte) error {
 	b, err := io.ReadAll(data)
 	if err != nil {
 		return err
 	}
 	l.fileContents[packageName] = b
+	l.fileSignatures[packageName] = signature
 	l.fileHashes[packageName] = contentHash
 	return nil
 }
@@ -92,6 +95,10 @@ func (l *InMemPackagesStore) GetContent() map[string][]byte {
 	return l.fileContents
 }
 
+func (l *InMemPackagesStore) GetSignature() map[string][]byte {
+	return l.fileSignatures
+}
+
 func (l *InMemPackagesStore) LastReportedStatuses() (*protobufs.PackageStatuses, error) {
 	return l.lastReportedStatuses, nil
 }
diff --git a/client/internal/packagessyncer.go b/client/internal/packagessyncer.go
index 068acd5..c02828d 100644
--- a/client/internal/packagessyncer.go
+++ b/client/internal/packagessyncer.go
@@ -290,10 +290,7 @@ func (s *packagesSyncer) downloadFile(ctx context.Context, pkgName string, file
 		return fmt.Errorf("cannot download file from %s, HTTP response=%v", file.DownloadUrl, resp.StatusCode)
 	}
 
-	// TODO: either add a callback to verify file.Signature or pass the Signature
-	// as a parameter to UpdateContent.
-
-	err = s.localState.UpdateContent(ctx, pkgName, resp.Body, file.ContentHash)
+	err = s.localState.UpdateContent(ctx, pkgName, resp.Body, file.ContentHash, file.Signature)
 	if err != nil {
 		return fmt.Errorf("failed to install/update the package %s downloaded from %s: %v", pkgName, file.DownloadUrl, err)
 	}
diff --git a/client/types/packagessyncer.go b/client/types/packagessyncer.go
index a0aee49..3662e6f 100644
--- a/client/types/packagessyncer.go
+++ b/client/types/packagessyncer.go
@@ -77,7 +77,7 @@ type PackagesStateProvider interface {
 	// an error.
 	// Content hash must be updated if the data is updated without failure.
 	// The function must cancel and return an error if the context is cancelled.
-	UpdateContent(ctx context.Context, packageName string, data io.Reader, contentHash []byte) error
+	UpdateContent(ctx context.Context, packageName string, data io.Reader, contentHash, signature []byte) error
 
 	// DeletePackage deletes the package from the Agent's local storage.
 	DeletePackage(packageName string) error


Test patch:
diff --git a/client/clientimpl_test.go b/client/clientimpl_test.go
index 76ca41a..a0328ef 100644
--- a/client/clientimpl_test.go
+++ b/client/clientimpl_test.go
@@ -1272,6 +1272,7 @@ type packageTestCase struct {
 	available           *protobufs.PackagesAvailable
 	expectedStatus      *protobufs.PackageStatuses
 	expectedFileContent map[string][]byte
+	expectedSignature   map[string][]byte
 	expectedError       string
 }
 
@@ -1393,6 +1394,10 @@ func verifyUpdatePackages(t *testing.T, testCase packageTestCase) {
 			for pkgName, receivedContent := range localPackageState.GetContent() {
 				expectedContent := testCase.expectedFileContent[pkgName]
 				assert.EqualValues(t, expectedContent, receivedContent)
+
+				actualSignature := localPackageState.GetSignature()[pkgName]
+				expectedSignature := testCase.expectedSignature[pkgName]
+				assert.EqualValues(t, expectedSignature, actualSignature)
 			}
 		}
 
@@ -1467,6 +1472,7 @@ func createPackageTestCase(name string, downloadSrv *httptest.Server) packageTes
 					File: &protobufs.DownloadableFile{
 						DownloadUrl: downloadSrv.URL + packageFileURL,
 						ContentHash: []byte{4, 5},
+						Signature:   []byte{6, 7},
 					},
 					Hash: []byte{1, 2, 3},
 				},
@@ -1492,6 +1498,10 @@ func createPackageTestCase(name string, downloadSrv *httptest.Server) packageTes
 		expectedFileContent: map[string][]byte{
 			"package1": packageFileContent,
 		},
+
+		expectedSignature: map[string][]byte{
+			"package1": {6, 7},
+		},
 	}
 }