Skip to content

Commit

Permalink
fix interacting with insecure HTTPS registries
Browse files Browse the repository at this point in the history
The Docker daemon allows to interact with insecure registries served
through plain HTTP or served through HTTPS with self-signed
certificates, when the target registry is included inside
"insecureRegistries". In this library it should be possible to interact
with insecure registries likewise by using the "name.Insecure" option
when creating references.

Nonetheless it's currently not possible to interact with insecure
registries served with HTTPS and self-signed certificates, since the
TLS certificate is checked anyway and an "invalid certificate" error is
returned.

A common workaround consists into passing a tls.Config with
InsecureSkipVerify set to true, but this disables TLS validation for
every HTTP request, while the desired behavior is disabling TLS
validation only when "name.Insecure" is in use.

This patch changes the default "remote" options in order to provide a
default tls.Config with InsecureSkipVerify set to true if and only if
"name.Insecure" is in use.

This also fixes bugs in dependent tools like Skaffold, that are using
"name.Insecure", are not using InsecureSKipVerify and are expecting to
be able to interact with insecure registries anyway.
  • Loading branch information
aler9 committed Oct 22, 2024
1 parent 4630c40 commit 38ffa80
Show file tree
Hide file tree
Showing 13 changed files with 146 additions and 16 deletions.
5 changes: 5 additions & 0 deletions pkg/name/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,11 @@ func (r Registry) isRFC1918() bool {
return false
}

// IsInsecure checks whether the registry is insecure.
func (r Registry) IsInsecure() bool {
return r.insecure
}

