Skip to content

Commit

Permalink
MCS: Validate provisioning token
Browse files Browse the repository at this point in the history
Implements: openshift/enhancements#443
Requires: openshift/installer#4372

Basically we need to protect the Ignition config since it
contains secrets (pull secret, kubeconfig) and we can't rely
solely on network firewalling in all cases.

The installer will generate a secret into both the pointer
config and as a secret in our namespace, the MCS checks it.
  • Loading branch information
cgwalters committed Nov 13, 2020
1 parent 4e6d106 commit 193679e
Show file tree
Hide file tree
Showing 7 changed files with 129 additions and 4 deletions.
3 changes: 3 additions & 0 deletions manifests/machineconfigserver/clusterrole.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,6 @@ rules:
- apiGroups: ["machineconfiguration.openshift.io"]
resources: ["machineconfigs", "machineconfigpools"]
verbs: ["*"]
- apiGroups: [""]
resources: ["configmaps", "secrets"]
verbs: ["get", "list", "watch"]
6 changes: 6 additions & 0 deletions manifests/machineconfigserver/daemonset.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -49,3 +49,9 @@ spec:
- name: certs
secret:
secretName: machine-config-server-tls
# See https://github.com/openshift/enhancements/pull/443
# This will only exist in 4.7+ clusters by default.
- name: provisioning-token
secret:
secretName: provisioning-token
optional: true
9 changes: 9 additions & 0 deletions pkg/operator/assets/bindata.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

