Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix signature verification for oci hosted bundles #6145

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion download/oci_download.go
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ func (d *OCIDownloader) download(ctx context.Context, m metrics.Metrics) (*downl
return nil, err
}
loader := bundle.NewTarballLoaderWithBaseURL(fileReader, d.localStorePath)
reader := bundle.NewCustomReader(loader).WithBaseDir(d.localStorePath).
reader := bundle.NewCustomReader(loader).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't this break verification for some existing OCI bundles?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As there was no test for it I assume that it didn't work before.

If you want to try that just add revert the line you mentioned and rerun the test.

The test uses a bundle that is signed directly via opa:

//go:generate go run github.com/open-policy-agent/opa build -b --signing-alg HS256 --signing-key secret testdata/signed_bundle_data --output testdata/signed.tar.gz

I still consider the #6147 the more correct thing to do, although I see that might lead to not backward compatible changes. As my assumption is that I could sign a bundle in one folder for example: /home/userx/bundles/myBundle then copy that to another system into /data/bundles/myBundle and verify the signature without having to move to bundle to a specific folder. Besides that point the other usages of the bundle reader do not seem to us the WithBaseDir option.

opa/download/download.go

Lines 331 to 338 in de896d9

reader := bundle.NewCustomReader(loader).
WithMetrics(m).
WithBundleVerificationConfig(d.bvc).
WithBundleEtag(etag).
WithLazyLoadingMode(d.lazyLoadingMode).
WithBundleName(d.bundleName).
WithBundlePersistence(d.persist)
if d.sizeLimitBytes != nil {

or

r := bundle.NewCustomReader(bundle.NewTarballLoaderWithBaseURL(f, ""))
if bvc != nil {
r = r.WithBundleVerificationConfig(bvc)
}
b, err := r.Read()
if err != nil {
return nil, err
}

WithMetrics(m).
WithBundleVerificationConfig(d.bvc).
WithBundleEtag(etag)
Expand Down
43 changes: 43 additions & 0 deletions download/oci_download_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"context"
"encoding/base64"
"fmt"
"github.com/open-policy-agent/opa/bundle"
"net/http"
"strings"
"testing"
Expand All @@ -16,6 +17,48 @@ import (
"github.com/open-policy-agent/opa/plugins/rest"
)

// when changed the layer hash & size should be updated in signed.manifest
//go:generate go run github.com/open-policy-agent/opa build -b --signing-alg HS256 --signing-key secret testdata/signed_bundle_data --output testdata/signed.tar.gz

func TestOCIDownloaderWithBundleVerificationConfig(t *testing.T) {
vc := bundle.NewVerificationConfig(map[string]*bundle.KeyConfig{"default": {Key: "secret", Algorithm: "HS256"}}, "", "", nil)
ctx := context.Background()
fixture := newTestFixture(t)
fixture.server.expEtag = "sha256:c5834dbce332cabe6ae68a364de171a50bf5b08024c27d7c08cc72878b4df7ff"

updates := make(chan *Update)

config := Config{}
if err := config.ValidateAndInjectDefaults(); err != nil {
t.Fatal(err)
}

d := NewOCI(config, fixture.client, "ghcr.io/org/repo:signed", "/tmp/opa/").WithCallback(func(_ context.Context, u Update) {
if u.Error != nil {
t.Fatalf("expected no error but got: %v", u.Error)
}
updates <- &u
}).WithBundleVerificationConfig(vc)

d.Start(ctx)

// Give time for some download events to occur
time.Sleep(1 * time.Second)

u1 := <-updates

if u1.Bundle == nil || len(u1.Bundle.Modules) == 0 {
t.Fatal("expected bundle with at least one module but got:", u1)
}

if !strings.HasSuffix(u1.Bundle.Modules[0].URL, u1.Bundle.Modules[0].Path) {
t.Fatalf("expected URL to have path as suffix but got %v and %v", u1.Bundle.Modules[0].URL, u1.Bundle.Modules[0].Path)
}

d.Stop(ctx)

}

func TestOCIStartStop(t *testing.T) {
ctx := context.Background()
fixture := newTestFixture(t)
Expand Down
File renamed without changes.
File renamed without changes.
19 changes: 19 additions & 0 deletions download/testdata/signed.manifest
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{
"schemaVersion":2,
"config":{
"mediaType":"application/vnd.oci.image.config.v1+json",
"digest":"sha256:44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a",
"size":2
},
"layers":[
{
"mediaType":"application/vnd.oci.image.layer.v1.tar+gzip",
"digest":"sha256:e060c7b9558fad3ec85df5ffa19d0d019f839c36d7ec146977c871dcbc70885e",
"size":629,
"annotations":{
"org.opencontainers.image.created":"2022-02-11T09:00:07Z",
"org.opencontainers.image.title":"dani/testpol"
}
}
]
}
Binary file added download/testdata/signed.tar.gz
Binary file not shown.
1 change: 1 addition & 0 deletions download/testdata/signed_bundle_data/a/b/c/data.json
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
[1,2,3]
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
package example
114 changes: 63 additions & 51 deletions download/testharness.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@ package download
import (
"bytes"
"context"
"crypto/sha256"
"encoding/json"
"errors"
"fmt"
"io"
"net/http"
"net/http/httptest"
"os"
Expand Down Expand Up @@ -246,6 +248,11 @@ func (t *testFixture) oneShot(ctx context.Context, u Update) {
t.etags["test/bundle1"] = u.ETag
}

type fileInfo struct {
name string
length int64
}

type testServer struct {
t *testing.T
customAuth func(http.ResponseWriter, *http.Request) error
Expand All @@ -256,6 +263,7 @@ type testServer struct {
server *httptest.Server
etagInResponse bool
longPoll bool
testdataHashes map[string]fileInfo
}

func newTestServer(t *testing.T) *testServer {
Expand Down Expand Up @@ -339,68 +347,58 @@ func (t *testServer) handle(w http.ResponseWriter, r *http.Request) {

var buf bytes.Buffer

if r.URL.Path == "/v2/org/repo/manifests/latest" {
w.Header().Add("Content-Length", "596")
w.Header().Add("Content-Type", "application/vnd.oci.image.manifest.v1+json")
w.Header().Add("Docker-Content-Digest", "sha256:fe9c2930b6d8cc1bf3fa0c560996a95c75f0d0668bee71138355d9784c8c99b8")
w.WriteHeader(200)
return
}
if r.URL.Path == "/v2/org/repo/manifests/sha256:fe9c2930b6d8cc1bf3fa0c560996a95c75f0d0668bee71138355d9784c8c99b8" {
w.Header().Add("Content-Length", "596")
w.Header().Add("Content-Type", "application/vnd.oci.image.manifest.v1+json")
w.Header().Add("Docker-Content-Digest", "sha256:fe9c2930b6d8cc1bf3fa0c560996a95c75f0d0668bee71138355d9784c8c99b8")
w.WriteHeader(200)
bs, err := os.ReadFile("testdata/manifest.layer")
if err != nil {
w.WriteHeader(404)
return
if strings.HasPrefix(r.URL.Path, "/v2/org/repo/") {
// build test data to hash map to serve testdata files by hash
if t.testdataHashes == nil {
t.testdataHashes = make(map[string]fileInfo)
files, err := os.ReadDir("testdata")
if err != nil {
t.t.Fatalf("failed to read testdata directory: %s", err)
}
for _, file := range files {
if file.IsDir() {
continue
}
hash, length, err := getFileSHAandSize("testdata/" + file.Name())
if err != nil {
t.t.Fatalf("failed to read testdata file: %s", err)
}
t.testdataHashes[fmt.Sprintf("%x", hash)] = fileInfo{name: file.Name(), length: length}
}
}
buf.WriteString(string(bs))
w.Write(buf.Bytes())
return
}
if r.URL.Path == "/v2/org/repo/blobs/sha256:c5834dbce332cabe6ae68a364de171a50bf5b08024c27d7c08cc72878b4df7ff" {
w.Header().Add("Content-Length", "464")
w.Header().Add("Content-Type", "application/vnd.oci.image.layer.v1.tar+gzip,application/vnd.oci.image.config.v1+json")
w.Header().Add("Docker-Content-Digest", "sha256:c5834dbce332cabe6ae68a364de171a50bf5b08024c27d7c08cc72878b4df7ff")
w.WriteHeader(200)
bs, err := os.ReadFile("testdata/manifest.layer")
if err != nil {
w.WriteHeader(404)
return
}
buf.WriteString(string(bs))
buf.WriteTo(w)

return
}
if r.URL.Path == "/v2/org/repo/blobs/sha256:b206ac766b0f3f880f6a62c4bb5ba5192d29deaefd989a1961603346a7555bdd" {
w.Header().Add("Content-Length", "568")
w.Header().Add("Content-Type", "application/vnd.oci.image.layer.v1.tar+gzip")
w.Header().Add("Docker-Content-Digest", "sha256:b206ac766b0f3f880f6a62c4bb5ba5192d29deaefd989a1961603346a7555bdd")
w.WriteHeader(200)
bs, err := os.ReadFile("testdata/tar.layer")
if err != nil {
w.WriteHeader(404)
if strings.HasPrefix(r.URL.Path, "/v2/org/repo/blobs/sha256:") || strings.HasPrefix(r.URL.Path, "/v2/org/repo/manifests/sha256:") {
sha := strings.TrimPrefix(strings.TrimPrefix(r.URL.Path, "/v2/org/repo/blobs/sha256:"), "/v2/org/repo/manifests/sha256:")
if fileInfo, ok := t.testdataHashes[sha]; ok {
w.Header().Add("Content-Length", strconv.Itoa(int(fileInfo.length)))
w.Header().Add("Content-Type", "application/gzip")
w.Header().Add("Docker-Content-Digest", "sha256:"+sha)
w.WriteHeader(200)
bs, err := os.ReadFile("testdata/" + fileInfo.name)
if err != nil {
w.WriteHeader(404)
return
}
buf.WriteString(string(bs))
w.Write(buf.Bytes())
return
}
buf.WriteString(string(bs))
w.Write(buf.Bytes())
w.WriteHeader(404)
return
}
if r.URL.Path == "/v2/org/repo/blobs/sha256:44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a" {
w.Header().Add("Content-Length", "2")
w.Header().Add("Content-Type", "application/vnd.oci.image.config.v1+json")
w.Header().Add("Docker-Content-Digest", "sha256:44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a")
w.WriteHeader(200)
bs, err := os.ReadFile("testdata/config.layer")

if strings.HasPrefix(r.URL.Path, "/v2/org/repo/manifests/") {
sha, size, err := getFileSHAandSize("testdata/" + strings.TrimPrefix(r.URL.Path, "/v2/org/repo/manifests/") + ".manifest")
if err != nil {
w.WriteHeader(404)
return
}
buf.WriteString(string(bs))
w.Write(buf.Bytes())

w.Header().Add("Content-Length", strconv.Itoa(int(size)))
w.Header().Add("Content-Type", "application/vnd.oci.image.manifest.v1+json")
w.Header().Add("Docker-Content-Digest", "sha256:"+fmt.Sprintf("%x", sha))
w.WriteHeader(200)
return
}
name := strings.TrimPrefix(r.URL.Path, "/bundles/")
Expand Down Expand Up @@ -487,3 +485,17 @@ func getPreferHeaderField(r *http.Request, field string) string {
}
return ""
}

func getFileSHAandSize(filePath string) ([]byte, int64, error) {
f, err := os.Open(filePath)
if err != nil {
return nil, 0, err
}
defer f.Close()
hash := sha256.New()
w, err := io.Copy(hash, f)
if err != nil {
return nil, w, err
}
return hash.Sum(nil), w, nil
}