// Scheme returns https scheme for all the endpoints except localhost or when explicitly defined.
func (r Registry) Scheme() string {
if r.insecure {
Expand Down
4 changes: 2 additions & 2 deletions pkg/v1/remote/catalog.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ type Catalogs struct {

// CatalogPage calls /_catalog, returning the list of repositories on the registry.
func CatalogPage(target name.Registry, last string, n int, options ...Option) ([]string, error) {
o, err := makeOptions(options...)
o, err := makeOptions(target, options...)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -73,7 +73,7 @@ func CatalogPage(target name.Registry, last string, n int, options ...Option) ([

// Catalog calls /_catalog, returning the list of repositories on the registry.
func Catalog(ctx context.Context, target name.Registry, options ...Option) ([]string, error) {
o, err := makeOptions(options...)
o, err := makeOptions(target, options...)
if err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/v1/remote/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import (

// Delete removes the specified image reference from the remote registry.
func Delete(ref name.Reference, options ...Option) error {
o, err := makeOptions(options...)
o, err := makeOptions(ref.Context(), options...)
if err != nil {
return err
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/v1/remote/descriptor.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ func Get(ref name.Reference, options ...Option) (*Descriptor, error) {
// Note that the server response will not have a body, so any errors encountered
// should be retried with Get to get more details.
func Head(ref name.Reference, options ...Option) (*v1.Descriptor, error) {
o, err := makeOptions(options...)
o, err := makeOptions(ref.Context(), options...)
if err != nil {
return nil, err
}
Expand All @@ -90,7 +90,7 @@ func Head(ref name.Reference, options ...Option) (*v1.Descriptor, error) {
// Handle options and fetch the manifest with the acceptable MediaTypes in the
// Accept header.
func get(ref name.Reference, acceptable []types.MediaType, options ...Option) (*Descriptor, error) {
o, err := makeOptions(options...)
o, err := makeOptions(ref.Context(), options...)
if err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/v1/remote/layer.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func (rl *remoteLayer) Exists() (bool, error) {
// digest of the blob to be read and the repository portion is the repo where
// that blob lives.
func Layer(ref name.Digest, options ...Option) (v1.Layer, error) {
o, err := makeOptions(options...)
o, err := makeOptions(ref.Context(), options...)
if err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/v1/remote/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func ListWithContext(ctx context.Context, repo name.Repository, options ...Optio
// List calls /tags/list for the given repository, returning the list of tags
// in the "tags" property.
func List(repo name.Repository, options ...Option) ([]string, error) {
o, err := makeOptions(options...)
o, err := makeOptions(repo, options...)
if err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/v1/remote/multi_write.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import (
// efficiently as possible, by deduping shared layer blobs while uploading them
// in parallel.
func MultiWrite(todo map[name.Reference]Taggable, options ...Option) (rerr error) {
o, err := makeOptions(options...)
o, err := makeOptions(nil, options...)
if err != nil {
return err
}
Expand Down
34 changes: 32 additions & 2 deletions pkg/v1/remote/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package remote

import (
"context"
"crypto/tls"
"errors"
"io"
"net"
Expand All @@ -26,6 +27,7 @@ import (
"github.com/google/go-containerregistry/internal/retry"
"github.com/google/go-containerregistry/pkg/authn"
"github.com/google/go-containerregistry/pkg/logs"
"github.com/google/go-containerregistry/pkg/name"
v1 "github.com/google/go-containerregistry/pkg/v1"
"github.com/google/go-containerregistry/pkg/v1/remote/transport"
)
Expand Down Expand Up @@ -125,9 +127,37 @@ var DefaultTransport http.RoundTripper = &http.Transport{
MaxIdleConnsPerHost: 50,
}

func makeOptions(opts ...Option) (*options, error) {
// create transport once to allow connection sharing.
var defaultTransportInsecure = func() http.RoundTripper {
tr := DefaultTransport.(*http.Transport).Clone()
tr.TLSClientConfig = &tls.Config{InsecureSkipVerify: true}
return tr
}()

func targetIsInsecure(target resource) bool {
switch target := target.(type) {
case name.Registry:
return target.IsInsecure()

case name.Repository:
return target.Registry.IsInsecure()

case name.Digest:
return target.Registry.IsInsecure()
}
return false
}

func makeOptions(target resource, opts ...Option) (*options, error) {
var tr http.RoundTripper
if target != nil && targetIsInsecure(target) {
tr = defaultTransportInsecure
} else {
tr = DefaultTransport
}

o := &options{
transport: DefaultTransport,
transport: tr,
platform: defaultPlatform,
context: context.Background(),
jobs: defaultJobs,
Expand Down
95 changes: 95 additions & 0 deletions pkg/v1/remote/options_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
package remote

import (
"crypto/tls"
"fmt"
"net/http"
"net/http/httptest"
"net/url"
"testing"

"github.com/google/go-containerregistry/pkg/name"
)

func TestOptionsInsecure(t *testing.T) {
for _, targetType := range []string{
"registry",
"repository",
"digest",
} {
for _, mode := range []string{
"secure",
"insecure",
} {
t.Run(targetType+"_"+mode, func(t *testing.T) {
server := httptest.NewTLSServer(
http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
}))
defer server.Close()

u, err := url.Parse(server.URL)
if err != nil {
t.Fatal(err)
}

options := []name.Option{}

if mode == "insecure" {
options = append(options, name.Insecure)
}

var target resource

switch targetType {
case "registry":
reg, err := name.NewRegistry("myregistry", options...)
if err != nil {
t.Fatal(err)
}
target = reg

case "repository":
ref, err := name.ParseReference("myregistry/name:tag", options...)
if err != nil {
t.Fatal(err)
}
target = ref.Context()

case "digest":
d, err := name.NewDigest("myregistry/name@sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", options...)
if err != nil {
t.Fatal(err)
}
target = d
}

opts, err := makeOptions(target, []Option{}...)
if err != nil {
t.Fatal(err)
}

c := &http.Client{Transport: opts.transport}

res, err := c.Get(u.String())

if mode == "secure" {
if ue, ok := err.(*url.Error); !ok {
t.Fatal(err)
} else if _, ok := ue.Err.(*tls.CertificateVerificationError); !ok {
t.Fatal(err)
}
} else {
if err != nil {
t.Fatal(err)
}
defer res.Body.Close()

if res.StatusCode != http.StatusOK {
t.Fatal(fmt.Printf("unexpected status code: %d", res.StatusCode))
}
}
})
}
}
}
2 changes: 1 addition & 1 deletion pkg/v1/remote/puller.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ type Puller struct {
}

func NewPuller(options ...Option) (*Puller, error) {
o, err := makeOptions(options...)
o, err := makeOptions(nil, options...)
if err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/v1/remote/pusher.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ type Pusher struct {
}

func NewPusher(options ...Option) (*Pusher, error) {
o, err := makeOptions(options...)
o, err := makeOptions(nil, options...)
if err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/v1/remote/referrers.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ import (
//
// The subject manifest doesn't have to exist in the registry for there to be descriptors that refer to it.
func Referrers(d name.Digest, options ...Option) (v1.ImageIndex, error) {
o, err := makeOptions(options...)
o, err := makeOptions(d, options...)
if err != nil {
return nil, err
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/v1/remote/write.go
Original file line number Diff line number Diff line change
Expand Up @@ -654,7 +654,7 @@ func WriteIndex(ref name.Reference, ii v1.ImageIndex, options ...Option) (rerr e

// WriteLayer uploads the provided Layer to the specified repo.
func WriteLayer(repo name.Repository, layer v1.Layer, options ...Option) (rerr error) {
o, err := makeOptions(options...)
o, err := makeOptions(repo, options...)
if err != nil {
return err
}
Expand Down Expand Up @@ -691,7 +691,7 @@ func Tag(tag name.Tag, t Taggable, options ...Option) error {
// should ensure that all blobs or manifests that are referenced by t exist
// in the target registry.
func Put(ref name.Reference, t Taggable, options ...Option) error {
o, err := makeOptions(options...)
o, err := makeOptions(ref.Context(), options...)
if err != nil {
return err
}
Expand All @@ -700,7 +700,7 @@ func Put(ref name.Reference, t Taggable, options ...Option) error {

// Push uploads the given Taggable to the specified reference.
func Push(ref name.Reference, t Taggable, options ...Option) (rerr error) {
o, err := makeOptions(options...)
o, err := makeOptions(ref.Context(), options...)
if err != nil {
return err
}
Expand Down

0 comments on commit 38ffa80

Please sign in to comment.