Skip to content

Commit

Permalink
Merge pull request #252 from securesign/fulcio-fix-generated-cert
Browse files Browse the repository at this point in the history
Fix generated Fulcio root certificate requirements
  • Loading branch information
openshift-merge-bot[bot] authored Mar 6, 2024
2 parents b6ca269 + 552c572 commit 06d1865
Show file tree
Hide file tree
Showing 10 changed files with 52 additions and 14 deletions.
1 change: 1 addition & 0 deletions api/v1alpha1/fulcio_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ type FulcioSpec struct {

// FulcioCert defines fields for system-generated certificate
// +kubebuilder:validation:XValidation:rule=(has(self.caRef) || self.commonName != ""),message=commonName cannot be empty
// +kubebuilder:validation:XValidation:rule=(has(self.caRef) || self.organizationName != ""),message=organizationName cannot be empty
// +kubebuilder:validation:XValidation:rule=(!has(self.caRef) || has(self.privateKeyRef)),message=privateKeyRef cannot be empty
type FulcioCert struct {
// Reference to CA private key
Expand Down
3 changes: 2 additions & 1 deletion api/v1alpha1/fulcio_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,8 @@ func generateFulcioObject(name string) *Fulcio {
},
},
Certificate: FulcioCert{
CommonName: "hostname",
CommonName: "hostname",
OrganizationName: "organization",
},
},
}
Expand Down
4 changes: 4 additions & 0 deletions config/crd/bases/rhtas.redhat.com_fulcios.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,8 @@ spec:
x-kubernetes-validations:
- message: commonName cannot be empty
rule: (has(self.caRef) || self.commonName != "")
- message: organizationName cannot be empty
rule: (has(self.caRef) || self.organizationName != "")
- message: privateKeyRef cannot be empty
rule: (!has(self.caRef) || has(self.privateKeyRef))
config:
Expand Down Expand Up @@ -314,6 +316,8 @@ spec:
x-kubernetes-validations:
- message: commonName cannot be empty
rule: (has(self.caRef) || self.commonName != "")
- message: organizationName cannot be empty
rule: (has(self.caRef) || self.organizationName != "")
- message: privateKeyRef cannot be empty
rule: (!has(self.caRef) || has(self.privateKeyRef))
conditions:
Expand Down
2 changes: 2 additions & 0 deletions config/crd/bases/rhtas.redhat.com_securesigns.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,8 @@ spec:
x-kubernetes-validations:
- message: commonName cannot be empty
rule: (has(self.caRef) || self.commonName != "")
- message: organizationName cannot be empty
rule: (has(self.caRef) || self.organizationName != "")
- message: privateKeyRef cannot be empty
rule: (!has(self.caRef) || has(self.privateKeyRef))
config:
Expand Down
5 changes: 4 additions & 1 deletion controllers/ctlog/actions/handle_fulcio_root.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package actions

