Skip to content

Commit a320f44

Browse files
committed
update helmchart reconciliation to verify charts using prov file
Signed-off-by: Sanskar Jaiswal <[email protected]>
1 parent fa3856c commit a320f44

File tree

7 files changed

+122
-11
lines changed

7 files changed

+122
-11
lines changed

api/v1beta2/helmchart_types.go

+14
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,20 @@ type HelmChartSpec struct {
8484
// NOTE: Not implemented, provisional as of https://github.com/fluxcd/flux2/pull/2092
8585
// +optional
8686
AccessFrom *acl.AccessFrom `json:"accessFrom,omitempty"`
87+
88+
// Keyring information for verifying the packaged chart's signature using a provenance file.
89+
// +optional
90+
VerificationKeyring *VerificationKeyring `json:"verificationKeyring,omitempty"`
91+
}
92+
93+
type VerificationKeyring struct {
94+
// +required
95+
SecretRef meta.LocalObjectReference `json:"secretRef,omitempty"`
96+
97+
// The key that corresponds to the keyring value.
98+
// +kubebuilder:default:=pubring.gpg
99+
// +optional
100+
Key string `json:"key,omitempty"`
87101
}
88102

89103
const (

config/crd/bases/source.toolkit.fluxcd.io_helmcharts.yaml

+19
Original file line numberDiff line numberDiff line change
@@ -404,6 +404,25 @@ spec:
404404
items:
405405
type: string
406406
type: array
407+
verificationKeyring:
408+
description: Keyring information for verifying the packaged chart's
409+
signature using a provenance file.
410+
properties:
411+
key:
412+
default: pubring.gpg
413+
description: The key that corresponds to the keyring value.
414+
type: string
415+
secretRef:
416+
description: LocalObjectReference contains enough information
417+
to locate the referenced Kubernetes resource object.
418+
properties:
419+
name:
420+
description: Name of the referent.
421+
type: string
422+
required:
423+
- name
424+
type: object
425+
type: object
407426
version:
408427
default: '*'
409428
description: Version is the chart version semver expression, ignored

controllers/helmchart_controller.go

+76-3
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,8 @@ var helmChartReadyCondition = summarize.Conditions{
9595
},
9696
}
9797

98+
const KeyringFileName = "pubring.gpg"
99+
98100
// +kubebuilder:rbac:groups=source.toolkit.fluxcd.io,resources=helmcharts,verbs=get;list;watch;create;update;patch;delete
99101
// +kubebuilder:rbac:groups=source.toolkit.fluxcd.io,resources=helmcharts/status,verbs=get;update;patch
100102
// +kubebuilder:rbac:groups=source.toolkit.fluxcd.io,resources=helmcharts/finalizers,verbs=get;create;update;patch;delete
@@ -467,13 +469,20 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
467469
opts.VersionMetadata = strconv.FormatInt(obj.Generation, 10)
468470
}
469471

472+
var keyring []byte
473+
keyring, err = r.getProvenanceKeyring(ctx, obj)
474+
if err != nil {
475+
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, err.Error())
476+
return sreconcile.ResultEmpty, err
477+
}
478+
470479
// Build the chart
471480
ref := chart.RemoteReference{Name: obj.Spec.Chart, Version: obj.Spec.Version}
472-
build, err := cb.Build(ctx, ref, util.TempPathForObj("", ".tgz", obj), opts)
481+
build, err := cb.Build(ctx, ref, util.TempPathForObj("", ".tgz", obj), opts, keyring)
482+
473483
if err != nil {
474484
return sreconcile.ResultEmpty, err
475485
}
476-
477486
*b = *build
478487
return sreconcile.ResultSuccess, nil
479488
}
@@ -590,13 +599,19 @@ func (r *HelmChartReconciler) buildFromTarballArtifact(ctx context.Context, obj
590599
}
591600
opts.VersionMetadata += strconv.FormatInt(obj.Generation, 10)
592601
}
602+
var keyring []byte
603+
keyring, err = r.getProvenanceKeyring(ctx, obj)
604+
if err != nil {
605+
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, err.Error())
606+
return sreconcile.ResultEmpty, err
607+
}
593608

