Skip to content

Commit

Permalink
fix/refactor: Move auth.token_key and other parameters to `AuthConf…
Browse files Browse the repository at this point in the history
…ig` structure

The intent is to have a central place to fetch the configuration and validate it.

This way, starting the server will fail if the needed crypto configuration is not
set up appropriately.

This also moves the `crypto` package to `internal` which is more appropriate.

Finally, the concept of `auth.token_key` changed from being a string to being a file.
The idea is that we'll have all secrets as files which will be referenced as kubernetes secrets.

Closes: #923
  • Loading branch information
JAORMX committed Sep 12, 2023
1 parent fcb94e3 commit 7bd922f
Show file tree
Hide file tree
Showing 29 changed files with 327 additions and 107 deletions.
4 changes: 4 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,12 @@ bootstrap: ## install build deps
# No passphrase (-N), don't overwrite existing keys ("n" to prompt)
echo n | ssh-keygen -t rsa -b 2048 -N "" -m PEM -f .ssh/access_token_rsa || true
echo n | ssh-keygen -t rsa -b 2048 -N "" -m PEM -f .ssh/refresh_token_rsa || true
@echo "Generating access token key pair"
openssl rsa -in .ssh/access_token_rsa -pubout -outform PEM -out .ssh/access_token_rsa.pub
@echo "Generating refresh token key pair"
openssl rsa -in .ssh/refresh_token_rsa -pubout -outform PEM -out .ssh/refresh_token_rsa.pub
@echo "Generating token key passphrase"
openssl rand -base64 32 > .ssh/token_key_passphrase
# Make sure the keys are readable by the docker user
chmod 644 .ssh/*

Expand Down
15 changes: 13 additions & 2 deletions cmd/server/app/serve.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,19 @@ var serveCmd = &cobra.Command{
return fmt.Errorf("unable to create server: %w", err)
}

s.ConsumeEvents(engine.NewExecutor(store))
s.ConsumeEvents(reconcilers.NewRecociler(store, evt))
exec, err := engine.NewExecutor(store, &cfg.Auth)
if err != nil {
return fmt.Errorf("unable to create executor: %w", err)
}

s.ConsumeEvents(exec)

rec, err := reconcilers.NewRecociler(store, evt, &cfg.Auth)
if err != nil {
return fmt.Errorf("unable to create reconciler: %w", err)
}

s.ConsumeEvents(rec)

// Start the gRPC and HTTP server in separate goroutines
errg.Go(func() error {
Expand Down
2 changes: 1 addition & 1 deletion config/config.yaml.example
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ auth:
token_expiry: 3600
refresh_expiry: 86400
nonce_period: 3600
session_key: "12345678901234567890123456789012"
token_key: "./.ssh/token_key_passphrase"

# Password Salting, these values are using argon2id
# https://en.wikipedia.org/wiki/Argon2
Expand Down
1 change: 1 addition & 0 deletions docker-compose.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ services:
- AUTH.ACCESS_TOKEN_PUBLIC_KEY=/app/.ssh/access_token_rsa.pub
- AUTH.REFRESH_TOKEN_PRIVATE_KEY=/app/.ssh/refresh_token_rsa
- AUTH.REFRESH_TOKEN_PUBLIC_KEY=/app/.ssh/refresh_token_rsa.pub
- AUTH.TOKEN_KEY=/app/.ssh/token_key
networks:
- app_net
depends_on:
Expand Down
41 changes: 41 additions & 0 deletions internal/config/auth.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
//
// Copyright 2023 Stacklok, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package config

// AuthConfig is the configuration for the auth package
type AuthConfig struct {
// AccessTokenPrivateKey is the private key used to sign the access token for authn/z
AccessTokenPrivateKey string `mapstructure:"access_token_private_key"`
// AccessTokenPublicKey is the public key used to verify the access token for authn/z
AccessTokenPublicKey string `mapstructure:"access_token_public_key"`
// RefreshTokenPrivateKey is the private key used to sign the refresh token for authn/z
RefreshTokenPrivateKey string `mapstructure:"refresh_token_private_key"`
// RefreshTokenPublicKey is the public key used to verify the refresh token for authn/z
RefreshTokenPublicKey string `mapstructure:"refresh_token_public_key"`
// TokenExpiry is the expiry time for the access token in seconds
TokenExpiry int64 `mapstructure:"token_expiry"`
// RefreshExpiry is the expiry time for the refresh token in seconds
RefreshExpiry int64 `mapstructure:"refresh_expiry"`
// NoncePeriod is the period in seconds for which a nonce is valid
NoncePeriod int64 `mapstructure:"nonce_period"`
// TokenKey is the key used to store the provider's token in the database
TokenKey string `mapstructure:"token_key"`
}

// GetAuthConfigWithDefaults returns a AuthConfig with default values
func GetAuthConfigWithDefaults() AuthConfig {
return AuthConfig{}
}
1 change: 1 addition & 0 deletions internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ type Config struct {
Metrics MetricsConfig `mapstructure:"metrics"`
Database DatabaseConfig `mapstructure:"database"`
Salt CryptoConfig `mapstructure:"salt"`
Auth AuthConfig `mapstructure:"auth"`
}

// ReadConfigFromViper reads the configuration from the given Viper instance.
Expand Down
47 changes: 45 additions & 2 deletions pkg/crypto/crypto.go → internal/crypto/crypto.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
"errors"
"fmt"
"io"
"os"
"strings"
"time"

Expand All @@ -52,6 +53,43 @@ var (
ErrIncompatibleVersion = errors.New("incompatible version of argon2")
)

// Engine is a structure that allows controller access to cryptographic functions.
// The intention is that this structure will be passed to the controller on creation
// and will validate that wrong parameters aren't passed to the functions.
type Engine struct {
encryptionKey string
}

// EngineFromAuthConfig creates a new crypto engine from an auth config
func EngineFromAuthConfig(authConfig *config.AuthConfig) (*Engine, error) {
if authConfig == nil {
return nil, errors.New("auth config is nil")
}

if authConfig.TokenKey == "" {
return nil, errors.New("token key is empty")
}

keyFile, err := os.Open(authConfig.TokenKey)
if err != nil {
return nil, fmt.Errorf("failed to open token key file: %s", err)
}

keyBytes, err := io.ReadAll(keyFile)
if err != nil {
return nil, fmt.Errorf("failed to read token key file: %s", err)
}

return NewEngine(string(keyBytes)), nil
}

// NewEngine creates a new crypto engine
func NewEngine(tokenKey string) *Engine {
return &Engine{
encryptionKey: string(tokenKey),
}
}

// GetCert gets a certificate from an envelope
func GetCert(envelope []byte) ([]byte, error) {
env := &Envelope{}
Expand Down Expand Up @@ -135,8 +173,13 @@ func EncryptBytes(key string, data []byte) ([]byte, error) {
return ciphertext, nil
}

// EncryptOAuthToken encrypts an oauth token
func (e *Engine) EncryptOAuthToken(data []byte) ([]byte, error) {
return EncryptBytes(e.encryptionKey, data)
}

// DecryptOAuthToken decrypts an encrypted oauth token
func DecryptOAuthToken(encToken string) (oauth2.Token, error) {
func (e *Engine) DecryptOAuthToken(encToken string) (oauth2.Token, error) {
var decryptedToken oauth2.Token

// base64 decode the token
Expand All @@ -146,7 +189,7 @@ func DecryptOAuthToken(encToken string) (oauth2.Token, error) {
}

// decrypt the token
token, err := DecryptBytes(viper.GetString("auth.token_key"), decodeToken)
token, err := DecryptBytes(e.encryptionKey, decodeToken)
if err != nil {
return decryptedToken, err
}
Expand Down
File renamed without changes.
File renamed without changes.
File renamed without changes.
19 changes: 14 additions & 5 deletions internal/engine/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import (
"github.com/ThreeDotsLabs/watermill/message"
"github.com/rs/zerolog"

"github.com/stacklok/mediator/internal/config"
"github.com/stacklok/mediator/internal/crypto"
"github.com/stacklok/mediator/internal/db"
"github.com/stacklok/mediator/internal/events"
pb "github.com/stacklok/mediator/pkg/generated/protobuf/go/mediator/v1"
Expand All @@ -36,14 +38,21 @@ const (

// Executor is the engine that executes the rules for a given event
type Executor struct {
querier db.Store
querier db.Store
crypteng *crypto.Engine
}

// NewExecutor creates a new executor
func NewExecutor(querier db.Store) *Executor {
return &Executor{
querier: querier,
func NewExecutor(querier db.Store, authCfg *config.AuthConfig) (*Executor, error) {
crypteng, err := crypto.EngineFromAuthConfig(authCfg)
if err != nil {
return nil, err
}

return &Executor{
querier: querier,
crypteng: crypteng,
}, nil
}

// Register implements the Consumer interface.
Expand Down Expand Up @@ -73,7 +82,7 @@ func (e *Executor) HandleEntityEvent(msg *message.Message) error {
return fmt.Errorf("error getting group: %w", err)
}

cli, err := providers.BuildClient(ctx, inf.Provider, inf.GroupID, e.querier)
cli, err := providers.BuildClient(ctx, inf.Provider, inf.GroupID, e.querier, e.crypteng)
if err != nil {
return fmt.Errorf("error building client: %w", err)
}
Expand Down
24 changes: 20 additions & 4 deletions internal/engine/executor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,23 +18,28 @@ import (
"database/sql"
"encoding/base64"
"encoding/json"
"os"
"testing"
"time"

"github.com/golang/mock/gomock"
"github.com/spf13/viper"
"github.com/stretchr/testify/require"
"golang.org/x/oauth2"
"google.golang.org/protobuf/types/known/structpb"

mockdb "github.com/stacklok/mediator/database/mock"
"github.com/stacklok/mediator/internal/config"
"github.com/stacklok/mediator/internal/crypto"
"github.com/stacklok/mediator/internal/db"
"github.com/stacklok/mediator/internal/engine"
"github.com/stacklok/mediator/pkg/crypto"
"github.com/stacklok/mediator/pkg/entities"
pb "github.com/stacklok/mediator/pkg/generated/protobuf/go/mediator/v1"
)

const (
fakeTokenKey = "foo-bar"
)

func generateFakeAccessToken(t *testing.T) string {
t.Helper()

Expand All @@ -51,7 +56,7 @@ func generateFakeAccessToken(t *testing.T) string {
require.NoError(t, err, "expected no error")

// encode token
encryptedToken, err := crypto.EncryptBytes(viper.GetString("auth.token_key"), jsonData)
encryptedToken, err := crypto.EncryptBytes(fakeTokenKey, jsonData)
require.NoError(t, err, "expected no error")

return base64.StdEncoding.EncodeToString(encryptedToken)
Expand Down Expand Up @@ -176,7 +181,18 @@ default allow = true`,

// -- end expectations

e := engine.NewExecutor(mockStore)
tmpdir := t.TempDir()
// write token key to file
tokenKeyPath := tmpdir + "/token_key"

// write key to file
err = os.WriteFile(tokenKeyPath, []byte(fakeTokenKey), 0600)
require.NoError(t, err, "expected no error")

e, err := engine.NewExecutor(mockStore, &config.AuthConfig{
TokenKey: tokenKeyPath,
})
require.NoError(t, err, "expected no error")

eiw := engine.NewEntityInfoWrapper().
WithProvider(provider).
Expand Down
2 changes: 1 addition & 1 deletion internal/reconcilers/artifacts.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ func (e *Reconciler) handleRepoReconcilerEvent(msg *message.Message) error {
// an specific repository
// nolint: gocyclo
func (e *Reconciler) handleArtifactsReconcilerEvent(ctx context.Context, prov string, evt *RepoReconcilerEvent) error {
cli, err := providers.BuildClient(ctx, prov, evt.Group, e.store)
cli, err := providers.BuildClient(ctx, prov, evt.Group, e.store, e.crypteng)
if err != nil {
return fmt.Errorf("error building client: %w", err)
}
Expand Down
21 changes: 15 additions & 6 deletions internal/reconcilers/reconcilers.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
package reconcilers

import (
"github.com/stacklok/mediator/internal/config"
"github.com/stacklok/mediator/internal/crypto"
"github.com/stacklok/mediator/internal/db"
"github.com/stacklok/mediator/internal/events"
)
Expand All @@ -30,16 +32,23 @@ const (

// Reconciler is a helper that reconciles entities
type Reconciler struct {
store db.Store
evt *events.Eventer
store db.Store
evt *events.Eventer
crypteng *crypto.Engine
}

// NewRecociler creates a new reconciler object
func NewRecociler(store db.Store, evt *events.Eventer) *Reconciler {
return &Reconciler{
store: store,
evt: evt,
func NewRecociler(store db.Store, evt *events.Eventer, authCfg *config.AuthConfig) (*Reconciler, error) {
crypteng, err := crypto.EngineFromAuthConfig(authCfg)
if err != nil {
return nil, err
}

return &Reconciler{
store: store,
evt: evt,
crypteng: crypteng,
}, nil
}

// Register implements the Consumer interface.
Expand Down
2 changes: 1 addition & 1 deletion pkg/controlplane/handlers_auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ import (
"google.golang.org/grpc/metadata"
"google.golang.org/grpc/status"

mcrypto "github.com/stacklok/mediator/internal/crypto"
"github.com/stacklok/mediator/internal/db"
"github.com/stacklok/mediator/pkg/auth"
mcrypto "github.com/stacklok/mediator/pkg/crypto"
pb "github.com/stacklok/mediator/pkg/generated/protobuf/go/mediator/v1"
)

Expand Down
2 changes: 1 addition & 1 deletion pkg/controlplane/handlers_auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,10 @@ import (

mockdb "github.com/stacklok/mediator/database/mock"
"github.com/stacklok/mediator/internal/config"
mcrypto "github.com/stacklok/mediator/internal/crypto"
"github.com/stacklok/mediator/internal/db"
"github.com/stacklok/mediator/internal/util"
"github.com/stacklok/mediator/pkg/auth"
mcrypto "github.com/stacklok/mediator/pkg/crypto"
pb "github.com/stacklok/mediator/pkg/generated/protobuf/go/mediator/v1"
)

Expand Down
6 changes: 3 additions & 3 deletions pkg/controlplane/handlers_authz.go
Original file line number Diff line number Diff line change
Expand Up @@ -450,7 +450,7 @@ func IsRequestAuthorized(ctx context.Context, value int32) bool {
}

// IsProviderCallAuthorized checks if the request is authorized
func IsProviderCallAuthorized(ctx context.Context, store db.Store, provider string, groupId int32) bool {
func (s *Server) IsProviderCallAuthorized(ctx context.Context, provider string, groupId int32) bool {
// currently everything is github
method, ok := grpc.Method(ctx)
if !ok {
Expand All @@ -460,15 +460,15 @@ func IsProviderCallAuthorized(ctx context.Context, store db.Store, provider stri
for _, item := range githubAuthorizations {
if item == method {
// check the github token
encToken, _, err := GetProviderAccessToken(ctx, store, provider, groupId, true)
encToken, _, err := s.GetProviderAccessToken(ctx, provider, groupId, true)
if err != nil {
return false
}

// check if token is expired
if encToken.Expiry.Unix() < time.Now().Unix() {
// remove from the database and deny the request
_ = store.DeleteAccessToken(ctx, db.DeleteAccessTokenParams{Provider: github.Github, GroupID: groupId})
_ = s.store.DeleteAccessToken(ctx, db.DeleteAccessTokenParams{Provider: github.Github, GroupID: groupId})

// remove from github
err := auth.DeleteAccessToken(ctx, provider, encToken.AccessToken)
Expand Down
Loading

0 comments on commit 7bd922f

Please sign in to comment.