Enable OSConfig agent to read GPG keys files with multiple entities (#…

…537)

Co-authored-by: Mahmoud Nada <mahmoudn@google.com>

config/repository_resource.go

@@ -31,7 +31,6 @@ import (
 	"github.com/GoogleCloudPlatform/osconfig/util"
 	"golang.org/x/crypto/openpgp"
 	"golang.org/x/crypto/openpgp/armor"
-	"golang.org/x/crypto/openpgp/packet"
 
 	agentendpointpb "google.golang.org/genproto/googleapis/cloud/osconfig/agentendpoint/v1"
 )
@@ -176,40 +175,50 @@ func zypperRepoContents(repo *agentendpointpb.OSPolicy_Resource_RepositoryResour
 	return buf.Bytes()
 }
 
-func fetchGPGKey(key string) ([]byte, error) {
+func serializeGPGKeyEntity(entityList openpgp.EntityList) ([]byte, error) {
+	var buf bytes.Buffer
+	for _, entity := range entityList {
+		if err := entity.Serialize(&buf); err != nil {
+			return nil, fmt.Errorf("error serializing gpg key: %v", err)
+		}
+	}
+	return buf.Bytes(), nil
+}
+
+func isArmoredGPGKey(keyData []byte) bool {
+	var buf bytes.Buffer
+	tee := io.TeeReader(bytes.NewReader(keyData), &buf)
+
+	// Try decoding as armored
+	decodedBlock, err := armor.Decode(tee)
+	if err == nil && decodedBlock != nil {
+		return true
+	}
+
+	return false
+}
+
+func fetchGPGKey(key string) (openpgp.EntityList, error) {
 	resp, err := http.Get(key)
 	if err != nil {
-		return nil, fmt.Errorf("error downloading gpg key: %v", err)
+		return nil, err
 	}
 	defer resp.Body.Close()
 	if resp.ContentLength > 1024*1024 {
 		return nil, fmt.Errorf("key size of %d too large", resp.ContentLength)
 	}
 
-	var buf bytes.Buffer
-	tee := io.TeeReader(resp.Body, &buf)
-
-	decoded, err := armor.Decode(tee)
-	if err != nil && err != io.EOF {
-		return nil, fmt.Errorf("error decoding gpg key: %v", err)
-	}
-
-	var entity *openpgp.Entity
-	if decoded == nil {
-		entity, err = openpgp.ReadEntity(packet.NewReader(&buf))
-	} else {
-		entity, err = openpgp.ReadEntity(packet.NewReader(decoded.Body))
-	}
+	responseBody, err := io.ReadAll(resp.Body)
 	if err != nil {
-		return nil, fmt.Errorf("error reading gpg key: %v", err)
+		return nil, fmt.Errorf("can not read response body for key %s, err: %v", key, err)
 	}
 
-	buf.Reset()
-	if err := entity.Serialize(&buf); err != nil {
-		return nil, fmt.Errorf("error serializing gpg key: %v", err)
+	if isArmoredGPGKey(responseBody) {
+		return openpgp.ReadArmoredKeyRing(bytes.NewBuffer(responseBody))
 	}
 
-	return buf.Bytes(), nil
+	return openpgp.ReadKeyRing(bytes.NewReader(responseBody))
+
 }
 
 func (r *repositoryResource) validate(ctx context.Context) (*ManagedResources, error) {
@@ -224,7 +233,11 @@ func (r *repositoryResource) validate(ctx context.Context) (*ManagedResources, e
 		r.managedRepository.RepoFileContents = aptRepoContents(r.GetApt())
 		repoFormat = agentconfig.AptRepoFormat()
 		if gpgkey != "" {
-			keyContents, err := fetchGPGKey(gpgkey)
+			entityList, err := fetchGPGKey(gpgkey)
+			if err != nil {
+				return nil, fmt.Errorf("error fetching apt gpg key %q: %v", gpgkey, err)
+			}
+			keyContents, err := serializeGPGKeyEntity(entityList)
 			if err != nil {
 				return nil, fmt.Errorf("error fetching apt gpg key %q: %v", gpgkey, err)
 			}

config/repository_resource_test.go

@@ -19,6 +19,7 @@ import (
 	"io/ioutil"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
@@ -277,3 +278,30 @@ func TestRepositoryResourceEnforceState(t *testing.T) {
 		})
 	}
 }
+
+func TestFetchGPGKey(t *testing.T) {
+	key := "https://packages.cloud.google.com/apt/doc/apt-key.gpg"
+
+	entityList, err := fetchGPGKey(key)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if len(entityList) != 2 {
+		t.Errorf("Expected: %v key(s), got: %v", 2, len(entityList))
+	}
+
+	// check if Artifact Regitry key exist or not
+	artifactRegistryKeyFound := false
+	for _, e := range entityList {
+		for key := range e.Identities {
+			if strings.Contains(key, "Artifact Registry") {
+				artifactRegistryKeyFound = true
+			}
+		}
+	}
+
+	if !artifactRegistryKeyFound {
+		t.Errorf("Expected to find Artifact Registry key in Google Cloud Public GPG key, but its missed.")
+	}
+}

policies/apt.go

@@ -28,7 +28,6 @@ import (
 	"github.com/GoogleCloudPlatform/osconfig/packages"
 	"golang.org/x/crypto/openpgp"
 	"golang.org/x/crypto/openpgp/armor"
-	"golang.org/x/crypto/openpgp/packet"
 
 	agentendpointpb "google.golang.org/genproto/googleapis/cloud/osconfig/agentendpoint/v1beta"
 )
@@ -40,7 +39,20 @@ var debArchiveTypeMap = map[agentendpointpb.AptRepository_ArchiveType]string{
 
 const aptGPGFile = "/etc/apt/trusted.gpg.d/osconfig_agent_managed.gpg"
 
-func getAptGPGKey(key string) (*openpgp.Entity, error) {
+func isArmoredGPGKey(keyData []byte) bool {
+	var buf bytes.Buffer
+	tee := io.TeeReader(bytes.NewReader(keyData), &buf)
+
+	// Try decoding as armored
+	decodedBlock, err := armor.Decode(tee)
+	if err == nil && decodedBlock != nil {
+		return true
+	}
+
+	return false
+}
+
+func getAptGPGKey(key string) (openpgp.EntityList, error) {
 	resp, err := http.Get(key)
 	if err != nil {
 		return nil, err
@@ -50,18 +62,16 @@ func getAptGPGKey(key string) (*openpgp.Entity, error) {
 		return nil, fmt.Errorf("key size of %d too large", resp.ContentLength)
 	}
 
-	var buf bytes.Buffer
-	tee := io.TeeReader(resp.Body, &buf)
-
-	b, err := armor.Decode(tee)
-	if err != nil && err != io.EOF {
-		return nil, err
+	responseBody, err := io.ReadAll(resp.Body)
+	if err != nil {
+		return nil, fmt.Errorf("can not read response body for key %s, err: %v", key, err)
 	}
 
-	if b == nil {
-		return openpgp.ReadEntity(packet.NewReader(&buf))
+	if isArmoredGPGKey(responseBody) {
+		return openpgp.ReadArmoredKeyRing(bytes.NewBuffer(responseBody))
 	}
-	return openpgp.ReadEntity(packet.NewReader(b.Body))
+
+	return openpgp.ReadKeyRing(bytes.NewReader(responseBody))
 }
 
 func containsEntity(es []*openpgp.Entity, e *openpgp.Entity) bool {
@@ -86,13 +96,15 @@ func aptRepositories(ctx context.Context, repos []*agentendpointpb.AptRepository
 
 	sort.Strings(keys)
 	for _, key := range keys {
-		e, err := getAptGPGKey(key)
+		entityList, err := getAptGPGKey(key)
 		if err != nil {
 			clog.Errorf(ctx, "Error fetching gpg key %q: %v", key, err)
 			continue
 		}
-		if !containsEntity(es, e) {
-			es = append(es, e)
+		for _, e := range entityList {
+			if !containsEntity(es, e) {
+				es = append(es, e)
+			}
 		}
 	}
 

policies/apt_test.go

@@ -20,6 +20,7 @@ import (
 	"io/ioutil"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 
 	agentendpointpb "google.golang.org/genproto/googleapis/cloud/osconfig/agentendpoint/v1beta"
@@ -80,3 +81,30 @@ func TestAptRepositories(t *testing.T) {
 		}
 	}
 }
+
+func TestGetAptGPGKey(t *testing.T) {
+	key := "https://packages.cloud.google.com/apt/doc/apt-key.gpg"
+
+	entityList, err := getAptGPGKey(key)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if len(entityList) != 2 {
+		t.Errorf("Expected: %v key(s), got: %v", 2, len(entityList))
+	}
+
+	// check if Artifact Regitry key exist or not
+	artifactRegistryKeyFound := false
+	for _, e := range entityList {
+		for key := range e.Identities {
+			if strings.Contains(key, "Artifact Registry") {
+				artifactRegistryKeyFound = true
+			}
+		}
+	}
+
+	if !artifactRegistryKeyFound {
+		t.Errorf("Expected to find Artifact Registry key in Google Cloud Public GPG key, but its missed.")
+	}
+}