Skip to content

Commit

Permalink
Review fixes. Added unit tests.
Browse files Browse the repository at this point in the history
  • Loading branch information
pawanpinjarkar committed May 16, 2024
1 parent 74a1c15 commit 2a0485c
Show file tree
Hide file tree
Showing 11 changed files with 139 additions and 38 deletions.
2 changes: 1 addition & 1 deletion cmd/agentbasedinstaller/client/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ const failureOutputPath = "/var/run/agent-installer/host-config-failures"

var Options struct {
ServiceBaseUrl string `envconfig:"SERVICE_BASE_URL" default:""`
Token string `envconfig:"PULL_SECRET_TOKEN" default:""`
Token string `envconfig:"AGENT_AUTH_TOKEN" default:""`
}

var RegisterOptions struct {
Expand Down
13 changes: 0 additions & 13 deletions cmd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package main

import (
"context"
"encoding/base64"
"encoding/json"
"flag"
"fmt"
Expand Down Expand Up @@ -270,18 +269,6 @@ func main() {
usageManager := usage.NewManager(log, notificationStream)
ocmClient := getOCMClient(log)

var decodedECPublicKeyPEM, decodedECPrivateKeyPEM []byte
if Options.Auth.AuthType == auth.TypeAgentLocal {
decodedECPublicKeyPEM, err = base64.StdEncoding.DecodeString(Options.Auth.ECPublicKeyPEM)
Options.Auth.ECPublicKeyPEM = string(decodedECPublicKeyPEM)
failOnError(err, "Error decoding public key:")

decodedECPrivateKeyPEM, err = base64.StdEncoding.DecodeString(Options.Auth.ECPrivateKeyPEM)
Options.Auth.ECPrivateKeyPEM = string(decodedECPrivateKeyPEM)
os.Setenv("EC_PRIVATE_KEY_PEM", string(decodedECPrivateKeyPEM))
failOnError(err, "Error decoding private key:")
}

authHandler, err := auth.NewAuthenticator(&Options.Auth, ocmClient, log.WithField("pkg", "auth"), db)
failOnError(err, "failed to create authenticator")
authzHandler := auth.NewAuthzHandler(&Options.Auth, ocmClient, log.WithField("pkg", "authz"), db)
Expand Down
1 change: 0 additions & 1 deletion internal/controller/controllers/agent_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -928,7 +928,6 @@ func generateControllerLogsDownloadURL(baseURL string, clusterID string, authTyp
}

downloadURL := fmt.Sprintf("%s%s", baseURL, u.RequestURI())
// might need to also check fot agent-installer-local
if authType != auth.TypeLocal {
return downloadURL, nil
}
Expand Down
1 change: 0 additions & 1 deletion internal/controller/controllers/agent_reclaimer.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,6 @@ func spokeReclaimSecretName(infraEnvID string) string {

func (r *agentReclaimer) ensureSpokeAgentSecret(ctx context.Context, c client.Client, log logrus.FieldLogger, infraEnvID string) error {
authToken := ""
// might need to add agent-install-local
if r.AuthType == auth.TypeLocal {
var err error
authToken, err = gencrypto.LocalJWT(infraEnvID, gencrypto.InfraEnvKey)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2275,7 +2275,6 @@ func (r *ClusterDeploymentsReconciler) generateControllerLogsDownloadURL(cluster
downloadURL := fmt.Sprintf("%s%s/v2/clusters/%s/logs",
r.ServiceBaseURL, restclient.DefaultBasePath, cluster.ID.String())

// might need to add agent-install-local
if r.AuthType != auth.TypeLocal {
return downloadURL, nil
}
Expand Down
1 change: 0 additions & 1 deletion internal/controller/controllers/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,6 @@ func IngressVipsEntriesToArray(entries []string) []*models.IngressVip {
}

func signURL(urlString string, authType auth.AuthType, id string, keyType gencrypto.LocalJWTKeyType) (string, error) {
// might need to add agent-install-local
if authType != auth.TypeLocal {
return urlString, nil
}
Expand Down
1 change: 0 additions & 1 deletion internal/controller/controllers/infraenv_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -670,7 +670,6 @@ func generateStaticNetworkConfigDownloadURL(baseURL string, infraEnvId string, a
}

downloadURL := fmt.Sprintf("%s%s", baseURL, u.RequestURI())
// might need to add agent-install-local
if authType != auth.TypeLocal {
return downloadURL, nil
}
Expand Down
1 change: 0 additions & 1 deletion internal/host/hostcommands/download_boot_artifacts_cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ func (c *downloadBootArtifactsCmd) GetSteps(ctx context.Context, host *models.Ho
return nil, fmt.Errorf("failed to generate urls for DownloadBootArtifactsRequest: %w", err)
}
// Reclaiming a host is only used in the operator scenario (not SaaS) so other auth types don't need to be considered
// might need to add agent-install-local
if c.authType == auth.TypeLocal {
bootArtifactURLs.InitrdURL, err = gencrypto.SignURL(bootArtifactURLs.InitrdURL, infraEnv.ID.String(), gencrypto.InfraEnvKey)
if err != nil {
Expand Down
42 changes: 25 additions & 17 deletions pkg/auth/agent_local_authenticator.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,40 +2,56 @@ package auth

import (
"crypto"
"encoding/base64"
"net/http"
"time"
"os"

"github.com/go-openapi/runtime"
"github.com/go-openapi/runtime/security"
"github.com/golang-jwt/jwt/v4"
"github.com/openshift/assisted-service/internal/common"
"github.com/openshift/assisted-service/internal/gencrypto"
"github.com/openshift/assisted-service/pkg/ocm"
"github.com/patrickmn/go-cache"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
"gorm.io/gorm"
)

type AgentLocalAuthenticator struct {
cache *cache.Cache
db *gorm.DB
log logrus.FieldLogger
publicKey crypto.PublicKey
}

func NewAgentLocalAuthenticator(cfg *Config, log logrus.FieldLogger, db *gorm.DB) (*AgentLocalAuthenticator, error) {
func NewAgentLocalAuthenticator(cfg *Config, log logrus.FieldLogger) (*AgentLocalAuthenticator, error) {
if cfg.ECPublicKeyPEM == "" {
return nil, errors.Errorf("agent installer local authentication requires an ecdsa Public Key")
}

// When generating an agent ISO, the Agent installer creates ECDSA public/private keys.
// However, the systemd services of the Agent installer fail to parse multiline keys accurately.
// To address this, the keys are encoded in base64 format to condense them into a single line
// before being transmitted to the assisted service.
// Upon reception, the assisted service decodes these keys back to their original multiline format
// for subsequent processing.

decodedECPublicKeyPEM, err := base64.StdEncoding.DecodeString(cfg.ECPublicKeyPEM)
if err != nil {
log.WithError(err).Fatal("Error decoding public key:")
}
cfg.ECPublicKeyPEM = string(decodedECPublicKeyPEM)

decodedECPrivateKeyPEM, err := base64.StdEncoding.DecodeString(cfg.ECPrivateKeyPEM)
if err != nil {
log.WithError(err).Fatal("Error decoding private key:")
}
cfg.ECPrivateKeyPEM = string(decodedECPrivateKeyPEM)
os.Setenv("EC_PRIVATE_KEY_PEM", string(decodedECPrivateKeyPEM))

key, err := jwt.ParseECPublicKeyFromPEM([]byte(cfg.ECPublicKeyPEM))
if err != nil {
return nil, err
}

a := &AgentLocalAuthenticator{
cache: cache.New(10*time.Minute, 30*time.Minute),
db: db,
log: log,
publicKey: key,
}
Expand Down Expand Up @@ -76,15 +92,7 @@ func (a *AgentLocalAuthenticator) AuthAgentAuth(token string) (interface{}, erro
a.log.Error(err)
return nil, common.NewInfraError(http.StatusUnauthorized, err)
}
if infraEnvOk {
_, exists := a.cache.Get(infraEnvID)
if !exists {
if infraEnvExists(a.db, infraEnvID) {
a.cache.Set(infraEnvID, "", cache.DefaultExpiration)
}
}
a.log.Infof("Authenticating infraEnv %s JWT", infraEnvID)
}
a.log.Infof("Authenticating infraEnv %s JWT", infraEnvID)

return ocm.AdminPayload(), nil
}
Expand Down
112 changes: 112 additions & 0 deletions pkg/auth/agent_local_authenticator_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
package auth

import (
"encoding/base64"
"encoding/json"
"strings"

"github.com/go-openapi/strfmt"
"github.com/google/uuid"
. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
"github.com/openshift/assisted-service/internal/common"
"github.com/openshift/assisted-service/internal/gencrypto"
"github.com/sirupsen/logrus"
)

var _ = Describe("AuthAgentAuth", func() {
var (
a *AgentLocalAuthenticator
token string
)

BeforeEach(func() {
infraEnvID := strfmt.UUID(uuid.New().String())

pubKey, privKey, err := gencrypto.ECDSAKeyPairPEM()
Expect(err).ToNot(HaveOccurred())

// Encode to Base64 (Standard encoding)
encodedPrivateKey := base64.StdEncoding.EncodeToString([]byte(privKey))
encodedPubKey := base64.StdEncoding.EncodeToString([]byte(pubKey))

cfg := &Config{
ECPublicKeyPEM: encodedPubKey,
ECPrivateKeyPEM: encodedPrivateKey,
}

token, err = gencrypto.LocalJWTForKey(infraEnvID.String(), privKey, gencrypto.InfraEnvKey)
Expect(err).ToNot(HaveOccurred())

a, err = NewAgentLocalAuthenticator(cfg, logrus.New())
Expect(err).ToNot(HaveOccurred())
})

fakeTokenAlg := func(t string) string {
parts := strings.Split(t, ".")

headerJSON, err := base64.RawStdEncoding.DecodeString(parts[0])
Expect(err).ToNot(HaveOccurred())

header := &map[string]interface{}{}
err = json.Unmarshal(headerJSON, header)
Expect(err).ToNot(HaveOccurred())

// change the algorithm in an otherwise valid token
(*header)["alg"] = "RS256"

headerBytes, err := json.Marshal(header)
Expect(err).ToNot(HaveOccurred())
newHeaderString := base64.RawStdEncoding.EncodeToString(headerBytes)

parts[0] = newHeaderString
return strings.Join(parts, ".")
}

validateErrorResponse := func(err error) {
infraError, ok := err.(*common.InfraErrorResponse)
Expect(ok).To(BeTrue())
Expect(infraError.StatusCode()).To(Equal(int32(401)))
}

It("Validates a token correctly", func() {
_, err := a.AuthAgentAuth(token)
Expect(err).ToNot(HaveOccurred())
})

It("Fails an invalid token", func() {
_, err := a.AuthAgentAuth(token + "asdf")
Expect(err).To(HaveOccurred())
validateErrorResponse(err)
})

It("Works with user auth", func() {
_, err := a.AuthUserAuth(token)
Expect(err).ToNot(HaveOccurred())
})

It("Works with URL auth", func() {
_, err := a.AuthURLAuth(token)
Expect(err).ToNot(HaveOccurred())
})

It("Fails with image auth", func() {
_, err := a.AuthImageAuth(token)
Expect(err).ToNot(HaveOccurred())
})

It("Fails a token with invalid signing method", func() {
newTok := fakeTokenAlg(token)
_, err := a.AuthAgentAuth(newTok)
Expect(err).To(HaveOccurred())
validateErrorResponse(err)
})

It("Fails with an RSA token", func() {
rsaToken, _ := GetTokenAndCert(false)
_, err := a.AuthAgentAuth(rsaToken)
Expect(err).To(HaveOccurred())
validateErrorResponse(err)
})

})
2 changes: 1 addition & 1 deletion pkg/auth/authenticator.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func NewAuthenticator(cfg *Config, ocmClient *ocm.Client, log logrus.FieldLogger
case TypeLocal:
a, err = NewLocalAuthenticator(cfg, log, db)
case TypeAgentLocal:
a, err = NewAgentLocalAuthenticator(cfg, log, db)
a, err = NewAgentLocalAuthenticator(cfg, log)
default:
err = fmt.Errorf("invalid authenticator type %v", cfg.AuthType)
}
Expand Down

0 comments on commit 2a0485c

Please sign in to comment.