594609
// Build chart
595610
cb := chart.NewLocalBuilder(dm)
596611
build, err := cb.Build(ctx, chart.LocalReference{
597612
WorkDir: sourceDir,
598613
Path: chartPath,
599-
}, util.TempPathForObj("", ".tgz", obj), opts)
614+
}, util.TempPathForObj("", ".tgz", obj), opts, keyring)
600615
if err != nil {
601616
return sreconcile.ResultEmpty, err
602617
}
@@ -670,6 +685,18 @@ func (r *HelmChartReconciler) reconcileArtifact(ctx context.Context, obj *source
670685
return sreconcile.ResultEmpty, e
671686
}
672687

688+
// the provenance file artifact is not recorded, but it shadows the HelmChart artifact
689+
// under the assumption that the file is always available at "chart.tgz.prov"
690+
if b.ProvFilePath != "" {
691+
provArtifact := r.Storage.NewArtifactFor(obj.Kind, obj.GetObjectMeta(), b.Version, fmt.Sprintf("%s-%s.tgz.prov", b.Name, b.Version))
692+
if err = r.Storage.CopyFromPath(&provArtifact, b.ProvFilePath); err != nil {
693+
return sreconcile.ResultEmpty, &serror.Event{
694+
Err: fmt.Errorf("unable to copy Helm chart provenance file to storage: %w", err),
695+
Reason: sourcev1.StorageOperationFailedReason,
696+
}
697+
}
698+
}
699+
673700
// Record it on the object
674701
obj.Status.Artifact = artifact.DeepCopy()
675702
obj.Status.ObservedChartName = b.Name
@@ -861,6 +888,22 @@ func (r *HelmChartReconciler) getHelmRepositorySecret(ctx context.Context, repos
861888
return &secret, nil
862889
}
863890

