Skip to content

Commit

Permalink
server: Support denying serving Ignition to active nodes and pods
Browse files Browse the repository at this point in the history
Ignition may contain secret data; pods running on the cluster
shouldn't have access.

This adds opt-in support for denying serving that data.  It is
disabled by default so we can check whether this would happen
in any CI scenarios to start.  Run
`oc -n openshift-machine-config-operator create configmap machine-config-server provision-check=yes`
to switch to enforcing mode.

First, we deny any request that appears to come from the pod overlay
network.  This closes off a lot of avenues without any risk.

However, we can't guarantee all in-cluster requests appear to originate from
the pod network; in some cases according to the SDN team, particularly
for machines that have multiple NICs.

Hence, this PR also closes off access to any IP that responds on
port 22, as that is a port that is:

 - Known to be active by default
 - Not firewalled

A previous attempt at this was to have an [auth token](openshift#736);
but this fix doesn't require changing the installer and people's PXE setups.

In the future we may reserve a port in the 9xxx range and have the
MCD respond on it so that admins who disable/firewall SSH don't
have indirectly reduced security.
  • Loading branch information
cgwalters committed Apr 22, 2020
1 parent e7baf4e commit 75db5c2
Show file tree
Hide file tree
Showing 5 changed files with 201 additions and 4 deletions.
6 changes: 6 additions & 0 deletions manifests/machineconfigserver/clusterrole.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,12 @@ metadata:
name: machine-config-server
namespace: {{.TargetNamespace}}
rules:
- apiGroups: [""]
resources: ["configmaps"]
verbs: ["get", "list", "watch"]
- apiGroups: ["machineconfiguration.openshift.io"]
resources: ["machineconfigs", "machineconfigpools"]
verbs: ["*"]
- apiGroups: ["config.openshift.io"]
resources: ["networks"]
verbs: ["get", "list", "watch"]
6 changes: 6 additions & 0 deletions pkg/operator/assets/bindata.go
Original file line number Diff line number Diff line change
Expand Up @@ -2536,9 +2536,15 @@ metadata:
name: machine-config-server
namespace: {{.TargetNamespace}}
rules:
- apiGroups: [""]
resources: ["configmaps"]
verbs: ["get", "list", "watch"]
- apiGroups: ["machineconfiguration.openshift.io"]
resources: ["machineconfigs", "machineconfigpools"]
verbs: ["*"]
- apiGroups: ["config.openshift.io"]
resources: ["networks"]
verbs: ["get", "list", "watch"]
`)

func manifestsMachineconfigserverClusterroleYamlBytes() ([]byte, error) {
Expand Down
11 changes: 9 additions & 2 deletions pkg/server/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

type poolRequest struct {
machineConfigPool string
remoteAddr string
}

// APIServer provides the HTTP(s) endpoint
Expand Down Expand Up @@ -92,15 +93,21 @@ func (sh *APIHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {

cr := poolRequest{
machineConfigPool: path.Base(r.URL.Path),
remoteAddr: r.RemoteAddr,
}

glog.Infof("Pool %s requested by %s", cr.machineConfigPool, r.RemoteAddr)

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
161 changes: 159 additions & 2 deletions pkg/server/cluster_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,25 @@ import (
"context"
"fmt"
"io/ioutil"
"net"
"path/filepath"
"sync"
"time"

"github.com/pkg/errors"

yaml "github.com/ghodss/yaml"
"github.com/golang/glog"
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"

oconfigv1 "github.com/openshift/client-go/config/clientset/versioned/typed/config/v1"
v1 "github.com/openshift/machine-config-operator/pkg/generated/clientset/versioned/typed/machineconfiguration.openshift.io/v1"
)

Expand All @@ -24,13 +33,27 @@ const (

//nolint:gosec
bootstrapTokenDir = "/etc/mcs/bootstrap-token"

// machineProvisionedTimeoutSecs is the maximum amount of time we will wait for a reply
// to the port. Note that Ignition by default times out requests after 10 seconds, so
// we need to be lower than that.
machineProvisionedTimeoutSecs = 5
)

// machineProvisionedPorts is a set of TCP ports the MCS connects to in order
// to check if a machine has already been provisioned. For now this is
// the SSH port and the kubelet. In the future we might have the MCD itself listen
// on a port that's inside the reserved 9000-9900 range.
var machineProvisionedPorts = []string{"22", "10250"}

// ensure clusterServer implements the
// Server interface.
var _ = Server(&clusterServer{})

type clusterServer struct {
client kubernetes.Clientset

configClient oconfigv1.ConfigV1Client
// machineClient is used to interact with the
// machine config, pool objects.
machineClient v1.MachineconfigurationV1Interface
Expand All @@ -47,16 +70,129 @@ type clusterServer struct {
func NewClusterServer(kubeConfig, apiserverURL string) (Server, error) {
restConfig, err := getClientConfig(kubeConfig)
if err != nil {
return nil, fmt.Errorf("Failed to create Kubernetes rest client: %v", err)
return nil, errors.Wrapf(err, "Failed to create Kubernetes rest client")
}

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

mc := v1.NewForConfigOrDie(restConfig)
mc, err := v1.NewForConfig(restConfig)
if err != nil {
return nil, errors.Wrapf(err, "creating mc client")
}
oc, err := oconfigv1.NewForConfig(restConfig)
if err != nil {
return nil, errors.Wrapf(err, "creating config client")
}
return &clusterServer{
client: *client,
machineClient: mc,
configClient: *oc,
kubeconfigFunc: func() ([]byte, []byte, error) { return kubeconfigFromSecret(bootstrapTokenDir, apiserverURL) },
}, nil
}

// remoteAddrIsProvisioned returns an error if the given IP address responds on a known port.
// If an error is returned, the machine is already up and should not be served Ignition.
func remoteAddrIsProvisioned(remoteAddr string) error {
startTime := time.Now()
defer func() {
glog.Infof("Checked address %s for provisioning in %v", remoteAddr, time.Since(startTime))
}()
var wg sync.WaitGroup
provisionedPort := make(chan error)
wg.Add(len(machineProvisionedPorts))
for _, port := range machineProvisionedPorts {
go func(port string) {
conn, err := net.DialTimeout("tcp", net.JoinHostPort(remoteAddr, port), time.Second*machineProvisionedTimeoutSecs)
if err != nil {
glog.Infof("Checking provisioned port %s: %v", port, err)
} else {
provisionedPort <- fmt.Errorf("Address %s responds on port %s; already provisioned", remoteAddr, port)
conn.Close()
}
wg.Done()
}(port)
}
// The above goroutines race to return the first port we find that is provisioned.
// But in the case where none are found, this goroutine waits for their completion
// and sends the empty string.
go func() {
wg.Wait()
provisionedPort <- nil
}()
ret := <-provisionedPort
return ret
}

// remoteAddrIsFromPod returns a non-empty string containing a CIDR if the given remote IP
// is from a pod. We don't want to allow
// pods running on a node to potentially access Ignition; it should
// only be read by the Ignition software itself when the machine
// boots up, before it joins the cluster.
// If the remote address is OK, then the empty string "" is returned.
// Otherwise, error is set.
func remoteAddrIsFromPod(configClient oconfigv1.ConfigV1Client, remoteAddr string) (string, error) {
network, err := configClient.Networks().Get(context.TODO(), "cluster", metav1.GetOptions{})
if err != nil {
return "", err
}
remoteIP := net.ParseIP(remoteAddr)
if remoteIP == nil {
return "", nil
}
for _, n := range network.Status.ClusterNetwork {
_, ipr, err := net.ParseCIDR(n.CIDR)
if err != nil {
glog.V(4).Infof("Parsing CIDR %s: %v", n.CIDR, err)
continue
}
if ipr.Contains(remoteIP) {
return n.CIDR, nil
}
}
return "", nil
}

func isLocalhost(addr string) bool {
return addr == "127.0.0.1" || addr == "::1"
}

// Don't serve Ignition to extant nodes; see
// https://bugzilla.redhat.com/show_bug.cgi?id=1707162
// https://github.com/openshift/machine-config-operator/issues/731
func (cs *clusterServer) shouldServeIgnition(remoteAddr string) error {
host, _, err := net.SplitHostPort(remoteAddr)
if err != nil {
return err
}

if !isLocalhost(host) {
fromPod, err := remoteAddrIsFromPod(cs.configClient, host)
if err != nil {
return err
}
if fromPod != "" {
return &configError{
msg: fmt.Sprintf("Address %s is in the pod network %s", host, fromPod),
forbidden: true,
}
}

err = remoteAddrIsProvisioned(host)
if err != nil {
return &configError{
msg: err.Error(),
forbidden: true,
}
}
}

return 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) {
Expand All @@ -65,6 +201,27 @@ func (cs *clusterServer) GetConfig(cr poolRequest) (*runtime.RawExtension, error
return nil, fmt.Errorf("could not fetch pool. err: %v", err)
}

if cr.remoteAddr != "" {
// By default for now, we run in warn-only mode
provisionCheckError := false
config, err := cs.client.CoreV1().ConfigMaps("openshift-machine-config-operator").Get(context.TODO(), "machine-config-server", metav1.GetOptions{})
if err != nil {
if !apierrors.IsNotFound(err) {
return nil, errors.Wrapf(err, "Fetching mcs config")
}
} else {
if config.Data["provision-check"] != "" {
provisionCheckError = true
}
}
if err := cs.shouldServeIgnition(cr.remoteAddr); err != nil {
if provisionCheckError {
return nil, err
}
glog.Warningf("Would deny serving ignition (enable configmap/machine-config-server/provision-check to enforce): %s", err)
}
}

currConf := mp.Status.Configuration.Name

mc, err := cs.machineClient.MachineConfigs().Get(context.TODO(), currConf, metav1.GetOptions{})
Expand Down
21 changes: 21 additions & 0 deletions pkg/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,27 @@ type kubeconfigFunc func() (kubeconfigData []byte, rootCAData []byte, err error)
// appenderFunc appends Config.
type appenderFunc func(*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

0 comments on commit 75db5c2

Please sign in to comment.