Skip to content

Commit

Permalink
Add AllowedRegistriesForImport to allow whitelisting registries allow…
Browse files Browse the repository at this point in the history
…ed for import
  • Loading branch information
mfojtik committed Apr 6, 2017
1 parent 83e3250 commit f16a9cf
Show file tree
Hide file tree
Showing 16 changed files with 337 additions and 62 deletions.
7 changes: 7 additions & 0 deletions pkg/cmd/server/api/serialization_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ func fuzzInternalObject(t *testing.T, forVersion unversioned.GroupVersion, item
if len(obj.APILevels) == 0 {
obj.APILevels = configapi.DefaultOpenShiftAPILevels
}
if obj.ImagePolicyConfig.AllowedRegistriesForImport == nil {
obj.ImagePolicyConfig.AllowedRegistriesForImport = &configapi.AllowedRegistries{}
}
if len(obj.Controllers) == 0 {
obj.Controllers = configapi.ControllersAll
}
Expand Down Expand Up @@ -259,6 +262,10 @@ func fuzzInternalObject(t *testing.T, forVersion unversioned.GroupVersion, item
if obj.ScheduledImageImportMinimumIntervalSeconds == 0 {
obj.ScheduledImageImportMinimumIntervalSeconds = 15 * 60
}
obj.AllowedRegistriesForImport = &configapi.AllowedRegistries{
{DomainName: "docker.io"},
{DomainName: "gcr.io"},
}
},
func(obj *configapi.DNSConfig, c fuzz.Continue) {
c.FuzzNoCustom(obj)
Expand Down
39 changes: 39 additions & 0 deletions pkg/cmd/server/api/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,23 @@ var (
}
KnownOpenShiftFeatures = []string{FeatureBuilder, FeatureS2I, FeatureWebConsole}
AtomicDisabledFeatures = []string{FeatureBuilder, FeatureS2I, FeatureWebConsole}

// List public registries that we are allowing to import images from by default.
// By default all registries have set to be "secure", iow. the port for them is
// defaulted to "443".
// If the registry you are adding here is insecure, you can add 'Insecure: true' to
// make it default to port '80'.
// If the registry you are adding use custom port, you have to specify the port as
// part of the domain name.
DefaultAllowedRegistriesForImport = &AllowedRegistries{
{DomainName: "docker.io"},
{DomainName: "*.docker.io"}, // registry-1.docker.io
{DomainName: "registry.access.redhat.com"},
{DomainName: "gcr.io"},
{DomainName: "quay.io"},
// FIXME: Probably need to have more fine-tuned pattern defined
{DomainName: "*.amazonaws.com"},
}
)

type ExtendedArguments map[string][]string
Expand Down Expand Up @@ -455,6 +472,28 @@ type ImagePolicyConfig struct {
// MaxScheduledImageImportsPerMinute is the maximum number of image streams that will be imported in the background per minute.
// The default value is 60. Set to -1 for unlimited.
MaxScheduledImageImportsPerMinute int
// AllowedRegistriesForImport limits the docker registries that normal users may import
// images from. Set this list to the registries that you trust to contain valid Docker
// images and that you want applications to be able to import from. Users with
// permission to create Images or ImageStreamMappings via the API are not affected by
// this policy - typically only administrators or system integrations will have those
// permissions.
AllowedRegistriesForImport *AllowedRegistries
}

// AllowedRegistries represents a list of registries allowed for the image import.
type AllowedRegistries []RegistryLocation

// RegistryLocation contains a location of the registry specified by the registry domain
// name. The domain name might include wildcards, like '*' or '??'.
type RegistryLocation struct {
// DomainName specifies a domain name for the registry
// In case the registry use non-standard (80 or 443) port, the port should be included
// in the domain name as well.
DomainName string
// Insecure indicates whether the registry is secure (https) or insecure (http)
// By default (if not specified) the registry is assumed as secure.
Insecure bool
}

type ProjectConfig struct {
Expand Down
22 changes: 22 additions & 0 deletions pkg/cmd/server/api/v1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,28 @@ type ImagePolicyConfig struct {
// MaxScheduledImageImportsPerMinute is the maximum number of scheduled image streams that will be imported in the
// background per minute. The default value is 60. Set to -1 for unlimited.
MaxScheduledImageImportsPerMinute int `json:"maxScheduledImageImportsPerMinute"`
// AllowedRegistriesForImport limits the docker registries that normal users may import
// images from. Set this list to the registries that you trust to contain valid Docker
// images and that you want applications to be able to import from. Users with
// permission to create Images or ImageStreamMappings via the API are not affected by
// this policy - typically only administrators or system integrations will have those
// permissions.
AllowedRegistriesForImport *AllowedRegistries `json:"allowedRegistriesForImport,omitempty"`
}

// AllowedRegistries represents a list of registries allowed for the image import.
type AllowedRegistries []RegistryLocation

// RegistryLocation contains a location of the registry specified by the registry domain
// name. The domain name might include wildcards, like '*' or '??'.
type RegistryLocation struct {
// DomainName specifies a domain name for the registry
// In case the registry use non-standard (80 or 443) port, the port should be included
// in the domain name as well.
DomainName string `json:"domainName"`
// Insecure indicates whether the registry is secure (https) or insecure (http)
// By default (if not specified) the registry is assumed as secure.
Insecure bool `json:"insecure,omitempty"`
}

// holds the necessary configuration options for
Expand Down
23 changes: 23 additions & 0 deletions pkg/cmd/server/api/validation/master.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"net/url"
"reflect"
"regexp"
"strconv"
"strings"
"time"

Expand Down Expand Up @@ -473,6 +474,28 @@ func ValidateImagePolicyConfig(config api.ImagePolicyConfig, fldPath *field.Path
if config.MaxScheduledImageImportsPerMinute == 0 || config.MaxScheduledImageImportsPerMinute < -1 {
errs = append(errs, field.Invalid(fldPath.Child("maxScheduledImageImportsPerMinute"), config.MaxScheduledImageImportsPerMinute, "must be a positive integer or -1"))
}
if config.AllowedRegistriesForImport != nil {
for i, registry := range *config.AllowedRegistriesForImport {
if len(registry.DomainName) == 0 {
errs = append(errs, field.Invalid(fldPath.Index(i).Child("allowedRegistriesForImport", "domainName"), registry.DomainName, "cannot be an empty string"))
}
parts := strings.Split(registry.DomainName, ":")
// Check for ':8080'
if len(parts) == 0 || len(parts[0]) == 0 {
errs = append(errs, field.Invalid(fldPath.Index(i).Child("allowedRegistriesForImport", "domainName"), registry.DomainName, "invalid domain specified, must be registry.url.local[:port]"))
}
// Check for 'foo:bar:1234'
if len(parts) > 2 {
errs = append(errs, field.Invalid(fldPath.Index(i).Child("allowedRegistriesForImport", "domainName"), registry.DomainName, "invalid format, must be registry.url.local[:port]"))
}
// Check for 'foo:bar'
if len(parts) == 2 {
if _, err := strconv.Atoi(parts[1]); err != nil {
errs = append(errs, field.Invalid(fldPath.Index(i).Child("allowedRegistriesForImport", "domainName"), registry.DomainName, "invalid port format, must be a number"))
}
}
}
}
return errs
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/server/origin/master.go
Original file line number Diff line number Diff line change
Expand Up @@ -729,7 +729,7 @@ func (c *MasterConfig) GetRestStorage() map[unversioned.GroupVersion]map[string]
importerDockerClientFn := func() dockerregistry.Client {
return dockerregistry.NewClient(20*time.Second, false)
}
imageStreamImportStorage := imagestreamimport.NewREST(importerFn, imageStreamRegistry, internalImageStreamStorage, imageStorage, c.ImageStreamImportSecretClient(), importTransport, insecureImportTransport, importerDockerClientFn)
imageStreamImportStorage := imagestreamimport.NewREST(importerFn, imageStreamRegistry, internalImageStreamStorage, imageStorage, c.ImageStreamImportSecretClient(), importTransport, insecureImportTransport, importerDockerClientFn, c.Options.ImagePolicyConfig.AllowedRegistriesForImport, c.RegistryNameFn, c.ImageStreamImportSARClient().SubjectAccessReviews())
imageStreamImageStorage := imagestreamimage.NewREST(imageRegistry, imageStreamRegistry)
imageStreamImageRegistry := imagestreamimage.NewRegistry(imageStreamImageStorage)

Expand Down
5 changes: 5 additions & 0 deletions pkg/cmd/server/origin/master_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -996,6 +996,11 @@ func (c *MasterConfig) ImageStreamImportSecretClient() *osclient.Client {
return c.PrivilegedLoopbackOpenShiftClient
}

// ImageStreamImportSARClient returns the client capable of performing self-SAR requests
func (c *MasterConfig) ImageStreamImportSARClient() *osclient.Client {
return c.PrivilegedLoopbackOpenShiftClient
}

// ResourceQuotaManagerClients returns the client capable of retrieving resources needed for resource quota
// evaluation
func (c *MasterConfig) ResourceQuotaManagerClients() (*osclient.Client, *kclientset.Clientset) {
Expand Down
11 changes: 11 additions & 0 deletions pkg/cmd/server/start/master_args.go
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,17 @@ func (args MasterArgs) BuildSerializeableMasterConfig() (*configapi.MasterConfig
Latest: args.ImageFormatArgs.ImageTemplate.Latest,
},

// List public registries that we are allowing to import images from by default.
// By default all registries have set to be "secure", iow. the port for them is
// defaulted to "443".
// If the registry you are adding here is insecure, you can add 'Insecure: true' which
// in that case it will default to port '80'.
// If the registry you are adding use custom port, you have to specify the port as
// part of the domain name.
ImagePolicyConfig: configapi.ImagePolicyConfig{
AllowedRegistriesForImport: configapi.DefaultAllowedRegistriesForImport,
},

ProjectConfig: configapi.ProjectConfig{
DefaultNodeSelector: "",
ProjectRequestMessage: "",
Expand Down
20 changes: 20 additions & 0 deletions pkg/image/api/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"bytes"
"encoding/json"
"fmt"
"net"
"net/url"
"regexp"
"sort"
Expand Down Expand Up @@ -34,6 +35,10 @@ const (

// TagReferenceAnnotationTagHidden indicates that a given TagReference is hidden from search results
TagReferenceAnnotationTagHidden = "hidden"

// ImportRegistryNotAllowed indicates that the image tag was not imported due to
// untrusted registry.
ImportRegistryNotAllowed = "registry is not allowed for import"
)

// DefaultRegistry returns the default Docker registry (host or host:port), or false if it is not available.
Expand Down Expand Up @@ -168,6 +173,21 @@ func (r DockerImageReference) RepositoryName() string {
return r.Exact()
}

// RegistryHostPort returns the registry hostname and the port.
// If the port is not specified in the registry hostname we default to 443.
// This will also default to Docker client defaults if the registry hostname is empty.
func (r DockerImageReference) RegistryHostPort(insecure bool) (string, string) {
registryHost := r.AsV2().DockerClientDefaults().Registry
if strings.Contains(registryHost, ":") {
hostname, port, _ := net.SplitHostPort(registryHost)
return hostname, port
}
if insecure {
return registryHost, "80"
}
return registryHost, "443"
}

// RepositoryName returns the registry relative name
func (r DockerImageReference) RegistryURL() *url.URL {
return &url.URL{
Expand Down
33 changes: 33 additions & 0 deletions pkg/image/api/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ import (
"k8s.io/kubernetes/pkg/util/diff"
"k8s.io/kubernetes/pkg/util/validation/field"

serverapi "github.com/openshift/origin/pkg/cmd/server/api"
"github.com/openshift/origin/pkg/image/api"
stringsutil "github.com/openshift/origin/pkg/util/strings"
)

// RepositoryNameComponentRegexp restricts registry path component names to
Expand Down Expand Up @@ -297,6 +299,37 @@ func ValidateImageStreamTagUpdate(newIST, oldIST *api.ImageStreamTag) field.Erro
return result
}

func ValidateRegistryAllowedForImport(path *field.Path, name, registryHost, registryPort string, allowedRegistries *serverapi.AllowedRegistries) field.ErrorList {
errs := field.ErrorList{}
if allowedRegistries == nil {
return errs
}
allowedRegistriesForHumans := []string{}
for _, registry := range *allowedRegistries {
allowedRegistryHost, allowedRegistryPort := "", ""
parts := strings.Split(registry.DomainName, ":")
switch len(parts) {
case 1:
allowedRegistryHost = parts[0]
if registry.Insecure {
allowedRegistryPort = "80"
} else {
allowedRegistryPort = "443"
}
case 2:
allowedRegistryHost, allowedRegistryPort = parts[0], parts[1]
default:
continue
}
if stringsutil.IsWildcardMatch(registryHost, allowedRegistryHost) && stringsutil.IsWildcardMatch(registryPort, allowedRegistryPort) {
return errs
}
allowedRegistriesForHumans = append(allowedRegistriesForHumans, registry.DomainName)
}
return append(errs, field.Invalid(path, name,
fmt.Sprintf("importing images from registry %q is forbidden, only images from %q are allowed", registryHost+":"+registryPort, strings.Join(allowedRegistriesForHumans, ","))))
}

func ValidateImageStreamImport(isi *api.ImageStreamImport) field.ErrorList {
specPath := field.NewPath("spec")
imagesPath := specPath.Child("images")
Expand Down
55 changes: 53 additions & 2 deletions pkg/image/registry/imagestreamimport/rest.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ import (
utilruntime "k8s.io/kubernetes/pkg/util/runtime"
"k8s.io/kubernetes/pkg/util/validation/field"

authorizationapi "github.com/openshift/origin/pkg/authorization/api"
"github.com/openshift/origin/pkg/client"
serverapi "github.com/openshift/origin/pkg/cmd/server/api"
"github.com/openshift/origin/pkg/dockerregistry"
"github.com/openshift/origin/pkg/image/api"
imageapiv1 "github.com/openshift/origin/pkg/image/api/v1"
Expand All @@ -43,6 +45,8 @@ type REST struct {
transport http.RoundTripper
insecureTransport http.RoundTripper
clientFn ImporterDockerRegistryFunc
strategy *strategy
sarClient client.SubjectAccessReviewInterface
}

// NewREST returns a REST storage implementation that handles importing images. The clientFn argument is optional
Expand All @@ -52,6 +56,9 @@ func NewREST(importFn ImporterFunc, streams imagestream.Registry, internalStream
images rest.Creater, secrets client.ImageStreamSecretsNamespacer,
transport, insecureTransport http.RoundTripper,
clientFn ImporterDockerRegistryFunc,
allowedImportRegistries *serverapi.AllowedRegistries,
registryFn api.DefaultRegistryFunc,
sarClient client.SubjectAccessReviewInterface,
) *REST {
return &REST{
importFn: importFn,
Expand All @@ -62,6 +69,8 @@ func NewREST(importFn ImporterFunc, streams imagestream.Registry, internalStream
transport: transport,
insecureTransport: insecureTransport,
clientFn: clientFn,
strategy: NewStrategy(allowedImportRegistries, registryFn),
sarClient: sarClient,
}
}

Expand All @@ -78,10 +87,52 @@ func (r *REST) Create(ctx kapi.Context, obj runtime.Object) (runtime.Object, err

inputMeta := isi.ObjectMeta

if err := rest.BeforeCreate(Strategy, ctx, obj); err != nil {
if err := rest.BeforeCreate(r.strategy, ctx, obj); err != nil {
return nil, err
}

// Check if the user is allowed to create Images or ImageStreamMappings.
// In case the user is allowed to create them, do not validate the ImageStreamImport
// registry location against the registry whitelist, but instead allow to create any
// image from any registry.
user, ok := kapi.UserFrom(ctx)
if !ok {
return nil, kapierrors.NewBadRequest("unable to get user from context")
}
isCreateImage, err := r.sarClient.Create(
&authorizationapi.SubjectAccessReview{
User: user.GetName(),
Action: authorizationapi.Action{
Verb: "create",
Group: api.GroupName,
Resource: "images",
},
},
)
if err != nil {
return nil, err
}

isCreateImageStreamMapping, err := r.sarClient.Create(
&authorizationapi.SubjectAccessReview{
User: user.GetName(),
Action: authorizationapi.Action{
Verb: "create",
Group: api.GroupName,
Resource: "imagestreammapping",
},
},
)
if err != nil {
return nil, err
}

if !isCreateImage.Allowed && !isCreateImageStreamMapping.Allowed {
if errs := r.strategy.ValidateAllowedRegistries(isi); len(errs) != 0 {
return nil, kapierrors.NewInvalid(api.Kind("ImageStreamImport"), isi.Name, errs)
}
}

namespace, ok := kapi.NamespaceFrom(ctx)
if !ok {
return nil, kapierrors.NewBadRequest("a namespace must be specified to import images")
Expand Down Expand Up @@ -374,7 +425,7 @@ func (r *REST) importSuccessful(
importPolicy api.TagImportPolicy, referencePolicy api.TagReferencePolicy,
importedImages map[string]error, updatedImages map[string]*api.Image,
) (*api.Image, bool) {
Strategy.PrepareImageForCreate(image)
r.strategy.PrepareImageForCreate(image)

pullSpec, _ := api.MostAccuratePullSpec(image.DockerImageReference, image.Name, "")
tagEvent := api.TagEvent{
Expand Down
Loading

0 comments on commit f16a9cf

Please sign in to comment.