891+
func (r *HelmChartReconciler) getVerificationKeyringSecret(ctx context.Context, chart *sourcev1.HelmChart) (*corev1.Secret, error) {
892+
if chart.Spec.VerificationKeyring == nil {
893+
return nil, nil
894+
}
895+
name := types.NamespacedName{
896+
Namespace: chart.GetNamespace(),
897+
Name: chart.Spec.VerificationKeyring.SecretRef.Name,
898+
}
899+
var secret corev1.Secret
900+
err := r.Client.Get(ctx, name, &secret)
901+
if err != nil {
902+
return nil, err
903+
}
904+
return &secret, nil
905+
}
906+
864907
func (r *HelmChartReconciler) indexHelmRepositoryByURL(o client.Object) []string {
865908
repo, ok := o.(*sourcev1.HelmRepository)
866909
if !ok {
@@ -1021,3 +1064,33 @@ func reasonForBuild(build *chart.Build) string {
10211064
}
10221065
return sourcev1.ChartPullSucceededReason
10231066
}
1067+
1068+
func (r *HelmChartReconciler) getProvenanceKeyring(ctx context.Context, chart *sourcev1.HelmChart) ([]byte, error) {
1069+
if chart.Spec.VerificationKeyring == nil {
1070+
return nil, nil
1071+
}
1072+
name := types.NamespacedName{
1073+
Namespace: chart.GetNamespace(),
1074+
Name: chart.Spec.VerificationKeyring.SecretRef.Name,
1075+
}
1076+
var secret corev1.Secret
1077+
err := r.Client.Get(ctx, name, &secret)
1078+
if err != nil {
1079+
e := &serror.Event{
1080+
Err: fmt.Errorf("failed to get secret '%s': %w", chart.Spec.VerificationKeyring.SecretRef.Name, err),
1081+
Reason: sourcev1.AuthenticationFailedReason,
1082+
}
1083+
return nil, e
1084+
}
1085+
key := chart.Spec.VerificationKeyring.Key
1086+
if val, ok := secret.Data[key]; !ok {
1087+
err = fmt.Errorf("secret doesn't contain the advertised verification keyring name %s", key)
1088+
e := &serror.Event{
1089+
Err: fmt.Errorf("invalid secret '%s': %w", secret.GetName(), err),
1090+
Reason: sourcev1.AuthenticationFailedReason,
1091+
}
1092+
return nil, e
1093+
} else {
1094+
return val, nil
1095+
}
1096+
}

controllers/storage.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,7 @@ func (s *Storage) RemoveAll(artifact sourcev1.Artifact) (string, error) {
120120
func (s *Storage) RemoveAllButCurrent(artifact sourcev1.Artifact) ([]string, error) {
121121
deletedFiles := []string{}
122122
localPath := s.LocalPath(artifact)
123+
localProvPath := localPath + ".prov"
123124
dir := filepath.Dir(localPath)
124125
var errors []string
125126
_ = filepath.Walk(dir, func(path string, info os.FileInfo, err error) error {
@@ -128,7 +129,7 @@ func (s *Storage) RemoveAllButCurrent(artifact sourcev1.Artifact) ([]string, err
128129
return nil
129130
}
130131

131-
if path != localPath && !info.IsDir() && info.Mode()&os.ModeSymlink != os.ModeSymlink {
132+
if path != localPath && path != localProvPath && !info.IsDir() && info.Mode()&os.ModeSymlink != os.ModeSymlink {
132133
if err := os.Remove(path); err != nil {
133134
errors = append(errors, info.Name())
134135
} else {

internal/helm/chart/builder_local.go

+5-1
Original file line numberDiff line numberDiff line change
@@ -148,15 +148,19 @@ func (b *localChartBuilder) Build(ctx context.Context, ref Reference, p string,
148148

149149
// If the chart at the path is already packaged and no custom values files
150150
// options are set, we can copy the chart without making modifications
151-
provFilePath = provenanceFilePath(localRef.Path)
152151
if !requiresPackaging {
152+
provFilePath = provenanceFilePath(p)
153153
if err = copyFileToPath(localRef.Path, p); err != nil {
154154
return result, &BuildError{Reason: ErrChartPull, Err: err}
155155
}
156+
if err = copyFileToPath(provenanceFilePath(localRef.Path), provFilePath); err != nil {
157+
return result, &BuildError{Reason: ErrChartPull, Err: err}
158+
}
156159
if err = verifyProvFile(localRef.Path, provFilePath); err != nil {
157160
return result, err
158161
}
159162
result.Path = p
163+
result.ProvFilePath = provFilePath
160164
return result, nil
161165
}
162166

internal/helm/chart/builder_remote.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ func (b *remoteChartBuilder) Build(_ context.Context, ref Reference, p string, o
166166
// This is needed, since the verification will work only if the .tgz file is untampered.
167167
// But we write the packaged chart to disk under a different name, so the provenance file
168168
// will not be valid for this _new_ packaged chart.
169-
chart, err := util.WriteBytesToFile(chartBuf, fmt.Sprintf("%s-%s.tgz", result.Name, result.Version), false)
169+
chart, err := util.WriteBytesToFile(chartBuf, fmt.Sprintf("%s-%s.tgz", cv.Name, cv.Version), false)
170170
defer os.Remove(chart.Name())
171171
if err != nil {
172172
return nil, err

internal/helm/chart/verify.go

+5-5
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@ import (
1111
"helm.sh/helm/v3/pkg/provenance"
1212
)
1313

14+
// Ref: https://github.com/helm/helm/blob/v3.8.0/pkg/downloader/chart_downloader.go#L328
15+
// modified to accept a custom provenance file path and an actual keyring instead of a
16+
// path to the file containing the keyring.
1417
func VerifyProvenanceFile(keyring io.Reader, chartPath, provFilePath string) error {
1518
switch fi, err := os.Stat(chartPath); {
1619
case err != nil:
@@ -43,15 +46,12 @@ func VerifyProvenanceFile(keyring io.Reader, chartPath, provFilePath string) err
4346
}
4447

4548
// isTar tests whether the given file is a tar file.
46-
//
47-
// Currently, this simply checks extension, since a subsequent function will
48-
// untar the file and validate its binary format.
4949
func isTar(filename string) bool {
5050
return strings.EqualFold(filepath.Ext(filename), ".tgz")
5151
}
5252

53-
// Returns the path of a provenance file related to a packaged chart
54-
// Adds ".prov" at the end, as per the Helm convention.
53+
// Returns the path of a provenance file related to a packaged chart by
54+
// adding ".prov" at the end, as per the Helm convention.
5555
func provenanceFilePath(path string) string {
5656
return path + ".prov"
5757
}

0 commit comments

Comments
 (0)