From b0969c5c6abb71918fd8ec34c1f51f73e3f14d32 Mon Sep 17 00:00:00 2001 From: Miguel Martinez Trivino Date: Sat, 24 Oct 2026 15:14:27 +0200 Subject: [PATCH 1/4] feat(cas): scope CAS downloader tokens to a single digest Add an optional digest claim to CAS downloader tokens. The CAS enforces it in the HTTP download handler, the bytestream read path and ResourceService.Describe, which now also requires the downloader role. Tokens without the claim keep the current behavior. CASCredentialsService.Get and CASRedirectService.GetDownloadURL accept scope_to_digest to mint a scoped token. The /download redirect, the CLI artifact download and the control plane CAS client now ask for one. Refs: SEC-634 Assisted-by: Claude Code Signed-off-by: Miguel Martinez Trivino Chainloop-Trace-Sessions: 34d67f3d-296c-4dce-b668-3a73637e6a43 --- .../internal/service/bytestream.go | 5 ++ .../internal/service/bytestream_test.go | 46 +++++++++++ app/artifact-cas/internal/service/download.go | 6 ++ .../internal/service/download_test.go | 58 ++++++++++++++ app/artifact-cas/internal/service/resource.go | 10 +++ .../internal/service/resource_test.go | 52 ++++++++++++- app/cli/cmd/artifact.go | 2 + .../api/controlplane/v1/cas_credentials.pb.go | 19 ++++- .../api/controlplane/v1/cas_credentials.proto | 3 + .../api/controlplane/v1/cas_redirect.pb.go | 20 +++-- .../api/controlplane/v1/cas_redirect.proto | 4 +- .../controlplane/v1/cas_redirect_grpc.pb.go | 2 +- .../controlplane/v1/cas_credentials.ts | 20 ++++- .../frontend/controlplane/v1/cas_redirect.ts | 21 ++++- ...edentialsServiceGetRequest.jsonschema.json | 8 ++ ...ASCredentialsServiceGetRequest.schema.json | 8 ++ ...e.v1.GetDownloadURLRequest.jsonschema.json | 10 +++ ...plane.v1.GetDownloadURLRequest.schema.json | 10 +++ .../internal/service/cascredential.go | 9 +++ .../internal/service/casredirect.go | 7 ++ .../service/casredirect_integration_test.go | 64 +++++++++++++++ app/controlplane/pkg/biz/casclient.go | 2 +- app/controlplane/pkg/biz/cascredentials.go | 14 ++++ internal/robotaccount/cas/robotaccount.go | 56 ++++++++++++++ .../robotaccount/cas/robotaccount_test.go | 77 +++++++++++++++++++ 25 files changed, 516 insertions(+), 17 deletions(-) diff --git a/app/artifact-cas/internal/service/bytestream.go b/app/artifact-cas/internal/service/bytestream.go index 75364903f..8b72d1074 100644 --- a/app/artifact-cas/internal/service/bytestream.go +++ b/app/artifact-cas/internal/service/bytestream.go @@ -332,6 +332,11 @@ func (s *ByteStreamService) Read(req *bytestream.ReadRequest, stream bytestream. return kerrors.BadRequest("resource name", "empty resource name") } + // A token scoped to a digest can only read that digest + if err := info.CheckDigest(req.ResourceName); err != nil { + return kerrors.Forbidden("service", err.Error()) + } + backend, err := s.loadBackend(ctx, info.BackendType, info.StoredSecretID) if err != nil && kerrors.IsNotFound(err) { return err diff --git a/app/artifact-cas/internal/service/bytestream_test.go b/app/artifact-cas/internal/service/bytestream_test.go index 2c10d6dc0..39c93de83 100644 --- a/app/artifact-cas/internal/service/bytestream_test.go +++ b/app/artifact-cas/internal/service/bytestream_test.go @@ -389,6 +389,49 @@ func (s *bytestreamSuite) TestDownloadOk() { s.False(info.Skipped) } +func (s *bytestreamSuite) TestReadAuthorizedDigest() { + const ( + digest = "b94d27b9934d3e08a52e52d7da7dabfac484efe37a5380ee9088f7ace2efcde9" + otherDigest = "7d865e959b2466918c9863afca942d0fb89d7c9ac0c99bafc3749504ded97730" + ) + + testCases := []struct { + name string + authorizedDigest string + wantErr bool + }{ + {name: "token scoped to the digest", authorizedDigest: "sha256:" + digest}, + {name: "token scoped to another digest", authorizedDigest: "sha256:" + otherDigest, wantErr: true}, + } + + for _, tc := range testCases { + s.Run(tc.name, func() { + // the mock fails the test on a download of a forbidden digest + if !tc.wantErr { + s.ociBackend.On("Download", mock.Anything, mock.Anything, digest). + Return(nil).Run(func(args mock.Arguments) { + _, err := io.WriteString(args.Get(1).(io.Writer), "hello world") + s.NoError(err) + }) + } + + ctx := metadata.AppendToOutgoingContext(s.downCtx, "authorized-digest", tc.authorizedDigest) + reader, err := s.client.Read(ctx, &bytestream.ReadRequest{ResourceName: digest}) + s.Require().NoError(err) + + got, err := reader.Recv() + if tc.wantErr { + assertGRPCError(s.T(), err, codes.PermissionDenied, "not authorized for this digest") + s.Empty(s.audit.published) + return + } + + s.Require().NoError(err) + s.Equal("hello world", string(got.Data)) + }) + } +} + func (s *bytestreamSuite) TestDownloadFoundMistmathedDigest() { s.ociBackend.On("Download", mock.Anything, mock.Anything, "deadbeef"). Return(nil).Run(func(args mock.Arguments) { @@ -487,6 +530,9 @@ func (s *bytestreamSuite) setup(opts ...NewOpt) { if v := md.Get("org-id"); len(v) > 0 { claims.OrgID = v[0] } + if v := md.Get("authorized-digest"); len(v) > 0 { + claims.Digest = v[0] + } if v := md.Get("source-internal"); len(v) > 0 { claims.SourceInternal = true } diff --git a/app/artifact-cas/internal/service/download.go b/app/artifact-cas/internal/service/download.go index 9b3e67f69..0d1a3726c 100644 --- a/app/artifact-cas/internal/service/download.go +++ b/app/artifact-cas/internal/service/download.go @@ -78,6 +78,12 @@ func (s *DownloadService) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } + // A token scoped to a digest can only read that digest + if err := auth.CheckDigest(wantChecksum.String()); err != nil { + http.Error(w, err.Error(), http.StatusForbidden) + return + } + b, err := s.loadBackend(ctx, auth.BackendType, auth.StoredSecretID) if err != nil && kerrors.IsNotFound(err) { http.Error(w, "backend not found", http.StatusNotFound) diff --git a/app/artifact-cas/internal/service/download_test.go b/app/artifact-cas/internal/service/download_test.go index 5fa8def1f..8f74a7055 100644 --- a/app/artifact-cas/internal/service/download_test.go +++ b/app/artifact-cas/internal/service/download_test.go @@ -373,3 +373,61 @@ func TestDownloadServiceConditionalRequests(t *testing.T) { }) } } + +func TestDownloadServiceAuthorizedDigest(t *testing.T) { + const otherDigest = "sha256:7d865e959b2466918c9863afca942d0fb89d7c9ac0c99bafc3749504ded97730" + + tests := []struct { + name string + digest string + wantStatus int + }{ + {name: "token without digest claim reads the artifact", wantStatus: http.StatusOK}, + {name: "token scoped to the digest reads the artifact", digest: "sha256:" + downloadDigestHex, wantStatus: http.StatusOK}, + {name: "token scoped to another digest is forbidden", digest: otherDigest, wantStatus: http.StatusForbidden}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + provider := mocks.NewProvider(t) + // the mocks fail the test on any backend call made for a forbidden digest + if tc.wantStatus == http.StatusOK { + uploaderDownloader := mocks.NewUploaderDownloader(t) + provider.On("FromCredentials", mock.Anything, mock.Anything).Return(uploaderDownloader, nil) + uploaderDownloader.On("Describe", mock.Anything, downloadDigestHex). + Return(&v1.CASResource{FileName: downloadFileName, Digest: downloadDigestHex, Size: int64(len(downloadContent))}, nil) + uploaderDownloader.On("Download", mock.Anything, mock.Anything, downloadDigestHex).Return(nil). + Run(func(args mock.Arguments) { + _, err := io.WriteString(args.Get(1).(io.Writer), downloadContent) + require.NoError(t, err) + }) + } + + svc := NewDownloadService( + backend.Providers{downloadBackendType: provider}, + WithLogger(log.DefaultLogger), + WithStagingDir(t.TempDir()), + ) + + claims := &casJWT.Claims{ + Role: casJWT.Downloader, + StoredSecretID: testStoredSecretID, + BackendType: downloadBackendType, + OrgID: testOrgID, + Digest: tc.digest, + } + + req := httptest.NewRequest(http.MethodGet, "/download/sha256:"+downloadDigestHex, nil) + req = mux.SetURLVars(req, map[string]string{"digest": "sha256:" + downloadDigestHex}) + req = req.WithContext(jwtMiddleware.NewContext(req.Context(), claims)) + + w := httptest.NewRecorder() + svc.ServeHTTP(w, req) + + assert.Equal(t, tc.wantStatus, w.Code) + if tc.wantStatus == http.StatusOK { + assert.Equal(t, downloadContent, w.Body.String()) + } + }) + } +} diff --git a/app/artifact-cas/internal/service/resource.go b/app/artifact-cas/internal/service/resource.go index e8defd0dd..e063f51ae 100644 --- a/app/artifact-cas/internal/service/resource.go +++ b/app/artifact-cas/internal/service/resource.go @@ -49,6 +49,16 @@ func (s *ResourceService) Describe(ctx context.Context, req *v1.ResourceServiceD return nil, err } + // Only downloader tokens are allowed + if err := info.CheckRole(casJWT.Downloader); err != nil { + return nil, errors.Unauthorized("service", err.Error()) + } + + // A token scoped to a digest can only describe that digest + if err := info.CheckDigest(req.Digest); err != nil { + return nil, errors.Forbidden("service", err.Error()) + } + b, err := s.loadBackend(ctx, info.BackendType, info.StoredSecretID) if err != nil && errors.IsNotFound(err) { return nil, err diff --git a/app/artifact-cas/internal/service/resource_test.go b/app/artifact-cas/internal/service/resource_test.go index 8c4addaa0..faaae7185 100644 --- a/app/artifact-cas/internal/service/resource_test.go +++ b/app/artifact-cas/internal/service/resource_test.go @@ -1,5 +1,5 @@ // -// Copyright 2023 The Chainloop Authors. +// Copyright 2023-2026 The Chainloop Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -24,6 +24,7 @@ import ( casjwt "github.com/chainloop-dev/chainloop/internal/robotaccount/cas" backend "github.com/chainloop-dev/chainloop/pkg/blobmanager" "github.com/chainloop-dev/chainloop/pkg/blobmanager/mocks" + "github.com/go-kratos/kratos/v2/errors" jwtmiddleware "github.com/go-kratos/kratos/v2/middleware/auth/jwt" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/suite" @@ -44,6 +45,48 @@ func (s *resourceSuite) TestDescribe() { s.Equal(&v1.ResourceServiceDescribeResponse{Result: want}, got) } +func (s *resourceSuite) TestDescribeAuthorization() { + const ( + digest = "b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c" + otherDigest = "7d865e959b2466918c9863afca942d0fb89d7c9ac0c99bafc3749504ded97730" + ) + + testCases := []struct { + name string + role casjwt.Role + digest string + wantErr bool + }{ + {name: "downloader without digest claim", role: casjwt.Downloader}, + {name: "downloader scoped to the digest", role: casjwt.Downloader, digest: "sha256:" + digest}, + {name: "downloader scoped to another digest", role: casjwt.Downloader, digest: "sha256:" + otherDigest, wantErr: true}, + {name: "uploader", role: casjwt.Uploader, wantErr: true}, + } + + for _, tc := range testCases { + s.Run(tc.name, func() { + want := &v1.CASResource{FileName: "test.txt", Digest: digest} + // the mock fails the test on a describe of a forbidden digest + if !tc.wantErr { + s.ociBackend.On("Describe", mock.Anything, digest).Return(want, nil) + } + + claims := &casjwt.Claims{StoredSecretID: "secret-id", BackendType: "backend1", Role: tc.role, Digest: tc.digest} + ctx := jwtmiddleware.NewContext(context.Background(), claims) + svc := service.NewResourceService(s.backendProviders) + got, err := svc.Describe(ctx, &v1.ResourceServiceDescribeRequest{Digest: digest}) + if tc.wantErr { + s.True(errors.IsForbidden(err) || errors.IsUnauthorized(err), "unexpected error: %v", err) + s.Nil(got) + return + } + + s.NoError(err) + s.Equal(want, got.GetResult()) + }) + } +} + type resourceSuite struct { suite.Suite ociBackend *mocks.UploaderDownloader @@ -54,7 +97,7 @@ func (s *resourceSuite) SetupTest() { ociBackendProvider := mocks.NewProvider(s.T()) ociBackend := mocks.NewUploaderDownloader(s.T()) ociBackendProvider.On("FromCredentials", mock.Anything, "secret-id"). - Return(ociBackend, nil) + Maybe().Return(ociBackend, nil) s.ociBackend = ociBackend s.backendProviders = backend.Providers{ @@ -62,6 +105,11 @@ func (s *resourceSuite) SetupTest() { } } +// SetupSubTest gives every s.Run subtest its own backend mocks +func (s *resourceSuite) SetupSubTest() { + s.SetupTest() +} + // Run the tests func TestResourceSuite(t *testing.T) { suite.Run(t, new(resourceSuite)) diff --git a/app/cli/cmd/artifact.go b/app/cli/cmd/artifact.go index 2ce0b2a6c..d55bcdcec 100644 --- a/app/cli/cmd/artifact.go +++ b/app/cli/cmd/artifact.go @@ -42,6 +42,8 @@ func wrappedArtifactConn(cpConn *grpc.ClientConn, role pb.CASCredentialsServiceG resp, err := client.Get(context.Background(), &pb.CASCredentialsServiceGetRequest{ Role: role, Digest: digest, + // a download is for a single digest, so the token can only read that digest + ScopeToDigest: role == pb.CASCredentialsServiceGetRequest_ROLE_DOWNLOADER && digest != "", }) if err != nil { return nil, err diff --git a/app/controlplane/api/controlplane/v1/cas_credentials.pb.go b/app/controlplane/api/controlplane/v1/cas_credentials.pb.go index c271ac4a5..afab483b8 100644 --- a/app/controlplane/api/controlplane/v1/cas_credentials.pb.go +++ b/app/controlplane/api/controlplane/v1/cas_credentials.pb.go @@ -94,8 +94,11 @@ type CASCredentialsServiceGetRequest struct { // flag the minted token as internal platform traffic so the CAS skips audit // event emission for it. Only honored for system API tokens, rejected otherwise. SourceInternal bool `protobuf:"varint,3,opt,name=source_internal,json=sourceInternal,proto3" json:"source_internal,omitempty"` - unknownFields protoimpl.UnknownFields - sizeCache protoimpl.SizeCache + // scope the downloader token to the requested digest, so the CAS rejects reads of any other artifact. + // Requires the downloader role and a digest. + ScopeToDigest bool `protobuf:"varint,4,opt,name=scope_to_digest,json=scopeToDigest,proto3" json:"scope_to_digest,omitempty"` + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache } func (x *CASCredentialsServiceGetRequest) Reset() { @@ -149,6 +152,13 @@ func (x *CASCredentialsServiceGetRequest) GetSourceInternal() bool { return false } +func (x *CASCredentialsServiceGetRequest) GetScopeToDigest() bool { + if x != nil { + return x.ScopeToDigest + } + return false +} + type CASCredentialsServiceGetResponse struct { state protoimpl.MessageState `protogen:"open.v1"` Result *CASCredentialsServiceGetResponse_Result `protobuf:"bytes,1,opt,name=result,proto3" json:"result,omitempty"` @@ -249,12 +259,13 @@ var File_controlplane_v1_cas_credentials_proto protoreflect.FileDescriptor const file_controlplane_v1_cas_credentials_proto_rawDesc = "" + "\n" + - "%controlplane/v1/cas_credentials.proto\x12\x0fcontrolplane.v1\x1a\x1bbuf/validate/validate.proto\x1a'controlplane/v1/response_messages.proto\"\xff\x01\n" + + "%controlplane/v1/cas_credentials.proto\x12\x0fcontrolplane.v1\x1a\x1bbuf/validate/validate.proto\x1a'controlplane/v1/response_messages.proto\"\xa7\x02\n" + "\x1fCASCredentialsServiceGetRequest\x12U\n" + "\x04role\x18\x01 \x01(\x0e25.controlplane.v1.CASCredentialsServiceGetRequest.RoleB\n" + "\xbaH\a\x82\x01\x04\x18\x01\x18\x02R\x04role\x12\x16\n" + "\x06digest\x18\x02 \x01(\tR\x06digest\x12'\n" + - "\x0fsource_internal\x18\x03 \x01(\bR\x0esourceInternal\"D\n" + + "\x0fsource_internal\x18\x03 \x01(\bR\x0esourceInternal\x12&\n" + + "\x0fscope_to_digest\x18\x04 \x01(\bR\rscopeToDigest\"D\n" + "\x04Role\x12\x14\n" + "\x10ROLE_UNSPECIFIED\x10\x00\x12\x13\n" + "\x0fROLE_DOWNLOADER\x10\x01\x12\x11\n" + diff --git a/app/controlplane/api/controlplane/v1/cas_credentials.proto b/app/controlplane/api/controlplane/v1/cas_credentials.proto index 04cbaf443..0585d93a4 100644 --- a/app/controlplane/api/controlplane/v1/cas_credentials.proto +++ b/app/controlplane/api/controlplane/v1/cas_credentials.proto @@ -38,6 +38,9 @@ message CASCredentialsServiceGetRequest { // flag the minted token as internal platform traffic so the CAS skips audit // event emission for it. Only honored for system API tokens, rejected otherwise. bool source_internal = 3; + // scope the downloader token to the requested digest, so the CAS rejects reads of any other artifact. + // Requires the downloader role and a digest. + bool scope_to_digest = 4; enum Role { ROLE_UNSPECIFIED = 0; diff --git a/app/controlplane/api/controlplane/v1/cas_redirect.pb.go b/app/controlplane/api/controlplane/v1/cas_redirect.pb.go index b49f5d605..b911aab5e 100644 --- a/app/controlplane/api/controlplane/v1/cas_redirect.pb.go +++ b/app/controlplane/api/controlplane/v1/cas_redirect.pb.go @@ -1,5 +1,5 @@ // -// Copyright 2023 The Chainloop Authors. +// Copyright 2023-2026 The Chainloop Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -38,8 +38,10 @@ const ( ) type GetDownloadURLRequest struct { - state protoimpl.MessageState `protogen:"open.v1"` - Digest string `protobuf:"bytes,1,opt,name=digest,proto3" json:"digest,omitempty"` + state protoimpl.MessageState `protogen:"open.v1"` + Digest string `protobuf:"bytes,1,opt,name=digest,proto3" json:"digest,omitempty"` + // scope the token in the URL to the requested digest, so the CAS rejects reads of any other artifact + ScopeToDigest bool `protobuf:"varint,2,opt,name=scope_to_digest,json=scopeToDigest,proto3" json:"scope_to_digest,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -81,6 +83,13 @@ func (x *GetDownloadURLRequest) GetDigest() string { return "" } +func (x *GetDownloadURLRequest) GetScopeToDigest() bool { + if x != nil { + return x.ScopeToDigest + } + return false +} + type GetDownloadURLResponse struct { state protoimpl.MessageState `protogen:"open.v1"` Result *GetDownloadURLResponse_Result `protobuf:"bytes,1,opt,name=result,proto3" json:"result,omitempty"` @@ -173,9 +182,10 @@ var File_controlplane_v1_cas_redirect_proto protoreflect.FileDescriptor const file_controlplane_v1_cas_redirect_proto_rawDesc = "" + "\n" + - "\"controlplane/v1/cas_redirect.proto\x12\x0fcontrolplane.v1\x1a\x1bbuf/validate/validate.proto\"8\n" + + "\"controlplane/v1/cas_redirect.proto\x12\x0fcontrolplane.v1\x1a\x1bbuf/validate/validate.proto\"`\n" + "\x15GetDownloadURLRequest\x12\x1f\n" + - "\x06digest\x18\x01 \x01(\tB\a\xbaH\x04r\x02\x10\x01R\x06digest\"|\n" + + "\x06digest\x18\x01 \x01(\tB\a\xbaH\x04r\x02\x10\x01R\x06digest\x12&\n" + + "\x0fscope_to_digest\x18\x02 \x01(\bR\rscopeToDigest\"|\n" + "\x16GetDownloadURLResponse\x12F\n" + "\x06result\x18\x01 \x01(\v2..controlplane.v1.GetDownloadURLResponse.ResultR\x06result\x1a\x1a\n" + "\x06Result\x12\x10\n" + diff --git a/app/controlplane/api/controlplane/v1/cas_redirect.proto b/app/controlplane/api/controlplane/v1/cas_redirect.proto index de311d970..91669bd4d 100644 --- a/app/controlplane/api/controlplane/v1/cas_redirect.proto +++ b/app/controlplane/api/controlplane/v1/cas_redirect.proto @@ -1,5 +1,5 @@ // -// Copyright 2023 The Chainloop Authors. +// Copyright 2023-2026 The Chainloop Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -28,6 +28,8 @@ service CASRedirectService { message GetDownloadURLRequest { string digest = 1 [(buf.validate.field).string = {min_len: 1}]; + // scope the token in the URL to the requested digest, so the CAS rejects reads of any other artifact + bool scope_to_digest = 2; } message GetDownloadURLResponse { diff --git a/app/controlplane/api/controlplane/v1/cas_redirect_grpc.pb.go b/app/controlplane/api/controlplane/v1/cas_redirect_grpc.pb.go index e09874d0f..bf274dd2f 100644 --- a/app/controlplane/api/controlplane/v1/cas_redirect_grpc.pb.go +++ b/app/controlplane/api/controlplane/v1/cas_redirect_grpc.pb.go @@ -1,5 +1,5 @@ // -// Copyright 2023 The Chainloop Authors. +// Copyright 2023-2026 The Chainloop Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. diff --git a/app/controlplane/api/gen/frontend/controlplane/v1/cas_credentials.ts b/app/controlplane/api/gen/frontend/controlplane/v1/cas_credentials.ts index 9546d91d1..2bc96fe5f 100644 --- a/app/controlplane/api/gen/frontend/controlplane/v1/cas_credentials.ts +++ b/app/controlplane/api/gen/frontend/controlplane/v1/cas_credentials.ts @@ -15,6 +15,11 @@ export interface CASCredentialsServiceGetRequest { * event emission for it. Only honored for system API tokens, rejected otherwise. */ sourceInternal: boolean; + /** + * scope the downloader token to the requested digest, so the CAS rejects reads of any other artifact. + * Requires the downloader role and a digest. + */ + scopeToDigest: boolean; } export enum CASCredentialsServiceGetRequest_Role { @@ -66,7 +71,7 @@ export interface CASCredentialsServiceGetResponse_Result { } function createBaseCASCredentialsServiceGetRequest(): CASCredentialsServiceGetRequest { - return { role: 0, digest: "", sourceInternal: false }; + return { role: 0, digest: "", sourceInternal: false, scopeToDigest: false }; } export const CASCredentialsServiceGetRequest = { @@ -80,6 +85,9 @@ export const CASCredentialsServiceGetRequest = { if (message.sourceInternal === true) { writer.uint32(24).bool(message.sourceInternal); } + if (message.scopeToDigest === true) { + writer.uint32(32).bool(message.scopeToDigest); + } return writer; }, @@ -111,6 +119,13 @@ export const CASCredentialsServiceGetRequest = { message.sourceInternal = reader.bool(); continue; + case 4: + if (tag !== 32) { + break; + } + + message.scopeToDigest = reader.bool(); + continue; } if ((tag & 7) === 4 || tag === 0) { break; @@ -125,6 +140,7 @@ export const CASCredentialsServiceGetRequest = { role: isSet(object.role) ? cASCredentialsServiceGetRequest_RoleFromJSON(object.role) : 0, digest: isSet(object.digest) ? String(object.digest) : "", sourceInternal: isSet(object.sourceInternal) ? Boolean(object.sourceInternal) : false, + scopeToDigest: isSet(object.scopeToDigest) ? Boolean(object.scopeToDigest) : false, }; }, @@ -133,6 +149,7 @@ export const CASCredentialsServiceGetRequest = { message.role !== undefined && (obj.role = cASCredentialsServiceGetRequest_RoleToJSON(message.role)); message.digest !== undefined && (obj.digest = message.digest); message.sourceInternal !== undefined && (obj.sourceInternal = message.sourceInternal); + message.scopeToDigest !== undefined && (obj.scopeToDigest = message.scopeToDigest); return obj; }, @@ -147,6 +164,7 @@ export const CASCredentialsServiceGetRequest = { message.role = object.role ?? 0; message.digest = object.digest ?? ""; message.sourceInternal = object.sourceInternal ?? false; + message.scopeToDigest = object.scopeToDigest ?? false; return message; }, }; diff --git a/app/controlplane/api/gen/frontend/controlplane/v1/cas_redirect.ts b/app/controlplane/api/gen/frontend/controlplane/v1/cas_redirect.ts index 798414410..d262ca3e1 100644 --- a/app/controlplane/api/gen/frontend/controlplane/v1/cas_redirect.ts +++ b/app/controlplane/api/gen/frontend/controlplane/v1/cas_redirect.ts @@ -7,6 +7,8 @@ export const protobufPackage = "controlplane.v1"; export interface GetDownloadURLRequest { digest: string; + /** scope the token in the URL to the requested digest, so the CAS rejects reads of any other artifact */ + scopeToDigest: boolean; } export interface GetDownloadURLResponse { @@ -18,7 +20,7 @@ export interface GetDownloadURLResponse_Result { } function createBaseGetDownloadURLRequest(): GetDownloadURLRequest { - return { digest: "" }; + return { digest: "", scopeToDigest: false }; } export const GetDownloadURLRequest = { @@ -26,6 +28,9 @@ export const GetDownloadURLRequest = { if (message.digest !== "") { writer.uint32(10).string(message.digest); } + if (message.scopeToDigest === true) { + writer.uint32(16).bool(message.scopeToDigest); + } return writer; }, @@ -43,6 +48,13 @@ export const GetDownloadURLRequest = { message.digest = reader.string(); continue; + case 2: + if (tag !== 16) { + break; + } + + message.scopeToDigest = reader.bool(); + continue; } if ((tag & 7) === 4 || tag === 0) { break; @@ -53,12 +65,16 @@ export const GetDownloadURLRequest = { }, fromJSON(object: any): GetDownloadURLRequest { - return { digest: isSet(object.digest) ? String(object.digest) : "" }; + return { + digest: isSet(object.digest) ? String(object.digest) : "", + scopeToDigest: isSet(object.scopeToDigest) ? Boolean(object.scopeToDigest) : false, + }; }, toJSON(message: GetDownloadURLRequest): unknown { const obj: any = {}; message.digest !== undefined && (obj.digest = message.digest); + message.scopeToDigest !== undefined && (obj.scopeToDigest = message.scopeToDigest); return obj; }, @@ -69,6 +85,7 @@ export const GetDownloadURLRequest = { fromPartial, I>>(object: I): GetDownloadURLRequest { const message = createBaseGetDownloadURLRequest(); message.digest = object.digest ?? ""; + message.scopeToDigest = object.scopeToDigest ?? false; return message; }, }; diff --git a/app/controlplane/api/gen/jsonschema/controlplane.v1.CASCredentialsServiceGetRequest.jsonschema.json b/app/controlplane/api/gen/jsonschema/controlplane.v1.CASCredentialsServiceGetRequest.jsonschema.json index 63fd56cf0..9931aaa85 100644 --- a/app/controlplane/api/gen/jsonschema/controlplane.v1.CASCredentialsServiceGetRequest.jsonschema.json +++ b/app/controlplane/api/gen/jsonschema/controlplane.v1.CASCredentialsServiceGetRequest.jsonschema.json @@ -3,6 +3,10 @@ "$schema": "https://json-schema.org/draft/2020-12/schema", "additionalProperties": false, "patternProperties": { + "^(scope_to_digest)$": { + "description": "scope the downloader token to the requested digest, so the CAS rejects reads of any other artifact.\n Requires the downloader role and a digest.", + "type": "boolean" + }, "^(source_internal)$": { "description": "flag the minted token as internal platform traffic so the CAS skips audit\n event emission for it. Only honored for system API tokens, rejected otherwise.", "type": "boolean" @@ -31,6 +35,10 @@ } ] }, + "scopeToDigest": { + "description": "scope the downloader token to the requested digest, so the CAS rejects reads of any other artifact.\n Requires the downloader role and a digest.", + "type": "boolean" + }, "sourceInternal": { "description": "flag the minted token as internal platform traffic so the CAS skips audit\n event emission for it. Only honored for system API tokens, rejected otherwise.", "type": "boolean" diff --git a/app/controlplane/api/gen/jsonschema/controlplane.v1.CASCredentialsServiceGetRequest.schema.json b/app/controlplane/api/gen/jsonschema/controlplane.v1.CASCredentialsServiceGetRequest.schema.json index a85a0e163..d483dad5c 100644 --- a/app/controlplane/api/gen/jsonschema/controlplane.v1.CASCredentialsServiceGetRequest.schema.json +++ b/app/controlplane/api/gen/jsonschema/controlplane.v1.CASCredentialsServiceGetRequest.schema.json @@ -3,6 +3,10 @@ "$schema": "https://json-schema.org/draft/2020-12/schema", "additionalProperties": false, "patternProperties": { + "^(scopeToDigest)$": { + "description": "scope the downloader token to the requested digest, so the CAS rejects reads of any other artifact.\n Requires the downloader role and a digest.", + "type": "boolean" + }, "^(sourceInternal)$": { "description": "flag the minted token as internal platform traffic so the CAS skips audit\n event emission for it. Only honored for system API tokens, rejected otherwise.", "type": "boolean" @@ -31,6 +35,10 @@ } ] }, + "scope_to_digest": { + "description": "scope the downloader token to the requested digest, so the CAS rejects reads of any other artifact.\n Requires the downloader role and a digest.", + "type": "boolean" + }, "source_internal": { "description": "flag the minted token as internal platform traffic so the CAS skips audit\n event emission for it. Only honored for system API tokens, rejected otherwise.", "type": "boolean" diff --git a/app/controlplane/api/gen/jsonschema/controlplane.v1.GetDownloadURLRequest.jsonschema.json b/app/controlplane/api/gen/jsonschema/controlplane.v1.GetDownloadURLRequest.jsonschema.json index 17790a06f..af0bb7069 100644 --- a/app/controlplane/api/gen/jsonschema/controlplane.v1.GetDownloadURLRequest.jsonschema.json +++ b/app/controlplane/api/gen/jsonschema/controlplane.v1.GetDownloadURLRequest.jsonschema.json @@ -2,10 +2,20 @@ "$id": "controlplane.v1.GetDownloadURLRequest.jsonschema.json", "$schema": "https://json-schema.org/draft/2020-12/schema", "additionalProperties": false, + "patternProperties": { + "^(scope_to_digest)$": { + "description": "scope the token in the URL to the requested digest, so the CAS rejects reads of any other artifact", + "type": "boolean" + } + }, "properties": { "digest": { "minLength": 1, "type": "string" + }, + "scopeToDigest": { + "description": "scope the token in the URL to the requested digest, so the CAS rejects reads of any other artifact", + "type": "boolean" } }, "title": "Get DownloadURL Request", diff --git a/app/controlplane/api/gen/jsonschema/controlplane.v1.GetDownloadURLRequest.schema.json b/app/controlplane/api/gen/jsonschema/controlplane.v1.GetDownloadURLRequest.schema.json index 955504f50..9e43ce281 100644 --- a/app/controlplane/api/gen/jsonschema/controlplane.v1.GetDownloadURLRequest.schema.json +++ b/app/controlplane/api/gen/jsonschema/controlplane.v1.GetDownloadURLRequest.schema.json @@ -2,10 +2,20 @@ "$id": "controlplane.v1.GetDownloadURLRequest.schema.json", "$schema": "https://json-schema.org/draft/2020-12/schema", "additionalProperties": false, + "patternProperties": { + "^(scopeToDigest)$": { + "description": "scope the token in the URL to the requested digest, so the CAS rejects reads of any other artifact", + "type": "boolean" + } + }, "properties": { "digest": { "minLength": 1, "type": "string" + }, + "scope_to_digest": { + "description": "scope the token in the URL to the requested digest, so the CAS rejects reads of any other artifact", + "type": "boolean" } }, "title": "Get DownloadURL Request", diff --git a/app/controlplane/internal/service/cascredential.go b/app/controlplane/internal/service/cascredential.go index df2e1d49d..5b6889669 100644 --- a/app/controlplane/internal/service/cascredential.go +++ b/app/controlplane/internal/service/cascredential.go @@ -82,6 +82,11 @@ func (s *CASCredentialsService) Get(ctx context.Context, req *pb.CASCredentialsS policyToCheck = authz.PolicyArtifactUpload } + // Without a digest the scoped request would silently mint an unscoped token + if req.GetScopeToDigest() && req.GetDigest() == "" { + return nil, errors.BadRequest("invalid argument", "a digest is required to scope the credentials") + } + // Enforce required role if ok, err := s.authzUC.Enforce(ctx, currentAuthzSubject, policyToCheck); err != nil { return nil, handleUseCaseErr(ctx, err, s.log) @@ -149,6 +154,10 @@ func (s *CASCredentialsService) Get(ctx context.Context, req *pb.CASCredentialsS } ref := &biz.CASCredsOpts{BackendType: string(backend.Provider), SecretPath: backend.SecretName, Role: role, MaxBytes: backend.Limits.MaxBytes, OrgID: backend.OrganizationID, SourceInternal: sourceInternal} + if req.GetScopeToDigest() { + ref.Digest = req.GetDigest() + } + t, err := s.casUC.GenerateTemporaryCredentials(ref) if err != nil { return nil, handleUseCaseErr(ctx, err, s.log) diff --git a/app/controlplane/internal/service/casredirect.go b/app/controlplane/internal/service/casredirect.go index 45cc4ad0d..c49e08ecb 100644 --- a/app/controlplane/internal/service/casredirect.go +++ b/app/controlplane/internal/service/casredirect.go @@ -137,6 +137,10 @@ func (s *CASRedirectService) GetDownloadURL(ctx context.Context, req *pb.GetDown // 2- add authentication token to the query params ?t=[token] if backend.SecretName != "" { ref := &biz.CASCredsOpts{BackendType: string(backend.Provider), SecretPath: backend.SecretName, Role: casJWT.Downloader, MaxBytes: backend.Limits.MaxBytes, OrgID: backend.OrganizationID} + if req.GetScopeToDigest() { + ref.Digest = req.GetDigest() + } + t, err := s.casCredsUseCase.GenerateTemporaryCredentials(ref) if err != nil { return nil, handleUseCaseErr(ctx, err, s.log) @@ -169,6 +173,9 @@ func (s *CASRedirectService) HTTPDownload(ctx khttp.Context) error { return err } + // The redirect is for a single digest, so the token in the URL can only read that digest + in.ScopeToDigest = true + // Run the downloadURLCode within the middleware chain // Set identifier so it can be skipped, filtered, selected during the middleware chain khttp.SetOperation(ctx, getDownloadURLOperation) diff --git a/app/controlplane/internal/service/casredirect_integration_test.go b/app/controlplane/internal/service/casredirect_integration_test.go index 8ee8fe647..870057c05 100644 --- a/app/controlplane/internal/service/casredirect_integration_test.go +++ b/app/controlplane/internal/service/casredirect_integration_test.go @@ -22,6 +22,7 @@ import ( "crypto/rand" "crypto/x509" "encoding/pem" + "net/url" "os" "path/filepath" "testing" @@ -33,9 +34,11 @@ import ( "github.com/chainloop-dev/chainloop/app/controlplane/pkg/biz" "github.com/chainloop-dev/chainloop/app/controlplane/pkg/biz/testhelpers" "github.com/chainloop-dev/chainloop/app/controlplane/pkg/usercontext/entities" + casJWT "github.com/chainloop-dev/chainloop/internal/robotaccount/cas" "github.com/chainloop-dev/chainloop/pkg/blobmanager/oci" creds "github.com/chainloop-dev/chainloop/pkg/credentials/mocks" kerrors "github.com/go-kratos/kratos/v2/errors" + "github.com/golang-jwt/jwt/v5" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/suite" ) @@ -182,6 +185,67 @@ func (s *casDownloadRBACIntegrationSuite) TestCASCredentialsGet() { }) } +// SEC-634: callers can ask for a downloader token that can only read the requested digest +func (s *casDownloadRBACIntegrationSuite) TestDigestScopedTokens() { + ctx := s.ctxForToken(s.orgToken) + + testCases := []struct { + name string + scopeToDigest bool + wantDigest string + }{ + {name: "token is not scoped by default"}, + {name: "token scoped to the requested digest", scopeToDigest: true, wantDigest: digestProjectA}, + } + + for _, tc := range testCases { + s.Run("GetDownloadURL: "+tc.name, func() { + resp, err := s.redirectSvc.GetDownloadURL(ctx, &pb.GetDownloadURLRequest{Digest: digestProjectA, ScopeToDigest: tc.scopeToDigest}) + s.Require().NoError(err) + + u, err := url.Parse(resp.GetResult().GetUrl()) + s.Require().NoError(err) + s.Equal(tc.wantDigest, s.casClaims(u.Query().Get("t")).Digest) + }) + + s.Run("CASCredentialsService.Get: "+tc.name, func() { + resp, err := s.credentialsSvc.Get(ctx, &pb.CASCredentialsServiceGetRequest{ + Digest: digestProjectA, + Role: pb.CASCredentialsServiceGetRequest_ROLE_DOWNLOADER, + ScopeToDigest: tc.scopeToDigest, + }) + s.Require().NoError(err) + s.Equal(tc.wantDigest, s.casClaims(resp.GetResult().GetToken()).Digest) + }) + } + + s.Run("CASCredentialsService.Get: scoped token requires a digest", func() { + _, err := s.credentialsSvc.Get(ctx, &pb.CASCredentialsServiceGetRequest{ + Role: pb.CASCredentialsServiceGetRequest_ROLE_DOWNLOADER, + ScopeToDigest: true, + }) + s.True(kerrors.IsBadRequest(err), "expected bad request, got %v", err) + }) + + s.Run("CASCredentialsService.Get: uploader token can not be scoped", func() { + _, err := s.credentialsSvc.Get(ctx, &pb.CASCredentialsServiceGetRequest{ + Digest: digestProjectA, + Role: pb.CASCredentialsServiceGetRequest_ROLE_UPLOADER, + ScopeToDigest: true, + }) + s.True(kerrors.IsBadRequest(err), "expected bad request, got %v", err) + }) +} + +// casClaims decodes the claims of a CAS token without verifying its signature +func (s *casDownloadRBACIntegrationSuite) casClaims(token string) *casJWT.Claims { + claims := &casJWT.Claims{} + _, _, err := jwt.NewParser().ParseUnverified(token, claims) + s.Require().NoError(err) + + return claims +} + // ctxForToken builds a context equivalent to the one the authz middlewares produce for an // API-token authenticated call: current org, current API token and the api-token authz subject. func (s *casDownloadRBACIntegrationSuite) ctxForToken(token *biz.APIToken) context.Context { diff --git a/app/controlplane/pkg/biz/casclient.go b/app/controlplane/pkg/biz/casclient.go index f09a3f0f7..1227ad401 100644 --- a/app/controlplane/pkg/biz/casclient.go +++ b/app/controlplane/pkg/biz/casclient.go @@ -134,7 +134,7 @@ func (uc *CASClientUseCase) Download(ctx context.Context, backendType, secretID uc.logger.Infow("msg", "download initialized", "digest", digest) // SourceInternal flags this as the control plane's own traffic so the CAS doesn't emit audit events for it - client, closeFn, err := uc.casAPIClient(&CASCredsOpts{BackendType: backendType, SecretPath: secretID, Role: casJWT.Downloader, OrgID: orgID, SourceInternal: true}) + client, closeFn, err := uc.casAPIClient(&CASCredsOpts{BackendType: backendType, SecretPath: secretID, Role: casJWT.Downloader, OrgID: orgID, SourceInternal: true, Digest: digest}) if err != nil { return fmt.Errorf("failed to create cas client: %w", err) } diff --git a/app/controlplane/pkg/biz/cascredentials.go b/app/controlplane/pkg/biz/cascredentials.go index cf7e36f15..468dcd42f 100644 --- a/app/controlplane/pkg/biz/cascredentials.go +++ b/app/controlplane/pkg/biz/cascredentials.go @@ -58,6 +58,8 @@ type CASCredsOpts struct { // SourceInternal flags tokens minted for the control plane's own CAS // client so the CAS can skip audit events for internal traffic SourceInternal bool + // Digest, when set, scopes a downloader token to that single artifact + Digest string } func (uc *CASCredentialsUseCase) GenerateTemporaryCredentials(backendRef *CASCredsOpts) (string, error) { @@ -70,5 +72,17 @@ func (uc *CASCredentialsUseCase) GenerateTemporaryCredentials(backendRef *CASCre opts = append(opts, robotaccount.WithSourceInternal()) } + if backendRef.Digest != "" { + if backendRef.Role != robotaccount.Downloader { + return "", NewErrValidationStr("only downloader credentials can be scoped to a digest") + } + + if err := validateDigest(backendRef.Digest); err != nil { + return "", err + } + + opts = append(opts, robotaccount.WithDigest(backendRef.Digest)) + } + return uc.jwtBuilder.GenerateJWT(backendRef.BackendType, backendRef.SecretPath, jwt.CASAudience, backendRef.Role, backendRef.MaxBytes, backendRef.OrgID.String(), opts...) } diff --git a/internal/robotaccount/cas/robotaccount.go b/internal/robotaccount/cas/robotaccount.go index 9c9d6e2f3..11cfe4859 100644 --- a/internal/robotaccount/cas/robotaccount.go +++ b/internal/robotaccount/cas/robotaccount.go @@ -21,11 +21,13 @@ import ( "errors" "fmt" "os" + "strings" "time" kerrors "github.com/go-kratos/kratos/v2/errors" kratosjwt "github.com/go-kratos/kratos/v2/middleware/auth/jwt" "github.com/golang-jwt/jwt/v5" + cr_v1 "github.com/google/go-containerregistry/pkg/v1" ) var SigningMethod = jwt.SigningMethodES512 @@ -51,6 +53,10 @@ type Claims struct { // The CAS skips audit event emission for this traffic so it doesn't // pollute per-org usage numbers. The zero value (false) means client traffic. SourceInternal bool `json:"source-internal,omitempty"` + // Digest, when set, scopes a downloader token to a single artifact, i.e sha256:[hex]. + // The CAS rejects reads of any other digest. An empty value keeps the + // token valid for every artifact in the backend. + Digest string `json:"digest,omitempty"` } type Role string @@ -127,6 +133,13 @@ func WithSourceInternal() GenerateOpt { } } +// WithDigest scopes a downloader token to a single artifact digest +func WithDigest(digest string) GenerateOpt { + return func(c *Claims) { + c.Digest = digest + } +} + // GenerateJWT mints a CAS token. All fields are required, including // orgID — managed providers (e.g. AWS-S3-ACCESS-POINT) need it to scope // per-tenant STS sessions and other providers still record it for @@ -179,6 +192,19 @@ func (ra *Builder) GenerateJWT(backendType, secretID, audience string, role Role opt(claims) } + if claims.Digest != "" { + if role != Downloader { + return "", fmt.Errorf("only downloader tokens can be scoped to a digest") + } + + h, err := parseDigest(claims.Digest) + if err != nil { + return "", fmt.Errorf("invalid digest: %w", err) + } + + claims.Digest = h.String() + } + resultToken := jwt.NewWithClaims(SigningMethod, claims) return resultToken.SignedString(ra.pk) } @@ -216,6 +242,36 @@ func (c *Claims) CheckRole(r Role) error { return nil } +// CheckDigest verifies that the token can read the given digest. +// The digest can be in its full form, i.e sha256:[hex], or only the hex part. +// Tokens without a digest claim can read any digest. +func (c *Claims) CheckDigest(digest string) error { + if c.Digest == "" { + return nil + } + + requested, err := parseDigest(digest) + if err != nil { + return fmt.Errorf("invalid digest: %w", err) + } + + // GenerateJWT stores the claim in its normalized form + if requested.String() != c.Digest { + return errors.New("token not authorized for this digest") + } + + return nil +} + +// parseDigest parses a digest in its full form or a sha256 hex string +func parseDigest(digest string) (cr_v1.Hash, error) { + if !strings.Contains(digest, ":") { + digest = "sha256:" + digest + } + + return cr_v1.NewHash(digest) +} + // InfoFromAuth extracts the JWT claims from the context, note that the JWT verification has happened in the middleware func InfoFromAuth(ctx context.Context) (*Claims, error) { rawClaims, ok := kratosjwt.FromContext(ctx) diff --git a/internal/robotaccount/cas/robotaccount_test.go b/internal/robotaccount/cas/robotaccount_test.go index 6175a652d..a33cae31e 100644 --- a/internal/robotaccount/cas/robotaccount_test.go +++ b/internal/robotaccount/cas/robotaccount_test.go @@ -187,6 +187,83 @@ func TestGenerateJWT(t *testing.T) { } } +func TestGenerateJWTWithDigest(t *testing.T) { + b, err := NewBuilder( + WithIssuer("my-issuer"), + WithPrivateKey("testdata/test-key.ec.pem"), + WithExpiration(5*time.Second), + ) + require.NoError(t, err) + + const hex = "b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c" + + tests := []struct { + name string + role Role + digest string + wantDigest string + wantErr bool + }{ + {name: "downloader with prefixed digest", role: Downloader, digest: "sha256:" + hex, wantDigest: "sha256:" + hex}, + {name: "downloader with bare hex digest", role: Downloader, digest: hex, wantDigest: "sha256:" + hex}, + {name: "invalid digest", role: Downloader, digest: "sha256:deadbeef", wantErr: true}, + {name: "uploader can not be scoped to a digest", role: Uploader, digest: "sha256:" + hex, wantErr: true}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + token, err := b.GenerateJWT("OCI", "secret-id", JWTAudience, tc.role, 0, "org-uuid", WithDigest(tc.digest)) + if tc.wantErr { + assert.Error(t, err) + return + } + require.NoError(t, err) + + rawKey, err := os.ReadFile("testdata/test-key.ec.pub") + require.NoError(t, err) + + claims := &Claims{} + _, err = jwt.ParseWithClaims(token, claims, loadPublicKey(rawKey)) + require.NoError(t, err) + assert.Equal(t, tc.wantDigest, claims.Digest) + }) + } +} + +func TestCheckDigest(t *testing.T) { + const ( + hex = "b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c" + otherHex = "7d865e959b2466918c9863afca942d0fb89d7c9ac0c99bafc3749504ded97730" + ) + + tests := []struct { + name string + claim string + requested string + wantErr bool + }{ + {name: "no claim allows any digest", claim: "", requested: otherHex}, + {name: "matching bare hex", claim: "sha256:" + hex, requested: hex}, + {name: "matching prefixed digest", claim: "sha256:" + hex, requested: "sha256:" + hex}, + {name: "different digest", claim: "sha256:" + hex, requested: otherHex, wantErr: true}, + {name: "different prefixed digest", claim: "sha256:" + hex, requested: "sha256:" + otherHex, wantErr: true}, + {name: "empty requested digest", claim: "sha256:" + hex, requested: "", wantErr: true}, + {name: "invalid requested digest", claim: "sha256:" + hex, requested: "sha256:nothex", wantErr: true}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + c := &Claims{Role: Downloader, Digest: tc.claim} + err := c.CheckDigest(tc.requested) + if tc.wantErr { + assert.Error(t, err) + return + } + assert.NoError(t, err) + }) + } +} + // load key for verification func loadPublicKey(rawKey []byte) jwt.Keyfunc { return func(_ *jwt.Token) (any, error) { From 327458c7cc63af99974d14b1be818b006d3095e7 Mon Sep 17 00:00:00 2001 From: Miguel Martinez Trivino Date: Mon, 26 Oct 2026 20:30:51 +0100 Subject: [PATCH 2/4] feat(cas): scope CAS uploader tokens to a single digest Uploader tokens can now carry the digest claim too, and the CAS rejects writes of any other digest before it checks whether the artifact exists. The control plane CAS client and chainloop artifact upload request scoped upload tokens, so a leaked token can only upload that one file. Assisted-by: Claude Code Signed-off-by: Miguel Martinez Trivino Chainloop-Trace-Sessions: 34d67f3d-296c-4dce-b668-3a73637e6a43 --- .../internal/service/bytestream.go | 6 +++ .../internal/service/bytestream_test.go | 50 +++++++++++++++++-- app/cli/cmd/artifact.go | 6 +-- app/cli/cmd/artifact_upload.go | 15 ++++-- app/cli/pkg/action/artifact_upload.go | 9 ++-- .../api/controlplane/v1/cas_credentials.pb.go | 6 +-- .../api/controlplane/v1/cas_credentials.proto | 6 +-- .../controlplane/v1/cas_credentials.ts | 6 +-- ...edentialsServiceGetRequest.jsonschema.json | 6 +-- ...ASCredentialsServiceGetRequest.schema.json | 6 +-- .../service/casredirect_integration_test.go | 24 +++++---- app/controlplane/pkg/biz/casclient.go | 2 +- app/controlplane/pkg/biz/cascredentials.go | 6 +-- internal/robotaccount/cas/robotaccount.go | 14 ++---- .../robotaccount/cas/robotaccount_test.go | 2 +- pkg/casclient/uploader.go | 28 +++++++++-- 16 files changed, 131 insertions(+), 61 deletions(-) diff --git a/app/artifact-cas/internal/service/bytestream.go b/app/artifact-cas/internal/service/bytestream.go index 8b72d1074..b3f886258 100644 --- a/app/artifact-cas/internal/service/bytestream.go +++ b/app/artifact-cas/internal/service/bytestream.go @@ -79,6 +79,12 @@ func (s *ByteStreamService) Write(stream bytestream.ByteStream_WriteServer) erro return kerrors.BadRequest("resource name", err.Error()) } + // A token scoped to a digest can only write that digest. Checked before the + // existence lookup so the token can not probe for other artifacts. + if err := info.CheckDigest(req.resource.Digest); err != nil { + return kerrors.Forbidden("service", err.Error()) + } + // A blob cached as present skips the secrets manager and the backend. The // key is namespaced by the organization and backend of the signed token. existenceKey := s.existenceKey(info, req.resource.Digest) diff --git a/app/artifact-cas/internal/service/bytestream_test.go b/app/artifact-cas/internal/service/bytestream_test.go index 39c93de83..f2fe4e230 100644 --- a/app/artifact-cas/internal/service/bytestream_test.go +++ b/app/artifact-cas/internal/service/bytestream_test.go @@ -204,6 +204,51 @@ func (s *bytestreamSuite) TestWriteOK() { s.Equal(resource.FileName, info.FileName) } +// otherDigest is a digest a scoped token is not authorized for +const otherDigest = "7d865e959b2466918c9863afca942d0fb89d7c9ac0c99bafc3749504ded97730" + +func (s *bytestreamSuite) TestWriteAuthorizedDigest() { + data := []byte("hello world") + resource := resourceWithDigest(data, "skynet.exe") + + testCases := []struct { + name string + authorizedDigest string + wantErr bool + }{ + {name: "token scoped to the digest", authorizedDigest: "sha256:" + resource.Digest}, + {name: "token scoped to another digest", authorizedDigest: "sha256:" + otherDigest, wantErr: true}, + } + + for _, tc := range testCases { + s.Run(tc.name, func() { + // the mock fails the test on any backend call for a forbidden digest + if !tc.wantErr { + s.ociBackend.On("Exists", mock.Anything, resource.Digest).Return(false, nil) + s.ociBackend.On("Upload", mock.Anything, mock.Anything, resource).Return(nil) + } + + ctx := metadata.AppendToOutgoingContext(s.upCtx, "authorized-digest", tc.authorizedDigest) + stream, err := s.client.Write(ctx) + s.Require().NoError(err) + _ = stream.Send(&bytestream.WriteRequest{ + ResourceName: encodeResource(s.T(), resource), + Data: data, + }) + + got, err := stream.CloseAndRecv() + if tc.wantErr { + assertGRPCError(s.T(), err, codes.PermissionDenied, "not authorized for this digest") + s.Empty(s.audit.published) + return + } + + s.Require().NoError(err) + s.Equal(int64(len(data)), got.CommittedSize) + }) + } +} + func (s *bytestreamSuite) TestWriteErrorUploading() { data := []byte("hello world") resource := resourceWithDigest(data, "skynet.exe") @@ -390,10 +435,7 @@ func (s *bytestreamSuite) TestDownloadOk() { } func (s *bytestreamSuite) TestReadAuthorizedDigest() { - const ( - digest = "b94d27b9934d3e08a52e52d7da7dabfac484efe37a5380ee9088f7ace2efcde9" - otherDigest = "7d865e959b2466918c9863afca942d0fb89d7c9ac0c99bafc3749504ded97730" - ) + const digest = "b94d27b9934d3e08a52e52d7da7dabfac484efe37a5380ee9088f7ace2efcde9" testCases := []struct { name string diff --git a/app/cli/cmd/artifact.go b/app/cli/cmd/artifact.go index d55bcdcec..83a57b14a 100644 --- a/app/cli/cmd/artifact.go +++ b/app/cli/cmd/artifact.go @@ -37,13 +37,13 @@ func newArtifactCmd() *cobra.Command { } func wrappedArtifactConn(cpConn *grpc.ClientConn, role pb.CASCredentialsServiceGetRequest_Role, digest string) (*grpc.ClientConn, error) { - // Retrieve temporary credentials for uploading + // Retrieve temporary credentials for the operation client := pb.NewCASCredentialsServiceClient(cpConn) resp, err := client.Get(context.Background(), &pb.CASCredentialsServiceGetRequest{ Role: role, Digest: digest, - // a download is for a single digest, so the token can only read that digest - ScopeToDigest: role == pb.CASCredentialsServiceGetRequest_ROLE_DOWNLOADER && digest != "", + // an upload or a download is for a single digest, so the token can only access that digest + ScopeToDigest: digest != "", }) if err != nil { return nil, err diff --git a/app/cli/cmd/artifact_upload.go b/app/cli/cmd/artifact_upload.go index 2b13dd927..1b08468fe 100644 --- a/app/cli/cmd/artifact_upload.go +++ b/app/cli/cmd/artifact_upload.go @@ -1,5 +1,5 @@ // -// Copyright 2023 The Chainloop Authors. +// Copyright 2023-2026 The Chainloop Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -18,12 +18,13 @@ package cmd import ( "github.com/chainloop-dev/chainloop/app/cli/pkg/action" pb "github.com/chainloop-dev/chainloop/app/controlplane/api/controlplane/v1" + "github.com/chainloop-dev/chainloop/pkg/casclient" "github.com/spf13/cobra" "google.golang.org/grpc" ) func newArtifactUploadCmd() *cobra.Command { - var filePath string + var filePath, digest string var artifactCASConn *grpc.ClientConn cmd := &cobra.Command{ @@ -31,9 +32,13 @@ func newArtifactUploadCmd() *cobra.Command { Short: "upload an artifact", PreRunE: func(cmd *cobra.Command, args []string) error { var err error + digest, err = casclient.FileDigest(filePath) + if err != nil { + return err + } - // Retrieve temporary credentials for uploading - artifactCASConn, err = wrappedArtifactConn(ActionOpts.CPConnection, pb.CASCredentialsServiceGetRequest_ROLE_UPLOADER, "") + // Retrieve temporary credentials for uploading, scoped to the file digest + artifactCASConn, err = wrappedArtifactConn(ActionOpts.CPConnection, pb.CASCredentialsServiceGetRequest_ROLE_UPLOADER, digest) if err != nil { return err } @@ -46,7 +51,7 @@ func newArtifactUploadCmd() *cobra.Command { ArtifactsCASConn: artifactCASConn, } - _, err := action.NewArtifactUpload(opts).Run(filePath) + _, err := action.NewArtifactUpload(opts).Run(filePath, digest) return err }, PostRunE: func(cmd *cobra.Command, args []string) error { diff --git a/app/cli/pkg/action/artifact_upload.go b/app/cli/pkg/action/artifact_upload.go index 11bf64fb6..821657bde 100644 --- a/app/cli/pkg/action/artifact_upload.go +++ b/app/cli/pkg/action/artifact_upload.go @@ -1,5 +1,5 @@ // -// Copyright 2023-2025 The Chainloop Authors. +// Copyright 2023-2026 The Chainloop Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -19,6 +19,7 @@ import ( "context" "fmt" "os" + "path" "time" "github.com/chainloop-dev/chainloop/pkg/casclient" @@ -44,7 +45,9 @@ func NewArtifactUpload(opts *ArtifactUploadOpts) *ArtifactUpload { return &ArtifactUpload{opts.ActionsOpts, opts.ArtifactsCASConn} } -func (a *ArtifactUpload) Run(filePath string) (*CASArtifact, error) { +// Run uploads the file. The digest is the one the credentials were scoped to, +// so the file is not hashed again. +func (a *ArtifactUpload) Run(filePath, digest string) (*CASArtifact, error) { client := casclient.New(a.artifactsCASConn, casclient.WithLogger(a.Logger)) // open file and calculate size @@ -63,7 +66,7 @@ func (a *ArtifactUpload) Run(filePath string) (*CASArtifact, error) { go renderOperationStatus(context.Background(), client.ProgressStatus, info.Size()) defer close(client.ProgressStatus) - res, err := client.UploadFile(context.Background(), filePath) + res, err := client.Upload(context.Background(), f, path.Base(filePath), digest) if err != nil { return nil, err } diff --git a/app/controlplane/api/controlplane/v1/cas_credentials.pb.go b/app/controlplane/api/controlplane/v1/cas_credentials.pb.go index afab483b8..3d1fbcfe6 100644 --- a/app/controlplane/api/controlplane/v1/cas_credentials.pb.go +++ b/app/controlplane/api/controlplane/v1/cas_credentials.pb.go @@ -89,13 +89,13 @@ func (CASCredentialsServiceGetRequest_Role) EnumDescriptor() ([]byte, []int) { type CASCredentialsServiceGetRequest struct { state protoimpl.MessageState `protogen:"open.v1"` Role CASCredentialsServiceGetRequest_Role `protobuf:"varint,1,opt,name=role,proto3,enum=controlplane.v1.CASCredentialsServiceGetRequest_Role" json:"role,omitempty"` - // during the download we need the digest to find the proper cas backend + // digest of the artifact, used to find the backend of a download and to scope the token when scope_to_digest is set Digest string `protobuf:"bytes,2,opt,name=digest,proto3" json:"digest,omitempty"` // flag the minted token as internal platform traffic so the CAS skips audit // event emission for it. Only honored for system API tokens, rejected otherwise. SourceInternal bool `protobuf:"varint,3,opt,name=source_internal,json=sourceInternal,proto3" json:"source_internal,omitempty"` - // scope the downloader token to the requested digest, so the CAS rejects reads of any other artifact. - // Requires the downloader role and a digest. + // scope the token to the requested digest, so the CAS rejects reads or writes of any other artifact. + // Requires a digest. ScopeToDigest bool `protobuf:"varint,4,opt,name=scope_to_digest,json=scopeToDigest,proto3" json:"scope_to_digest,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache diff --git a/app/controlplane/api/controlplane/v1/cas_credentials.proto b/app/controlplane/api/controlplane/v1/cas_credentials.proto index 0585d93a4..1a1941565 100644 --- a/app/controlplane/api/controlplane/v1/cas_credentials.proto +++ b/app/controlplane/api/controlplane/v1/cas_credentials.proto @@ -33,13 +33,13 @@ message CASCredentialsServiceGetRequest { 2 ] }]; - // during the download we need the digest to find the proper cas backend + // digest of the artifact, used to find the backend of a download and to scope the token when scope_to_digest is set string digest = 2; // flag the minted token as internal platform traffic so the CAS skips audit // event emission for it. Only honored for system API tokens, rejected otherwise. bool source_internal = 3; - // scope the downloader token to the requested digest, so the CAS rejects reads of any other artifact. - // Requires the downloader role and a digest. + // scope the token to the requested digest, so the CAS rejects reads or writes of any other artifact. + // Requires a digest. bool scope_to_digest = 4; enum Role { diff --git a/app/controlplane/api/gen/frontend/controlplane/v1/cas_credentials.ts b/app/controlplane/api/gen/frontend/controlplane/v1/cas_credentials.ts index 2bc96fe5f..d39a30012 100644 --- a/app/controlplane/api/gen/frontend/controlplane/v1/cas_credentials.ts +++ b/app/controlplane/api/gen/frontend/controlplane/v1/cas_credentials.ts @@ -8,7 +8,7 @@ export const protobufPackage = "controlplane.v1"; export interface CASCredentialsServiceGetRequest { role: CASCredentialsServiceGetRequest_Role; - /** during the download we need the digest to find the proper cas backend */ + /** digest of the artifact, used to find the backend of a download and to scope the token when scope_to_digest is set */ digest: string; /** * flag the minted token as internal platform traffic so the CAS skips audit @@ -16,8 +16,8 @@ export interface CASCredentialsServiceGetRequest { */ sourceInternal: boolean; /** - * scope the downloader token to the requested digest, so the CAS rejects reads of any other artifact. - * Requires the downloader role and a digest. + * scope the token to the requested digest, so the CAS rejects reads or writes of any other artifact. + * Requires a digest. */ scopeToDigest: boolean; } diff --git a/app/controlplane/api/gen/jsonschema/controlplane.v1.CASCredentialsServiceGetRequest.jsonschema.json b/app/controlplane/api/gen/jsonschema/controlplane.v1.CASCredentialsServiceGetRequest.jsonschema.json index 9931aaa85..6fc1ddff0 100644 --- a/app/controlplane/api/gen/jsonschema/controlplane.v1.CASCredentialsServiceGetRequest.jsonschema.json +++ b/app/controlplane/api/gen/jsonschema/controlplane.v1.CASCredentialsServiceGetRequest.jsonschema.json @@ -4,7 +4,7 @@ "additionalProperties": false, "patternProperties": { "^(scope_to_digest)$": { - "description": "scope the downloader token to the requested digest, so the CAS rejects reads of any other artifact.\n Requires the downloader role and a digest.", + "description": "scope the token to the requested digest, so the CAS rejects reads or writes of any other artifact.\n Requires a digest.", "type": "boolean" }, "^(source_internal)$": { @@ -14,7 +14,7 @@ }, "properties": { "digest": { - "description": "during the download we need the digest to find the proper cas backend", + "description": "digest of the artifact, used to find the backend of a download and to scope the token when scope_to_digest is set", "type": "string" }, "role": { @@ -36,7 +36,7 @@ ] }, "scopeToDigest": { - "description": "scope the downloader token to the requested digest, so the CAS rejects reads of any other artifact.\n Requires the downloader role and a digest.", + "description": "scope the token to the requested digest, so the CAS rejects reads or writes of any other artifact.\n Requires a digest.", "type": "boolean" }, "sourceInternal": { diff --git a/app/controlplane/api/gen/jsonschema/controlplane.v1.CASCredentialsServiceGetRequest.schema.json b/app/controlplane/api/gen/jsonschema/controlplane.v1.CASCredentialsServiceGetRequest.schema.json index d483dad5c..8867815a3 100644 --- a/app/controlplane/api/gen/jsonschema/controlplane.v1.CASCredentialsServiceGetRequest.schema.json +++ b/app/controlplane/api/gen/jsonschema/controlplane.v1.CASCredentialsServiceGetRequest.schema.json @@ -4,7 +4,7 @@ "additionalProperties": false, "patternProperties": { "^(scopeToDigest)$": { - "description": "scope the downloader token to the requested digest, so the CAS rejects reads of any other artifact.\n Requires the downloader role and a digest.", + "description": "scope the token to the requested digest, so the CAS rejects reads or writes of any other artifact.\n Requires a digest.", "type": "boolean" }, "^(sourceInternal)$": { @@ -14,7 +14,7 @@ }, "properties": { "digest": { - "description": "during the download we need the digest to find the proper cas backend", + "description": "digest of the artifact, used to find the backend of a download and to scope the token when scope_to_digest is set", "type": "string" }, "role": { @@ -36,7 +36,7 @@ ] }, "scope_to_digest": { - "description": "scope the downloader token to the requested digest, so the CAS rejects reads of any other artifact.\n Requires the downloader role and a digest.", + "description": "scope the token to the requested digest, so the CAS rejects reads or writes of any other artifact.\n Requires a digest.", "type": "boolean" }, "source_internal": { diff --git a/app/controlplane/internal/service/casredirect_integration_test.go b/app/controlplane/internal/service/casredirect_integration_test.go index 870057c05..e81b190e6 100644 --- a/app/controlplane/internal/service/casredirect_integration_test.go +++ b/app/controlplane/internal/service/casredirect_integration_test.go @@ -185,7 +185,7 @@ func (s *casDownloadRBACIntegrationSuite) TestCASCredentialsGet() { }) } -// SEC-634: callers can ask for a downloader token that can only read the requested digest +// SEC-634: callers can ask for a CAS token that can only access the requested digest func (s *casDownloadRBACIntegrationSuite) TestDigestScopedTokens() { ctx := s.ctxForToken(s.orgToken) @@ -208,15 +208,17 @@ func (s *casDownloadRBACIntegrationSuite) TestDigestScopedTokens() { s.Equal(tc.wantDigest, s.casClaims(u.Query().Get("t")).Digest) }) - s.Run("CASCredentialsService.Get: "+tc.name, func() { - resp, err := s.credentialsSvc.Get(ctx, &pb.CASCredentialsServiceGetRequest{ - Digest: digestProjectA, - Role: pb.CASCredentialsServiceGetRequest_ROLE_DOWNLOADER, - ScopeToDigest: tc.scopeToDigest, + for _, role := range []pb.CASCredentialsServiceGetRequest_Role{pb.CASCredentialsServiceGetRequest_ROLE_DOWNLOADER, pb.CASCredentialsServiceGetRequest_ROLE_UPLOADER} { + s.Run("CASCredentialsService.Get "+role.String()+": "+tc.name, func() { + resp, err := s.credentialsSvc.Get(ctx, &pb.CASCredentialsServiceGetRequest{ + Digest: digestProjectA, + Role: role, + ScopeToDigest: tc.scopeToDigest, + }) + s.Require().NoError(err) + s.Equal(tc.wantDigest, s.casClaims(resp.GetResult().GetToken()).Digest) }) - s.Require().NoError(err) - s.Equal(tc.wantDigest, s.casClaims(resp.GetResult().GetToken()).Digest) - }) + } } s.Run("CASCredentialsService.Get: scoped token requires a digest", func() { @@ -227,9 +229,9 @@ func (s *casDownloadRBACIntegrationSuite) TestDigestScopedTokens() { s.True(kerrors.IsBadRequest(err), "expected bad request, got %v", err) }) - s.Run("CASCredentialsService.Get: uploader token can not be scoped", func() { + s.Run("CASCredentialsService.Get: invalid digest", func() { _, err := s.credentialsSvc.Get(ctx, &pb.CASCredentialsServiceGetRequest{ - Digest: digestProjectA, + Digest: "sha256:deadbeef", Role: pb.CASCredentialsServiceGetRequest_ROLE_UPLOADER, ScopeToDigest: true, }) diff --git a/app/controlplane/pkg/biz/casclient.go b/app/controlplane/pkg/biz/casclient.go index 1227ad401..2527c7dda 100644 --- a/app/controlplane/pkg/biz/casclient.go +++ b/app/controlplane/pkg/biz/casclient.go @@ -111,7 +111,7 @@ func (uc *CASClientUseCase) Upload(ctx context.Context, backendType, secretID st // client with temporary set of credentials // SourceInternal flags this as the control plane's own traffic so the CAS doesn't emit audit events for it - client, closeFn, err := uc.casAPIClient(&CASCredsOpts{BackendType: backendType, SecretPath: secretID, Role: casJWT.Uploader, OrgID: orgID, SourceInternal: true}) + client, closeFn, err := uc.casAPIClient(&CASCredsOpts{BackendType: backendType, SecretPath: secretID, Role: casJWT.Uploader, OrgID: orgID, SourceInternal: true, Digest: digest}) if err != nil { return fmt.Errorf("failed to create cas client: %w", err) } diff --git a/app/controlplane/pkg/biz/cascredentials.go b/app/controlplane/pkg/biz/cascredentials.go index 468dcd42f..8fab0d5ca 100644 --- a/app/controlplane/pkg/biz/cascredentials.go +++ b/app/controlplane/pkg/biz/cascredentials.go @@ -58,7 +58,7 @@ type CASCredsOpts struct { // SourceInternal flags tokens minted for the control plane's own CAS // client so the CAS can skip audit events for internal traffic SourceInternal bool - // Digest, when set, scopes a downloader token to that single artifact + // Digest, when set, scopes the token to that single artifact Digest string } @@ -73,10 +73,6 @@ func (uc *CASCredentialsUseCase) GenerateTemporaryCredentials(backendRef *CASCre } if backendRef.Digest != "" { - if backendRef.Role != robotaccount.Downloader { - return "", NewErrValidationStr("only downloader credentials can be scoped to a digest") - } - if err := validateDigest(backendRef.Digest); err != nil { return "", err } diff --git a/internal/robotaccount/cas/robotaccount.go b/internal/robotaccount/cas/robotaccount.go index 11cfe4859..eccaf2f38 100644 --- a/internal/robotaccount/cas/robotaccount.go +++ b/internal/robotaccount/cas/robotaccount.go @@ -53,8 +53,8 @@ type Claims struct { // The CAS skips audit event emission for this traffic so it doesn't // pollute per-org usage numbers. The zero value (false) means client traffic. SourceInternal bool `json:"source-internal,omitempty"` - // Digest, when set, scopes a downloader token to a single artifact, i.e sha256:[hex]. - // The CAS rejects reads of any other digest. An empty value keeps the + // Digest, when set, scopes the token to a single artifact, i.e sha256:[hex]. + // The CAS rejects reads and writes of any other digest. An empty value keeps the // token valid for every artifact in the backend. Digest string `json:"digest,omitempty"` } @@ -133,7 +133,7 @@ func WithSourceInternal() GenerateOpt { } } -// WithDigest scopes a downloader token to a single artifact digest +// WithDigest scopes the token to a single artifact digest func WithDigest(digest string) GenerateOpt { return func(c *Claims) { c.Digest = digest @@ -193,10 +193,6 @@ func (ra *Builder) GenerateJWT(backendType, secretID, audience string, role Role } if claims.Digest != "" { - if role != Downloader { - return "", fmt.Errorf("only downloader tokens can be scoped to a digest") - } - h, err := parseDigest(claims.Digest) if err != nil { return "", fmt.Errorf("invalid digest: %w", err) @@ -242,9 +238,9 @@ func (c *Claims) CheckRole(r Role) error { return nil } -// CheckDigest verifies that the token can read the given digest. +// CheckDigest verifies that the token can read or write the given digest. // The digest can be in its full form, i.e sha256:[hex], or only the hex part. -// Tokens without a digest claim can read any digest. +// Tokens without a digest claim can access any digest. func (c *Claims) CheckDigest(digest string) error { if c.Digest == "" { return nil diff --git a/internal/robotaccount/cas/robotaccount_test.go b/internal/robotaccount/cas/robotaccount_test.go index a33cae31e..01e853416 100644 --- a/internal/robotaccount/cas/robotaccount_test.go +++ b/internal/robotaccount/cas/robotaccount_test.go @@ -207,7 +207,7 @@ func TestGenerateJWTWithDigest(t *testing.T) { {name: "downloader with prefixed digest", role: Downloader, digest: "sha256:" + hex, wantDigest: "sha256:" + hex}, {name: "downloader with bare hex digest", role: Downloader, digest: hex, wantDigest: "sha256:" + hex}, {name: "invalid digest", role: Downloader, digest: "sha256:deadbeef", wantErr: true}, - {name: "uploader can not be scoped to a digest", role: Uploader, digest: "sha256:" + hex, wantErr: true}, + {name: "uploader with prefixed digest", role: Uploader, digest: "sha256:" + hex, wantDigest: "sha256:" + hex}, } for _, tc := range tests { diff --git a/pkg/casclient/uploader.go b/pkg/casclient/uploader.go index 550e66ccd..c31787720 100644 --- a/pkg/casclient/uploader.go +++ b/pkg/casclient/uploader.go @@ -1,5 +1,5 @@ // -// Copyright 2023-2025 The Chainloop Authors. +// Copyright 2023-2026 The Chainloop Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -44,9 +44,9 @@ func (c *Client) UploadFile(ctx context.Context, filepath string) (*UpDownStatus } defer f.Close() - hash, _, err := cr_v1.SHA256(f) + digest, err := readerDigest(f) if err != nil { - return nil, fmt.Errorf("generating digest: %w", err) + return nil, err } // Since we have already iterated on the file to calculate the digest @@ -56,7 +56,27 @@ func (c *Client) UploadFile(ctx context.Context, filepath string) (*UpDownStatus return nil, fmt.Errorf("rewinding file pointer: %w", err) } - return c.Upload(ctx, f, path.Base(filepath), hash.String()) + return c.Upload(ctx, f, path.Base(filepath), digest) +} + +// FileDigest returns the digest of the file as the CAS stores it, i.e sha256:[hex] +func FileDigest(filepath string) (string, error) { + f, err := os.Open(filepath) + if err != nil { + return "", fmt.Errorf("can't open file to upload: %w", err) + } + defer f.Close() + + return readerDigest(f) +} + +func readerDigest(r io.Reader) (string, error) { + hash, _, err := cr_v1.SHA256(r) + if err != nil { + return "", fmt.Errorf("generating digest: %w", err) + } + + return hash.String(), nil } func (c *Client) Upload(ctx context.Context, r io.Reader, filename, digest string) (*UpDownStatus, error) { From 58141a1a2dc3dd51f7502f38b449915626219d1a Mon Sep 17 00:00:00 2001 From: Miguel Martinez Trivino Date: Mon, 26 Oct 2026 20:37:24 +0100 Subject: [PATCH 3/4] fix(cli): upload the hashed content in chainloop artifact upload The command now keeps the file open from hashing to upload, so the uploaded content is always the content the scoped token was minted for, even if the file at the path is replaced in between. Assisted-by: Claude Code Signed-off-by: Miguel Martinez Trivino Chainloop-Trace-Sessions: 34d67f3d-296c-4dce-b668-3a73637e6a43 --- app/cli/cmd/artifact_upload.go | 24 +++- app/cli/pkg/action/artifact_upload.go | 16 +-- app/cli/pkg/action/artifact_upload_test.go | 138 +++++++++++++++++++++ pkg/casclient/uploader.go | 28 ++--- 4 files changed, 171 insertions(+), 35 deletions(-) create mode 100644 app/cli/pkg/action/artifact_upload_test.go diff --git a/app/cli/cmd/artifact_upload.go b/app/cli/cmd/artifact_upload.go index 1b08468fe..d8b033502 100644 --- a/app/cli/cmd/artifact_upload.go +++ b/app/cli/cmd/artifact_upload.go @@ -16,6 +16,10 @@ package cmd import ( + "errors" + "fmt" + "os" + "github.com/chainloop-dev/chainloop/app/cli/pkg/action" pb "github.com/chainloop-dev/chainloop/app/controlplane/api/controlplane/v1" "github.com/chainloop-dev/chainloop/pkg/casclient" @@ -25,6 +29,7 @@ import ( func newArtifactUploadCmd() *cobra.Command { var filePath, digest string + var file *os.File var artifactCASConn *grpc.ClientConn cmd := &cobra.Command{ @@ -32,7 +37,13 @@ func newArtifactUploadCmd() *cobra.Command { Short: "upload an artifact", PreRunE: func(cmd *cobra.Command, args []string) error { var err error - digest, err = casclient.FileDigest(filePath) + // The file stays open until the upload, so the uploaded content is the hashed content + file, err = os.Open(filePath) + if err != nil { + return fmt.Errorf("can't open file to upload: %w", err) + } + + digest, err = casclient.Digest(file) if err != nil { return err } @@ -51,15 +62,20 @@ func newArtifactUploadCmd() *cobra.Command { ArtifactsCASConn: artifactCASConn, } - _, err := action.NewArtifactUpload(opts).Run(filePath, digest) + _, err := action.NewArtifactUpload(opts).Run(file, digest) return err }, PostRunE: func(cmd *cobra.Command, args []string) error { + var errs []error + if file != nil { + errs = append(errs, file.Close()) + } + if artifactCASConn != nil { - return artifactCASConn.Close() + errs = append(errs, artifactCASConn.Close()) } - return nil + return errors.Join(errs...) }, } diff --git a/app/cli/pkg/action/artifact_upload.go b/app/cli/pkg/action/artifact_upload.go index 821657bde..92b479917 100644 --- a/app/cli/pkg/action/artifact_upload.go +++ b/app/cli/pkg/action/artifact_upload.go @@ -45,18 +45,12 @@ func NewArtifactUpload(opts *ArtifactUploadOpts) *ArtifactUpload { return &ArtifactUpload{opts.ActionsOpts, opts.ArtifactsCASConn} } -// Run uploads the file. The digest is the one the credentials were scoped to, -// so the file is not hashed again. -func (a *ArtifactUpload) Run(filePath, digest string) (*CASArtifact, error) { +// Run uploads the open file with its digest, the one the credentials were +// scoped to. Taking the open file, and not its path, makes the uploaded +// content the hashed content even if the file at the path is replaced. +func (a *ArtifactUpload) Run(f *os.File, digest string) (*CASArtifact, error) { client := casclient.New(a.artifactsCASConn, casclient.WithLogger(a.Logger)) - // open file and calculate size - f, err := os.Open(filePath) - if err != nil { - return nil, fmt.Errorf("can't open file to upload: %w", err) - } - defer f.Close() - info, err := f.Stat() if err != nil { return nil, fmt.Errorf("retrieving file information: %w", err) @@ -66,7 +60,7 @@ func (a *ArtifactUpload) Run(filePath, digest string) (*CASArtifact, error) { go renderOperationStatus(context.Background(), client.ProgressStatus, info.Size()) defer close(client.ProgressStatus) - res, err := client.Upload(context.Background(), f, path.Base(filePath), digest) + res, err := client.Upload(context.Background(), f, path.Base(f.Name()), digest) if err != nil { return nil, err } diff --git a/app/cli/pkg/action/artifact_upload_test.go b/app/cli/pkg/action/artifact_upload_test.go new file mode 100644 index 000000000..b5d5afcf9 --- /dev/null +++ b/app/cli/pkg/action/artifact_upload_test.go @@ -0,0 +1,138 @@ +// +// Copyright 2026 The Chainloop Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package action + +import ( + "bytes" + "context" + "crypto/sha256" + "encoding/base64" + "encoding/gob" + "encoding/hex" + "errors" + "io" + "net" + "os" + "path/filepath" + "testing" + + v1 "github.com/chainloop-dev/chainloop/app/artifact-cas/api/cas/v1" + "github.com/rs/zerolog" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "google.golang.org/genproto/googleapis/bytestream" + "google.golang.org/grpc" + "google.golang.org/grpc/credentials/insecure" + "google.golang.org/grpc/test/bufconn" +) + +func TestArtifactUploadRun(t *testing.T) { + content := []byte("hello world") + sum := sha256.Sum256(content) + digestHex := hex.EncodeToString(sum[:]) + + testCases := []struct { + name string + // replace swaps the file at the path after it was opened and hashed + replace bool + }{ + {name: "uploads the file with its digest"}, + {name: "uploads the hashed content when the file is replaced", replace: true}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + filePath := filepath.Join(t.TempDir(), "artifact.txt") + require.NoError(t, os.WriteFile(filePath, content, 0o600)) + + f, err := os.Open(filePath) + require.NoError(t, err) + defer f.Close() + + if tc.replace { + other := filepath.Join(t.TempDir(), "other.txt") + require.NoError(t, os.WriteFile(other, []byte("other content"), 0o600)) + require.NoError(t, os.Rename(other, filePath)) + } + + srv := &fakeByteStream{} + upload := NewArtifactUpload(&ArtifactUploadOpts{ + ActionsOpts: &ActionsOpts{Logger: zerolog.Nop()}, + ArtifactsCASConn: newFakeByteStreamConn(t, srv), + }) + + got, err := upload.Run(f, "sha256:"+digestHex) + require.NoError(t, err) + assert.Equal(t, "sha256:"+digestHex, got.Digest) + + assert.Equal(t, digestHex, srv.resource.Digest) + assert.Equal(t, "artifact.txt", srv.resource.FileName) + assert.Equal(t, content, srv.data.Bytes()) + }) + } +} + +// fakeByteStream records the resource and the bytes of a single upload +type fakeByteStream struct { + bytestream.UnimplementedByteStreamServer + resource v1.CASResource + data bytes.Buffer +} + +func (s *fakeByteStream) Write(stream bytestream.ByteStream_WriteServer) error { + for { + req, err := stream.Recv() + if errors.Is(err, io.EOF) { + return stream.SendAndClose(&bytestream.WriteResponse{CommittedSize: int64(s.data.Len())}) + } else if err != nil { + return err + } + + if s.resource.Digest == "" { + raw, err := base64.StdEncoding.DecodeString(req.GetResourceName()) + if err != nil { + return err + } + if err := gob.NewDecoder(bytes.NewReader(raw)).Decode(&s.resource); err != nil { + return err + } + } + + s.data.Write(req.GetData()) + } +} + +func newFakeByteStreamConn(t *testing.T, srv bytestream.ByteStreamServer) *grpc.ClientConn { + t.Helper() + + l := bufconn.Listen(1 << 20) + s := grpc.NewServer() + bytestream.RegisterByteStreamServer(s, srv) + go func() { _ = s.Serve(l) }() + + conn, err := grpc.NewClient("passthrough:///bufnet", + grpc.WithContextDialer(func(ctx context.Context, _ string) (net.Conn, error) { return l.DialContext(ctx) }), + grpc.WithTransportCredentials(insecure.NewCredentials()), + ) + require.NoError(t, err) + + t.Cleanup(func() { + conn.Close() + s.Stop() + }) + + return conn +} diff --git a/pkg/casclient/uploader.go b/pkg/casclient/uploader.go index c31787720..ecdb8a09b 100644 --- a/pkg/casclient/uploader.go +++ b/pkg/casclient/uploader.go @@ -44,38 +44,26 @@ func (c *Client) UploadFile(ctx context.Context, filepath string) (*UpDownStatus } defer f.Close() - digest, err := readerDigest(f) + digest, err := Digest(f) if err != nil { return nil, err } - // Since we have already iterated on the file to calculate the digest - // we need to rewind the file pointer - _, err = f.Seek(0, io.SeekStart) - if err != nil { - return nil, fmt.Errorf("rewinding file pointer: %w", err) - } - return c.Upload(ctx, f, path.Base(filepath), digest) } -// FileDigest returns the digest of the file as the CAS stores it, i.e sha256:[hex] -func FileDigest(filepath string) (string, error) { - f, err := os.Open(filepath) - if err != nil { - return "", fmt.Errorf("can't open file to upload: %w", err) - } - defer f.Close() - - return readerDigest(f) -} - -func readerDigest(r io.Reader) (string, error) { +// Digest returns the digest of the content as the CAS stores it, i.e sha256:[hex]. +// It rewinds r, so the hashed content can then be uploaded from the same reader. +func Digest(r io.ReadSeeker) (string, error) { hash, _, err := cr_v1.SHA256(r) if err != nil { return "", fmt.Errorf("generating digest: %w", err) } + if _, err := r.Seek(0, io.SeekStart); err != nil { + return "", fmt.Errorf("rewinding file pointer: %w", err) + } + return hash.String(), nil } From 1d2cedddae8928b13e057ac73f44cbdc40b85d5b Mon Sep 17 00:00:00 2001 From: Miguel Martinez Trivino Date: Mon, 26 Oct 2026 20:49:09 +0100 Subject: [PATCH 4/4] test(controlplane): avoid a repeated digest literal in the CAS scope test Assisted-by: Claude Code Signed-off-by: Miguel Martinez Trivino Chainloop-Trace-Sessions: 34d67f3d-296c-4dce-b668-3a73637e6a43 --- .../internal/service/casredirect_integration_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controlplane/internal/service/casredirect_integration_test.go b/app/controlplane/internal/service/casredirect_integration_test.go index e81b190e6..afc7b8c4a 100644 --- a/app/controlplane/internal/service/casredirect_integration_test.go +++ b/app/controlplane/internal/service/casredirect_integration_test.go @@ -231,7 +231,7 @@ func (s *casDownloadRBACIntegrationSuite) TestDigestScopedTokens() { s.Run("CASCredentialsService.Get: invalid digest", func() { _, err := s.credentialsSvc.Get(ctx, &pb.CASCredentialsServiceGetRequest{ - Digest: "sha256:deadbeef", + Digest: "sha256:nothex", Role: pb.CASCredentialsServiceGetRequest_ROLE_UPLOADER, ScopeToDigest: true, })