import (
"context"
k8sErrors "k8s.io/apimachinery/pkg/api/errors"
"slices"

"github.com/securesign/operator/api/v1alpha1"
Expand Down Expand Up @@ -92,7 +93,9 @@ func (g handleFulcioCert) Handle(ctx context.Context, instance *v1alpha1.CTlog)
Namespace: instance.Namespace,
},
}); err != nil {
return g.Failed(err)
if !k8sErrors.IsNotFound(err) {
return g.Failed(err)
}
}
instance.Status.ServerConfigRef = nil
}
Expand Down
6 changes: 4 additions & 2 deletions controllers/ctlog/actions/handle_keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@ package actions
import (
"context"
"fmt"

"github.com/securesign/operator/api/v1alpha1"
"github.com/securesign/operator/controllers/common/action"
k8sutils "github.com/securesign/operator/controllers/common/utils/kubernetes"
"github.com/securesign/operator/controllers/constants"
"github.com/securesign/operator/controllers/ctlog/utils"
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/equality"
k8sErrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -168,7 +168,9 @@ func (g handleKeys) Handle(ctx context.Context, instance *v1alpha1.CTlog) *actio
Namespace: instance.Namespace,
},
}); err != nil {
return g.Failed(err)
if !k8sErrors.IsNotFound(err) {
return g.Failed(err)
}
}
instance.Status.ServerConfigRef = nil
}
Expand Down
2 changes: 1 addition & 1 deletion controllers/fulcio/actions/generate_cert.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ func (g handleCert) setupCert(instance *v1alpha1.Fulcio) (*utils.FulcioCertConfi
}
config.PrivateKey = key
} else {
key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
key, err := ecdsa.GenerateKey(elliptic.P384(), rand.Reader)
if err != nil {
return nil, err
}
Expand Down
37 changes: 31 additions & 6 deletions controllers/fulcio/utils/fulcio_secrets.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"crypto/x509"
"crypto/x509/pkix"
"encoding/pem"
"errors"
"fmt"
"math/big"
"time"
Expand Down Expand Up @@ -81,8 +82,8 @@ func CreateCAPub(key crypto.PublicKey) ([]byte, error) {
func CreateFulcioCA(config *FulcioCertConfig, instance *rhtasv1alpha1.Fulcio) ([]byte, error) {
var err error

if instance.Spec.Certificate.CommonName == "" || instance.Spec.Certificate.OrganizationEmail == "" || instance.Spec.Certificate.OrganizationName == "" {
return nil, fmt.Errorf("could not create certificate: missing OrganizationName, OrganizationEmail or CommonName from config")
if instance.Spec.Certificate.CommonName == "" || instance.Spec.Certificate.OrganizationName == "" {
return nil, fmt.Errorf("could not create certificate: missing OrganizationName or CommonName from config")
}

block, _ := pem.Decode(config.PrivateKey)
Expand All @@ -107,14 +108,25 @@ func CreateFulcioCA(config *FulcioCertConfig, instance *rhtasv1alpha1.Fulcio) ([
Organization: []string{instance.Spec.Certificate.OrganizationName},
}

serialNumber, err := GenerateSerialNumber()
if err != nil {
return nil, err
}

emailAddresses := make([]string, 0)

if instance.Spec.Certificate.OrganizationEmail != "" {
emailAddresses = append(emailAddresses, instance.Spec.Certificate.OrganizationEmail)
}

template := x509.Certificate{
SerialNumber: big.NewInt(0),
SerialNumber: serialNumber,
Subject: issuer,
EmailAddresses: []string{instance.Spec.Certificate.OrganizationEmail},
SignatureAlgorithm: x509.ECDSAWithSHA256,
EmailAddresses: emailAddresses,
SignatureAlgorithm: x509.ECDSAWithSHA384,
BasicConstraintsValid: true,
IsCA: true,
KeyUsage: x509.KeyUsageCertSign,
KeyUsage: x509.KeyUsageCertSign | x509.KeyUsageCRLSign,
Issuer: issuer,
NotBefore: notBefore,
NotAfter: notAfter,
Expand All @@ -136,3 +148,16 @@ func CreateFulcioCA(config *FulcioCertConfig, instance *rhtasv1alpha1.Fulcio) ([

return pemFulcioRoot.Bytes(), nil
}

// GenerateSerialNumber creates a compliant serial number as per RFC 5280 4.1.2.2.
// Serial numbers must be positive, and can be no longer than 20 bytes.
// The serial number is generated with 159 bits, so that the first bit will always
// be 0, resulting in a positive serial number.
func GenerateSerialNumber() (*big.Int, error) {
// Pick a random number from 0 to 2^159.
serial, err := rand.Int(rand.Reader, (&big.Int{}).Exp(big.NewInt(2), big.NewInt(159), nil))
if err != nil {
return nil, errors.New("error generating serial number")
}
return serial, nil
}
2 changes: 1 addition & 1 deletion controllers/rekor/actions/server/generate_signer.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ func (g generateSigner) CreateRekorKey(instance *v1alpha1.Rekor) (*RekorCertConf
return config, nil
}

key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
key, err := ecdsa.GenerateKey(elliptic.P384(), rand.Reader)
if err != nil {
return nil, err
}
Expand Down
4 changes: 2 additions & 2 deletions e2e/provided_certs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -355,14 +355,14 @@ func initCertificates(passwordProtected bool) ([]byte, []byte, []byte, error) {
}
//Create certificate templet
template := x509.Certificate{
SerialNumber: big.NewInt(0),
SerialNumber: big.NewInt(1),
Subject: issuer,
SignatureAlgorithm: x509.ECDSAWithSHA256,
NotBefore: notBefore,
NotAfter: notAfter,
BasicConstraintsValid: true,
IsCA: true,
KeyUsage: x509.KeyUsageCertSign,
KeyUsage: x509.KeyUsageCertSign | x509.KeyUsageCRLSign,
Issuer: issuer,
}
//Create certificate using templet
Expand Down

0 comments on commit 06d1865

Please sign in to comment.