19 changes: 15 additions & 4 deletions pkg/server/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@ const (

type poolRequest struct {
machineConfigPool string
version *semver.Version
// The provisioning token, see https://github.com/openshift/enhancements/pull/443
token string
version *semver.Version
}

// APIServer provides the HTTP(s) endpoint
Expand Down Expand Up @@ -114,7 +116,10 @@ func (sh *APIHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
poolName := path.Base(r.URL.Path)
useragent := r.Header.Get("User-Agent")
acceptHeader := r.Header.Get("Accept")
glog.Infof("Pool %s requested by address:%q User-Agent:%q Accept-Header: %q", poolName, r.RemoteAddr, useragent, acceptHeader)
q := r.URL.Query()
token := q.Get("token")
tokenProvided := token != ""
glog.Infof("Pool %s requested by address:%q User-Agent:%q Accept-Header: %q TokenPresent: %v", poolName, r.RemoteAddr, useragent, acceptHeader, tokenProvided)

reqConfigVer, err := detectSpecVersionFromAcceptHeader(acceptHeader)
if err != nil {
Expand All @@ -126,14 +131,20 @@ func (sh *APIHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {

cr := poolRequest{
machineConfigPool: poolName,
token: token,
version: reqConfigVer,
}

conf, err := sh.server.GetConfig(cr)
if err != nil {
w.Header().Set("Content-Length", "0")
w.WriteHeader(http.StatusInternalServerError)
glog.Errorf("couldn't get config for req: %v, error: %v", cr, err)
if IsForbidden(err) {
w.WriteHeader(http.StatusForbidden)
glog.Infof("Denying unauthorized request: %v", err)
} else {
w.WriteHeader(http.StatusInternalServerError)
glog.Errorf("couldn't get config for req: %v, error: %v", cr, err)
}
return
}
if conf == nil {
Expand Down
41 changes: 41 additions & 0 deletions pkg/server/cluster_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,16 @@ import (
"fmt"
"io/ioutil"
"path/filepath"
"time"

yaml "github.com/ghodss/yaml"
ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common"
"github.com/pkg/errors"
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/client-go/kubernetes"
rest "k8s.io/client-go/rest"
clientcmd "k8s.io/client-go/tools/clientcmd"
clientcmdv1 "k8s.io/client-go/tools/clientcmd/api/v1"
Expand All @@ -33,6 +37,8 @@ const (
var _ = Server(&clusterServer{})

type clusterServer struct {
client kubernetes.Interface

// machineClient is used to interact with the
// machine config, pool objects.
machineClient v1.MachineconfigurationV1Interface
Expand All @@ -52,16 +58,51 @@ func NewClusterServer(kubeConfig, apiserverURL string) (Server, error) {
return nil, fmt.Errorf("Failed to create Kubernetes rest client: %v", err)
}

client, err := kubernetes.NewForConfig(restConfig)
if err != nil {
return nil, errors.Wrapf(err, "creating core client")
}

mc := v1.NewForConfigOrDie(restConfig)
return &clusterServer{
client: client,
machineClient: mc,
kubeconfigFunc: func() ([]byte, []byte, error) { return kubeconfigFromSecret(bootstrapTokenDir, apiserverURL) },
}, nil
}

// authorizeRequest checks the provided token
func (cs *clusterServer) authorizeRequest(cr poolRequest) (bool, error) {
s, err := cs.client.CoreV1().Secrets("openshift-machine-config-operator").Get(context.TODO(), "provisioning-token", metav1.GetOptions{})
if err != nil {
if !apierrors.IsNotFound(err) {
return false, errors.Wrapf(err, "Fetching provisioning-token")
}
// If the cluster doesn't have a `provisioning-token` secret, we don't require it.
return true, nil
}
// Unconditionally sleep to mitigate brute force attacks
time.Sleep(1 * time.Second)
if cr.token != string(s.Data["token"]) {
return false, nil
}
return true, nil
}

// GetConfig fetches the machine config(type - Ignition) from the cluster,
// based on the pool request.
func (cs *clusterServer) GetConfig(cr poolRequest) (*runtime.RawExtension, error) {
authorized, err := cs.authorizeRequest(cr)
if err != nil {
return nil, err
}
if !authorized {
return nil, &configError{
msg: "Provided token is invalid",
forbidden: true,
}
}

mp, err := cs.machineClient.MachineConfigPools().Get(context.TODO(), cr.machineConfigPool, metav1.GetOptions{})
if err != nil {
return nil, fmt.Errorf("could not fetch pool. err: %v", err)
Expand Down
21 changes: 21 additions & 0 deletions pkg/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,27 @@ type kubeconfigFunc func() (kubeconfigData []byte, rootCAData []byte, err error)
// appenderFunc appends Config.
type appenderFunc func(*igntypes.Config, *mcfgv1.MachineConfig) error

// configError is returned by the GetConfig API
type configError struct {
msg string
forbidden bool
}

// configError returns the string
func (e *configError) Error() string {
return e.msg
}

// IsForbidden says if err is an configError with forbidden set
func IsForbidden(err error) bool {
switch t := err.(type) {
case *configError:
return t.forbidden
default:
return false
}
}

// Server defines the interface that is implemented by different
// machine config server implementations.
type Server interface {
Expand Down
34 changes: 34 additions & 0 deletions pkg/server/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"github.com/stretchr/testify/assert"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
k8sfake "k8s.io/client-go/kubernetes/fake"

mcfgv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1"
ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common"
Expand Down Expand Up @@ -243,6 +244,8 @@ func TestClusterServer(t *testing.T) {
t.Fatalf("unexpected error while unmarshaling machine-config: %s, err: %v", mcPath, err)
}

basecs := k8sfake.NewSimpleClientset()

cs := fake.NewSimpleClientset()
_, err = cs.MachineconfigurationV1().MachineConfigPools().Create(context.TODO(), mp, metav1.CreateOptions{})
if err != nil {
Expand All @@ -254,6 +257,7 @@ func TestClusterServer(t *testing.T) {
}

csc := &clusterServer{
client: basecs,
machineClient: cs.MachineconfigurationV1(),
kubeconfigFunc: func() ([]byte, []byte, error) { return getKubeConfigContent(t) },
}
Expand Down Expand Up @@ -320,6 +324,36 @@ func TestClusterServer(t *testing.T) {
if !foundEncapsulated {
t.Errorf("missing %s", daemonconsts.MachineConfigEncapsulatedPath)
}

// Test https://github.com/openshift/enhancements/pull/443
provisioningSecret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{Name: "provisioning-token"},
Data: map[string][]byte{
"token": []byte("somesecrettoken"),
},
}
basecs.CoreV1().Secrets("openshift-machine-config-operator").Create(context.TODO(), provisioningSecret, metav1.CreateOptions{})

// Do a request without a token
res, err = csc.GetConfig(poolRequest{
machineConfigPool: testPool,
})
assert.Error(t, err)

// Incorrect token
res, err = csc.GetConfig(poolRequest{
token: "someothertoken",
machineConfigPool: testPool,
})
assert.Error(t, err)

// Valid token
res, err = csc.GetConfig(poolRequest{
token: "somesecrettoken",
machineConfigPool: testPool,
})
assert.Nil(t, err)
assert.NotNil(t, res)
}

func getKubeConfigContent(t *testing.T) ([]byte, []byte, error) {
Expand Down

0 comments on commit 193679e

Please sign in to comment.