Skip to content

Commit

Permalink
fix(provisioners): request correct signing type
Browse files Browse the repository at this point in the history
The requested certificate type was backwards, so requesting an RSA
signature resulted in an ECC signature from the Origin CA API and
vice-versa.

This changeset corrects the requested signature type, and adds tests to
verify the provisioner's behaviors.

Fixes #72
  • Loading branch information
terinjokes committed Nov 22, 2022
1 parent c093362 commit ca05b86
Show file tree
Hide file tree
Showing 5 changed files with 205 additions and 5 deletions.
2 changes: 2 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ require (
github.com/jetstack/cert-manager v1.7.2
github.com/rs/zerolog v1.25.0
github.com/spf13/pflag v1.0.5
gotest.tools/v3 v3.0.3
k8s.io/api v0.24.0
k8s.io/apimachinery v0.24.0
k8s.io/client-go v0.24.0
Expand Down Expand Up @@ -53,6 +54,7 @@ require (
golang.org/x/term v0.0.0-20210927222741-03fcf44c2211 // indirect
golang.org/x/text v0.3.7 // indirect
golang.org/x/time v0.0.0-20220210224613-90d013bbcef8 // indirect
golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 // indirect
gomodules.xyz/jsonpatch/v2 v2.2.0 // indirect
google.golang.org/appengine v1.6.7 // indirect
google.golang.org/protobuf v1.27.1 // indirect
Expand Down
1 change: 1 addition & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -939,6 +939,7 @@ gopkg.in/yaml.v3 v3.0.0-20200615113413-eeeca48fe776/go.mod h1:K4uyk7z7BCEPqu6E+C
gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b h1:h8qDotaEPuJATrMmW04NCwg7v22aHH28wwpauUhK9Oo=
gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
gotest.tools/v3 v3.0.2/go.mod h1:3SzNCllyD9/Y+b5r9JIKQ474KzkZyqLqEfYqMsX94Bk=
gotest.tools/v3 v3.0.3 h1:4AuOwCGf4lLR9u3YOe2awrHygurzhO/HeQ6laiA6Sx0=
gotest.tools/v3 v3.0.3/go.mod h1:Z7Lb0S5l+klDB31fvDQX8ss/FlKDxtlFlw3Oa8Ymbl8=
honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
honnef.co/go/tools v0.0.0-20190106161140-3f1c8253044a/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
Expand Down
2 changes: 1 addition & 1 deletion hack/derivation.nix
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ pkgs.buildGo117Module rec {

src = lib.sourceFilesBySuffices ../. [ ".go" ".mod" ".sum" ];

vendorSha256 = "1qr3mh1ws9ff21qs6pf7d4hf5v1aw33zailzi9r3r6dykndjkhwg";
vendorSha256 = "sha256-YZYR6e07kZFcGYTGYJG6ywJI2sMJBOQi8I3m3GSgIBM=";

subPackages = [ "cmd/controller" ];

Expand Down
13 changes: 9 additions & 4 deletions pkgs/provisioners/origin.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,19 @@ func CollectionWith(items []CollectionItem) *Collection {
// Provisioner allows for CertificateRequests to be signed using the stored
// Cloudflare API client.
type Provisioner struct {
client cfapi.Interface
client Signer
log logr.Logger

reqType v1.RequestType
}

// Signer implements the Origin CA signing API.
type Signer interface {
Sign(ctx context.Context, req *cfapi.SignRequest) (*cfapi.SignResponse, error)
}

// New returns a new provisioner.
func New(client cfapi.Interface, reqType v1.RequestType, log logr.Logger) (*Provisioner, error) {
func New(client Signer, reqType v1.RequestType, log logr.Logger) (*Provisioner, error) {
p := &Provisioner{
client: client,
log: log,
Expand Down Expand Up @@ -106,9 +111,9 @@ func (p *Provisioner) Sign(ctx context.Context, cr *certmanager.CertificateReque
var reqType string
switch p.reqType {
case v1.RequestTypeOriginECC:
reqType = "origin-rsa"
case v1.RequestTypeOriginRSA:
reqType = "origin-ecc"
case v1.RequestTypeOriginRSA:
reqType = "origin-rsa"
}

resp, err := p.client.Sign(ctx, &cfapi.SignRequest{
Expand Down
192 changes: 192 additions & 0 deletions pkgs/provisioners/origin_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,192 @@
package provisioners

import (
"context"
"crypto/x509"
"errors"
"testing"
"testing/quick"
"time"

"github.com/cloudflare/origin-ca-issuer/internal/cfapi"
v1 "github.com/cloudflare/origin-ca-issuer/pkgs/apis/v1"
"github.com/go-logr/logr"
"github.com/google/go-cmp/cmp/cmpopts"
certmanager "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1"
cmgen "github.com/jetstack/cert-manager/test/unit/gen"
"gotest.tools/v3/assert"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

func TestSign(t *testing.T) {
type testCase struct {
name string
reqType v1.RequestType
req *certmanager.CertificateRequest
signReq *cfapi.SignRequest
expected []byte
}

run := func(t *testing.T, tc testCase) {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()

signer := SignerFunc(func(ctx context.Context, req *cfapi.SignRequest) (*cfapi.SignResponse, error) {
assert.DeepEqual(t, req, tc.signReq, cmpopts.IgnoreFields(cfapi.SignRequest{}, "CSR"))
return &cfapi.SignResponse{
Certificate: "-----BEGIN CERTIFICATE-----\n-----END CERTIFICATE-----\n",
}, nil
})

provisioner, err := New(signer, tc.reqType, logr.Discard())
assert.NilError(t, err)

res, err := provisioner.Sign(ctx, tc.req)
assert.NilError(t, err)
assert.DeepEqual(t, res, tc.expected)
}

testCases := []testCase{
{
name: "origin rsa",
reqType: v1.RequestTypeOriginRSA,
req: cmgen.CertificateRequest("foobar",
cmgen.SetCertificateRequestNamespace("default"),
cmgen.SetCertificateRequestDuration(&metav1.Duration{Duration: 7 * 24 * time.Hour}),
cmgen.SetCertificateRequestCSR((func() []byte {
csr, _, err := cmgen.CSR(x509.RSA, cmgen.SetCSRDNSNames("example.com"))
assert.NilError(t, err)

return csr
})()),
),
signReq: &cfapi.SignRequest{
Hostnames: []string{"example.com"},
Validity: 7,
Type: "origin-rsa",
CSR: "",
},
expected: []byte("-----BEGIN CERTIFICATE-----\n-----END CERTIFICATE-----\n"),
},
{
name: "origin ecc",
reqType: v1.RequestTypeOriginECC,
req: cmgen.CertificateRequest("foobar",
cmgen.SetCertificateRequestNamespace("default"),
cmgen.SetCertificateRequestDuration(&metav1.Duration{Duration: 7 * 24 * time.Hour}),
cmgen.SetCertificateRequestCSR((func() []byte {
csr, _, err := cmgen.CSR(x509.ECDSA, cmgen.SetCSRDNSNames("example.com"))
assert.NilError(t, err)

return csr
})()),
),
signReq: &cfapi.SignRequest{
Hostnames: []string{"example.com"},
Validity: 7,
Type: "origin-ecc",
CSR: "",
},
expected: []byte("-----BEGIN CERTIFICATE-----\n-----END CERTIFICATE-----\n"),
},
{
name: "find closest duration",
reqType: v1.RequestTypeOriginECC,
req: cmgen.CertificateRequest("foobar",
cmgen.SetCertificateRequestNamespace("default"),
cmgen.SetCertificateRequestDuration(&metav1.Duration{Duration: 10 * 365 * 24 * time.Hour}),
cmgen.SetCertificateRequestCSR((func() []byte {
csr, _, err := cmgen.CSR(x509.ECDSA, cmgen.SetCSRDNSNames("example.com"))
assert.NilError(t, err)

return csr
})()),
),
signReq: &cfapi.SignRequest{
Hostnames: []string{"example.com"},
Validity: 5475,
Type: "origin-ecc",
CSR: "",
},
expected: []byte("-----BEGIN CERTIFICATE-----\n-----END CERTIFICATE-----\n"),
},
{
name: "default duration",
reqType: v1.RequestTypeOriginECC,
req: cmgen.CertificateRequest("foobar",
cmgen.SetCertificateRequestNamespace("default"),
cmgen.SetCertificateRequestCSR((func() []byte {
csr, _, err := cmgen.CSR(x509.ECDSA, cmgen.SetCSRDNSNames("example.com"))
assert.NilError(t, err)

return csr
})()),
),
signReq: &cfapi.SignRequest{
Hostnames: []string{"example.com"},
Validity: 7,
Type: "origin-ecc",
CSR: "",
},
expected: []byte("-----BEGIN CERTIFICATE-----\n-----END CERTIFICATE-----\n"),
},
}

for _, tc := range testCases {
tc := tc
t.Run(tc.name, func(t *testing.T) {
run(t, tc)
})
}
}

func TestSign_Error(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()

signer := SignerFunc(func(ctx context.Context, req *cfapi.SignRequest) (*cfapi.SignResponse, error) {
return nil, errors.New("cfapi error")
})

req := cmgen.CertificateRequest("foobar",
cmgen.SetCertificateRequestNamespace("default"),
cmgen.SetCertificateRequestCSR((func() []byte {
csr, _, err := cmgen.CSR(x509.ECDSA, cmgen.SetCSRDNSNames("example.com"))
assert.NilError(t, err)

return csr
})()),
)

provisioner, err := New(signer, v1.RequestTypeOriginECC, logr.Discard())
assert.NilError(t, err)

_, err = provisioner.Sign(ctx, req)
assert.Error(t, err, "unable to sign request: cfapi error")
}

func TestClosest(t *testing.T) {
index := func(x int, s []int) int {
for i, n := range s {
if x == n {
return i
}
}

return -1
}

f := func(x int) bool {
d := closest(x, allowedValidty)
return index(d, allowedValidty) >= 0
}

err := quick.Check(f, nil)
assert.NilError(t, err)
}

type SignerFunc func(ctx context.Context, req *cfapi.SignRequest) (*cfapi.SignResponse, error)

func (f SignerFunc) Sign(ctx context.Context, req *cfapi.SignRequest) (*cfapi.SignResponse, error) {
return f(ctx, req)
}

0 comments on commit ca05b86

Please sign in to comment.