From 5e307196fa92a4c5db27c6e6cb7fe80183cd7c8f Mon Sep 17 00:00:00 2001 From: Homayoon Alimohammadi Date: Fri, 11 Oct 2024 14:30:16 +0400 Subject: [PATCH] validate capi-auth-token on dqlite/remove (#56) --- pkg/api/v2/consts.go | 6 +++++ pkg/api/v2/register.go | 4 +++- pkg/api/v2/remove.go | 13 +++++++++-- pkg/api/v2/remove_test.go | 40 ++++++++++++++++++++++++++++---- pkg/snap/interface.go | 5 ++++ pkg/snap/mock/mock.go | 14 +++++++++++ pkg/snap/options.go | 7 ++++++ pkg/snap/snap.go | 18 ++++++++++++++ pkg/snap/snap_capi_token_test.go | 37 +++++++++++++++++++++++++++++ 9 files changed, 137 insertions(+), 7 deletions(-) create mode 100644 pkg/api/v2/consts.go create mode 100644 pkg/snap/snap_capi_token_test.go diff --git a/pkg/api/v2/consts.go b/pkg/api/v2/consts.go new file mode 100644 index 0000000..2fb8a82 --- /dev/null +++ b/pkg/api/v2/consts.go @@ -0,0 +1,6 @@ +package v2 + +const ( + // CAPIAuthTokenHeader is the header used to pass the CAPI auth token. + CAPIAuthTokenHeader = "capi-auth-token" +) diff --git a/pkg/api/v2/register.go b/pkg/api/v2/register.go index 4edb6ad..115bd46 100644 --- a/pkg/api/v2/register.go +++ b/pkg/api/v2/register.go @@ -67,7 +67,9 @@ func (a *API) RegisterServer(server *http.ServeMux, middleware func(f http.Handl return } - if rc, err := a.RemoveFromDqlite(r.Context(), req); err != nil { + token := r.Header.Get(CAPIAuthTokenHeader) + + if rc, err := a.RemoveFromDqlite(r.Context(), req, token); err != nil { httputil.Error(w, rc, fmt.Errorf("failed to remove from dqlite: %w", err)) return } diff --git a/pkg/api/v2/remove.go b/pkg/api/v2/remove.go index e989afa..0fd00c0 100644 --- a/pkg/api/v2/remove.go +++ b/pkg/api/v2/remove.go @@ -11,11 +11,20 @@ import ( // RemoveFromDqliteRequest represents a request to remove a node from the dqlite cluster. type RemoveFromDqliteRequest struct { // RemoveEndpoint is the endpoint of the node to remove from the dqlite cluster. - RemoveEndpoint string `json:"removeEndpoint"` + RemoveEndpoint string `json:"remove_endpoint"` } // RemoveFromDqlite implements the "POST /v2/dqlite/remove" endpoint and removes a node from the dqlite cluster. -func (a *API) RemoveFromDqlite(ctx context.Context, req RemoveFromDqliteRequest) (int, error) { +func (a *API) RemoveFromDqlite(ctx context.Context, req RemoveFromDqliteRequest, token string) (int, error) { + isValid, err := a.Snap.IsCAPIAuthTokenValid(token) + if err != nil { + return http.StatusInternalServerError, fmt.Errorf("failed to validate CAPI auth token: %w", err) + } + + if !isValid { + return http.StatusUnauthorized, fmt.Errorf("invalid CAPI auth token %q", token) + } + if err := snaputil.RemoveNodeFromDqlite(ctx, a.Snap, req.RemoveEndpoint); err != nil { return http.StatusInternalServerError, fmt.Errorf("failed to remove node from dqlite: %w", err) } diff --git a/pkg/api/v2/remove_test.go b/pkg/api/v2/remove_test.go index 5ad4aba..2371b4d 100644 --- a/pkg/api/v2/remove_test.go +++ b/pkg/api/v2/remove_test.go @@ -17,23 +17,55 @@ func TestRemove(t *testing.T) { cmdErr := errors.New("failed to run command") apiv2 := &v2.API{ Snap: &mock.Snap{ - RunCommandErr: cmdErr, + RunCommandErr: cmdErr, + CAPIAuthTokenValid: true, }, } - rc, err := apiv2.RemoveFromDqlite(context.Background(), v2.RemoveFromDqliteRequest{RemoveEndpoint: "1.1.1.1:1234"}) + rc, err := apiv2.RemoveFromDqlite(context.Background(), v2.RemoveFromDqliteRequest{RemoveEndpoint: "1.1.1.1:1234"}, "token") g := NewWithT(t) g.Expect(err).To(MatchError(cmdErr)) g.Expect(rc).To(Equal(http.StatusInternalServerError)) }) + t.Run("InvalidToken", func(t *testing.T) { + apiv2 := &v2.API{ + Snap: &mock.Snap{ + CAPIAuthTokenValid: false, // explicitly set to false + }, + } + + rc, err := apiv2.RemoveFromDqlite(context.Background(), v2.RemoveFromDqliteRequest{RemoveEndpoint: "1.1.1.1:1234"}, "token") + + g := NewWithT(t) + g.Expect(err).To(HaveOccurred()) + g.Expect(rc).To(Equal(http.StatusUnauthorized)) + }) + + t.Run("TokenFileNotFound", func(t *testing.T) { + tokenErr := errors.New("token file not found") + apiv2 := &v2.API{ + Snap: &mock.Snap{ + CAPIAuthTokenError: tokenErr, + }, + } + + rc, err := apiv2.RemoveFromDqlite(context.Background(), v2.RemoveFromDqliteRequest{RemoveEndpoint: "1.1.1.1:1234"}, "token") + + g := NewWithT(t) + g.Expect(err).To(MatchError(tokenErr)) + g.Expect(rc).To(Equal(http.StatusInternalServerError)) + }) + t.Run("RemovesSuccessfully", func(t *testing.T) { apiv2 := &v2.API{ - Snap: &mock.Snap{}, + Snap: &mock.Snap{ + CAPIAuthTokenValid: true, + }, } - rc, err := apiv2.RemoveFromDqlite(context.Background(), v2.RemoveFromDqliteRequest{RemoveEndpoint: "1.1.1.1:1234"}) + rc, err := apiv2.RemoveFromDqlite(context.Background(), v2.RemoveFromDqliteRequest{RemoveEndpoint: "1.1.1.1:1234"}, "token") g := NewWithT(t) g.Expect(err).ToNot(HaveOccurred()) diff --git a/pkg/snap/interface.go b/pkg/snap/interface.go index b783da9..b4e4ed7 100644 --- a/pkg/snap/interface.go +++ b/pkg/snap/interface.go @@ -13,6 +13,8 @@ type Snap interface { GetSnapDataPath(parts ...string) string // GetSnapCommonPath returns the path to a file or directory in the snap's common directory. GetSnapCommonPath(parts ...string) string + // GetCAPIPath returns the path to a file or directory in the CAPI directory. + GetCAPIPath(parts ...string) string // RunCommand runs a shell command. RunCommand(ctx context.Context, commands ...string) error @@ -98,6 +100,9 @@ type Snap interface { // GetKnownToken returns the token for a known user from the known_users.csv file. GetKnownToken(username string) (string, error) + // IsCAPIAuthTokenValid returns true if token is a valid CAPI auth token. + IsCAPIAuthTokenValid(token string) (bool, error) + // SignCertificate signs the certificate signing request, and returns the certificate in PEM format. SignCertificate(ctx context.Context, csrPEM []byte) ([]byte, error) diff --git a/pkg/snap/mock/mock.go b/pkg/snap/mock/mock.go index f719342..bcc267c 100644 --- a/pkg/snap/mock/mock.go +++ b/pkg/snap/mock/mock.go @@ -34,6 +34,7 @@ type Snap struct { SnapDir string SnapDataDir string SnapCommonDir string + CAPIDir string RunCommandCalledWith []RunCommandCall RunCommandErr error @@ -85,6 +86,9 @@ type Snap struct { KubeletTokens map[string]string // map hostname to token KnownTokens map[string]string // map username to token + CAPIAuthTokenValid bool + CAPIAuthTokenError error + SignCertificateCalledWith []string // string(csrPEM) SignedCertificate string @@ -116,6 +120,11 @@ func (s *Snap) GetSnapCommonPath(parts ...string) string { return filepath.Join(append([]string{s.SnapCommonDir}, parts...)...) } +// GetCAPIPath is a mock implementation for the snap.Snap interface. +func (s *Snap) GetCAPIPath(parts ...string) string { + return filepath.Join(append([]string{s.CAPIDir}, parts...)...) +} + // RunCommand is a mock implementation for the snap.Snap interface. func (s *Snap) RunCommand(_ context.Context, commands ...string) error { s.RunCommandCalledWith = append(s.RunCommandCalledWith, RunCommandCall{Commands: commands}) @@ -320,6 +329,11 @@ func (s *Snap) GetKnownToken(username string) (string, error) { return "", fmt.Errorf("no known token for user %s", username) } +// IsCAPIAuthTokenValid is a mock implementation for the snap.Snap interface. +func (s *Snap) IsCAPIAuthTokenValid(token string) (bool, error) { + return s.CAPIAuthTokenValid, s.CAPIAuthTokenError +} + // RunUpgrade is a mock implementation for the snap.Snap interface. func (s *Snap) RunUpgrade(ctx context.Context, upgrade string, phase string) error { s.RunUpgradeCalledWith = append(s.RunUpgradeCalledWith, fmt.Sprintf("%s %s", upgrade, phase)) diff --git a/pkg/snap/options.go b/pkg/snap/options.go index a439e41..f58456c 100644 --- a/pkg/snap/options.go +++ b/pkg/snap/options.go @@ -22,3 +22,10 @@ func WithCommandRunner(f func(context.Context, ...string) error) func(s *snap) { s.runCommand = f } } + +// WithCAPIPath configures the path to the CAPI directory. +func WithCAPIPath(path string) func(s *snap) { + return func(s *snap) { + s.capiPath = path + } +} diff --git a/pkg/snap/snap.go b/pkg/snap/snap.go index 2e9f10d..3f63c48 100644 --- a/pkg/snap/snap.go +++ b/pkg/snap/snap.go @@ -23,6 +23,7 @@ type snap struct { snapDir string snapDataDir string snapCommonDir string + capiPath string runCommand func(context.Context, ...string) error clusterTokensMu sync.Mutex @@ -34,6 +35,10 @@ type snap struct { applyCNIBackoff time.Duration } +const ( + defaultCAPIPath = "/capi" +) + // NewSnap creates a new interface with the MicroK8s snap. // NewSnap accepts the $SNAP, $SNAP_DATA and $SNAP_COMMON, directories, and a number of options. func NewSnap(snapDir, snapDataDir, snapCommonDir string, options ...func(s *snap)) Snap { @@ -41,6 +46,7 @@ func NewSnap(snapDir, snapDataDir, snapCommonDir string, options ...func(s *snap snapDir: snapDir, snapDataDir: snapDataDir, snapCommonDir: snapCommonDir, + capiPath: defaultCAPIPath, runCommand: util.RunCommand, } @@ -65,6 +71,9 @@ func (s *snap) GetSnapDataPath(parts ...string) string { func (s *snap) GetSnapCommonPath(parts ...string) string { return filepath.Join(append([]string{s.snapCommonDir}, parts...)...) } +func (s *snap) GetCAPIPath(parts ...string) string { + return filepath.Join(append([]string{s.capiPath}, parts...)...) +} func (s *snap) GetGroupName() string { if s.isStrict() { @@ -331,6 +340,15 @@ func (s *snap) GetKnownToken(username string) (string, error) { return "", fmt.Errorf("no known token found for user %s", username) } +// IsCAPIAuthTokenValid checks if the given CAPI auth token is valid. +func (s *snap) IsCAPIAuthTokenValid(token string) (bool, error) { + contents, err := util.ReadFile(s.GetCAPIPath("etc", "token")) + if err != nil { + return false, fmt.Errorf("failed to read token file: %w", err) + } + return strings.TrimSpace(contents) == token, nil +} + func (s *snap) SignCertificate(ctx context.Context, csrPEM []byte) ([]byte, error) { // TODO: consider using crypto/x509 for this instead of relying on openssl commands. // NOTE(neoaggelos): x509.CreateCertificate() has some hardcoded fields that are incompatible with MicroK8s. diff --git a/pkg/snap/snap_capi_token_test.go b/pkg/snap/snap_capi_token_test.go new file mode 100644 index 0000000..23bdb96 --- /dev/null +++ b/pkg/snap/snap_capi_token_test.go @@ -0,0 +1,37 @@ +package snap_test + +import ( + "os" + "path/filepath" + "testing" + + . "github.com/onsi/gomega" + + "github.com/canonical/microk8s-cluster-agent/pkg/snap" +) + +func TestCAPIAuthToken(t *testing.T) { + capiTestPath := "./capi-test" + os.RemoveAll(capiTestPath) + s := snap.NewSnap("", "", "", snap.WithCAPIPath(capiTestPath)) + token := "token123" + + g := NewWithT(t) + + isValid, err := s.IsCAPIAuthTokenValid(token) + g.Expect(err).To(MatchError(os.ErrNotExist)) + g.Expect(isValid).To(BeFalse()) + + capiEtc := filepath.Join(capiTestPath, "etc") + defer os.RemoveAll(capiTestPath) + g.Expect(os.MkdirAll(capiEtc, 0755)).To(Succeed()) + g.Expect(os.WriteFile("./capi-test/etc/token", []byte(token), 0600)).To(Succeed()) + + isValid, err = s.IsCAPIAuthTokenValid("random-token") + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(isValid).To(BeFalse()) + + isValid, err = s.IsCAPIAuthTokenValid(token) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(isValid).To(BeTrue()) +}