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

feat: keyless #1659

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open
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
4 changes: 2 additions & 2 deletions .github/workflows/.reusable-unit-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ jobs:
- name: Setup
uses: actions/setup-go@cdcb36043654635271a94b9a6d1392de5bb323a7 # v5.0.1
with:
go-version: '1.21'
go-version: '1.22'
- name: Test
run: go test ./... -race -coverprofile=coverage.out -covermode=atomic
run: go test ./internal/... -race -coverprofile=coverage.out -covermode=atomic
- name: Upload
uses: codecov/codecov-action@eaaf4bedf32dbdc6b720b63067d99c4d77d6047d # v3.1.4
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ annihilate:
kubectl delete ns $(NAMESPACE)

test:
go test ./... -race -coverprofile=cover.out -covermode=atomic; go tool cover -func cover.out
go test ./internal/... -race -coverprofile=cover.out -covermode=atomic; go tool cover -func cover.out

upgrade:
helm upgrade connaisseur charts/connaisseur --namespace $(NAMESPACE) --wait
Expand Down
4 changes: 2 additions & 2 deletions charts/connaisseur/Chart.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ apiVersion: v2
name: connaisseur
description: Helm chart for Connaisseur - a Kubernetes admission controller to integrate container image signature verification and trust pinning into a cluster.
type: application
version: 2.5.1
appVersion: 3.5.1
version: 2.6.0
appVersion: 3.6.0
keywords:
- container image
- signature
Expand Down
12 changes: 10 additions & 2 deletions charts/connaisseur/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ kubernetes:
type: RuntimeDefault # remove when using Kubernetes prior v1.19, openshift or OKD 4
podSecurityContext: {}
tls: {} # set to use custom tls certificates for redis
# -----------------------------------------------------
# -----------------------------------------------------

# additional labels for connaisseur resources
# (except for pods – see kubernetes.deployment.podLabels for that)
Expand Down Expand Up @@ -125,6 +125,13 @@ application:
MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEsx28WV7BsQfnHF1kZmpdCTTLJaWe
d0CA+JOi8H4REuBaWSZ5zPDe468WuOJ6f71E7WFg3CVEVYHuoZt2UYbN/Q==
-----END PUBLIC KEY-----
- name: keyless
Copy link
Member

Choose a reason for hiding this comment

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

Commented out, I can see this as an example, but it shouldn't be part of the default config

type: cosign
trustRoots:
- name: default
keyless:
issuer: github
subject: [email protected]

# policy options: https://sse-secure-systems.github.io/connaisseur/latest/basics/#configuration-options_2
# explanation of policy patterns: https://sse-secure-systems.github.io/connaisseur/latest/basics/#image-policy
Expand All @@ -139,6 +146,8 @@ application:
trustRoot: sse
- pattern: "registry.k8s.io/*:*"
validator: allow
- pattern: "phbelitz/*:*"
validator: keyless

# feature descriptions: https://sse-secure-systems.github.io/connaisseur/latest/features/
features:
Expand All @@ -158,7 +167,6 @@ application:
cacheErrors: true

logLevel: info

# alerting options: https://sse-secure-systems.github.io/connaisseur/latest/features/alerting/#configuration-options
# alerting:
# clusterIdentifier: example-cluster-staging-europe # defaults to "not specified"
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ require (
github.com/prometheus/client_model v0.6.1
github.com/redis/go-redis/v9 v9.5.3
github.com/sigstore/cosign/v2 v2.2.4
github.com/sigstore/rekor v1.3.6
github.com/sigstore/sigstore v1.8.4
github.com/sigstore/sigstore/pkg/signature/kms/aws v1.8.4
github.com/sigstore/sigstore/pkg/signature/kms/azure v1.8.4
Expand Down Expand Up @@ -208,7 +209,6 @@ require (
github.com/segmentio/ksuid v1.0.4 // indirect
github.com/shibumi/go-pathspec v1.3.0 // indirect
github.com/sigstore/fulcio v1.4.5 // indirect
github.com/sigstore/rekor v1.3.6 // indirect
github.com/sigstore/timestamp-authority v1.2.2 // indirect
github.com/skratchdot/open-golang v0.0.0-20200116055534-eef842397966 // indirect
github.com/sourcegraph/conc v0.3.0 // indirect
Expand Down
2 changes: 1 addition & 1 deletion internal/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -478,7 +478,7 @@ func TestValidateErrors(t *testing.T) {
},
},
},
"Key must be set if Cert isn't",
"Key is a required field",
Copy link
Member

Choose a reason for hiding this comment

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

That's not true, though?

},
}
for idx, tc := range testCases {
Expand Down
2 changes: 2 additions & 0 deletions internal/policy/rule.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ type RuleOptions struct {
TrustRoot string `yaml:"trustRoot"`
// flag to indicate whether to verify the image against a transparency log (cosign)
VerifyTLog *bool `yaml:"verifyInTransparencyLog"` // pointer to distinguish false and unset
// flag to indicate whether to check for signed certificate timestamps in transparency log (cosign)
VerifySCT *bool `yaml:"verifySCT"` // pointer to distinguish false and unset
// threshold of cosign signatures to require
Threshold int `yaml:"threshold" validate:"gte=0"`
// list of trust roots whose signatures need to
Expand Down
9 changes: 9 additions & 0 deletions internal/utils/validation.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package utils

import (
"crypto/x509"
"fmt"
"reflect"

Expand Down Expand Up @@ -53,3 +54,11 @@ func Validate(s interface{}) error {

return fmt.Errorf("%s has %d errors:\n%s", ty, len(err.(validation.ValidationErrors)), msg)
}

func ValidateCertificate(cert string) error {
certPool := x509.NewCertPool()
if !certPool.AppendCertsFromPEM([]byte(cert)) {
Copy link
Member

Choose a reason for hiding this comment

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

That's not really what this function is supposed to check.

// AppendCertsFromPEM attempts to parse a series of PEM encoded certificates.
// It appends any certificates found to s and reports whether any certificates
// were successfully parsed.

If there is a single valid certificate inside the bytestring and then a bunch of garbage, this succeeds

Also a nit: the return value is never used in its form and could be a bool with as IsValidCert or similar

Suggested change
if !certPool.AppendCertsFromPEM([]byte(cert)) {
return certPool.AppendCertsFromPEM([]byte(cert))

return fmt.Errorf("invalid certificate")
}
return nil
}
13 changes: 11 additions & 2 deletions internal/validator/auth/trust_root.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,18 @@ type TrustRoot struct {
Name string `yaml:"name" validate:"required"`
// public key of the trust root. either
// inline key or kms reference
Key string `yaml:"key" validate:"required_without=Cert,excluded_with=Cert"`
Key string `yaml:"key" validate:"required_without_all=Cert Keyless,excluded_with_all=Cert Keyless"`
// certificate of the trust root (notaryv2)
Cert string `yaml:"cert" validate:"required_without=Key,excluded_with=Key"`
Cert string `yaml:"cert" validate:"required_without_all=Key Keyless,excluded_with_all=Key Keyless"`
// keyless configuration
Keyless Keyless `yaml:"keyless" validate:"required_without_all=Key Cert,excluded_with_all=Key Cert"`
}

type Keyless struct {
// issuer of the trust root (e.g. a mail address)
Issuer string `yaml:"issuer"`
// provider of the trust root (e.g. an oidc provider)
Subject string `yaml:"subject"`
phbelitz marked this conversation as resolved.
Show resolved Hide resolved
}

// Returns the trust roots for the given key references matching
Expand Down
40 changes: 23 additions & 17 deletions internal/validator/auth/trust_root_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@ import (
)

func TestGetTrustRoots(t *testing.T) {
emptyKeyless := Keyless{}
a := TrustRoot{"a", "a", "", emptyKeyless}
b := TrustRoot{"b", "b", "", emptyKeyless}
c := TrustRoot{"c", "c", "", emptyKeyless}
defaultRoot := TrustRoot{"default", "default", "", emptyKeyless}

var testCases = []struct {
keyRefs []string
trustRoots []TrustRoot
Expand All @@ -16,70 +22,70 @@ func TestGetTrustRoots(t *testing.T) {
}{
{
[]string{"a"},
[]TrustRoot{{"a", "a", ""}, {"b", "b", ""}},
[]TrustRoot{a, b},
false,
[]TrustRoot{{"a", "a", ""}},
[]TrustRoot{a},
"",
},
{
[]string{"a", "c"},
[]TrustRoot{{"a", "a", ""}, {"b", "b", ""}, {"c", "c", ""}},
[]TrustRoot{a, b, c},
false,
[]TrustRoot{{"a", "a", ""}, {"c", "c", ""}},
[]TrustRoot{a, c},
"",
},
{
[]string{"*"},
[]TrustRoot{{"a", "a", ""}, {"b", "b", ""}, {"c", "c", ""}},
[]TrustRoot{a, b, c},
false,
[]TrustRoot{{"a", "a", ""}, {"b", "b", ""}, {"c", "c", ""}},
[]TrustRoot{a, b, c},
"",
},
{
[]string{"a"},
[]TrustRoot{{"a", "a", ""}, {"default", "default", ""}},
[]TrustRoot{a, defaultRoot},
true,
[]TrustRoot{{"a", "a", ""}},
[]TrustRoot{a},
"",
},
{
[]string{},
[]TrustRoot{{"a", "a", ""}, {"default", "default", ""}},
[]TrustRoot{a, defaultRoot},
true,
[]TrustRoot{{"default", "default", ""}},
[]TrustRoot{defaultRoot},
"",
},
{
nil,
[]TrustRoot{{"a", "a", ""}, {"default", "default", ""}},
[]TrustRoot{a, defaultRoot},
true,
[]TrustRoot{{"default", "default", ""}},
[]TrustRoot{defaultRoot},
"",
},
{
[]string{"c"},
[]TrustRoot{{"a", "a", ""}, {"default", "default", ""}},
[]TrustRoot{a, defaultRoot},
true,
nil,
"unable to find trust root c",
},
{
[]string{},
[]TrustRoot{{"a", "a", ""}, {"b", "b", ""}},
[]TrustRoot{a, b},
false,
nil,
"no trust roots defined for key references",
},
{
[]string{""},
[]TrustRoot{{"a", "a", ""}, {"default", "default", ""}},
[]TrustRoot{a, defaultRoot},
true,
[]TrustRoot{{"default", "default", ""}},
[]TrustRoot{defaultRoot},
"",
},
{
[]string{""},
[]TrustRoot{{"a", "a", ""}, {"b", "b", ""}},
[]TrustRoot{a, b},
false,
nil,
"unable to find trust root",
Expand Down
Loading
Loading