From c46101499309fe0810f256a1f576497581c34064 Mon Sep 17 00:00:00 2001 From: Roberto Dip Date: Tue, 17 Sep 2024 12:03:58 -0300 Subject: [PATCH] Allow CA certificates with extendedKeyUsage attributes. for #22158 --- changes/22158-scep | 1 + server/mdm/crypto/scep.go | 26 ++++-- server/mdm/crypto/scep_test.go | 139 +++++++++++++++++++++++++++++++++ 3 files changed, 161 insertions(+), 5 deletions(-) create mode 100644 changes/22158-scep diff --git a/changes/22158-scep b/changes/22158-scep new file mode 100644 index 000000000000..ab7557468018 --- /dev/null +++ b/changes/22158-scep @@ -0,0 +1 @@ +* Allow custom SCEP CA certificates with any kind of extendedKeyUsage attributes. diff --git a/server/mdm/crypto/scep.go b/server/mdm/crypto/scep.go index 1367030bf4bf..2ba39d37d608 100644 --- a/server/mdm/crypto/scep.go +++ b/server/mdm/crypto/scep.go @@ -7,6 +7,7 @@ import ( "fmt" "github.com/fleetdm/fleet/v4/server/fleet" + "github.com/fleetdm/fleet/v4/server/mdm/assets" "github.com/fleetdm/fleet/v4/server/mdm/nanomdm/http/mdm" ) @@ -33,15 +34,21 @@ func (s *SCEPVerifier) Verify(cert *x509.Certificate) error { } // TODO(roberto): nano interfaces don't allow to pass a context to this function - assets, err := s.ds.GetAllMDMConfigAssetsByName(context.Background(), []fleet.MDMAssetName{ - fleet.MDMAssetCACert, - }) + rootCert, err := assets.X509Cert(context.Background(), s.ds, fleet.MDMAssetCACert) if err != nil { return fmt.Errorf("loading existing assets from the database: %w", err) } + opts.Roots.AddCert(rootCert) - if ok := opts.Roots.AppendCertsFromPEM(assets[fleet.MDMAssetCACert].Value); !ok { - return errors.New("unable to append cerver SCEP cert to pool verifier") + // the default SCEP cert issued by fleet doesn't have any extra key + // usages, however, customers might configure the server with any + // certificate they want (generally for touchless MDM migrations) + // + // given that go verifies ext key usages on the whole chain, we relax + // the constraints when the provided certificate has any ext key usage + // that would cause a failure. + if hasOtherKeyUsages(rootCert, x509.ExtKeyUsageClientAuth) { + opts.KeyUsages = []x509.ExtKeyUsage{x509.ExtKeyUsageAny} } if _, err := cert.Verify(opts); err != nil { @@ -50,3 +57,12 @@ func (s *SCEPVerifier) Verify(cert *x509.Certificate) error { return nil } + +func hasOtherKeyUsages(cert *x509.Certificate, usage x509.ExtKeyUsage) bool { + for _, u := range cert.ExtKeyUsage { + if u != usage { + return true + } + } + return false +} diff --git a/server/mdm/crypto/scep_test.go b/server/mdm/crypto/scep_test.go index a8865b58f9ad..179864b9d3ff 100644 --- a/server/mdm/crypto/scep_test.go +++ b/server/mdm/crypto/scep_test.go @@ -1,8 +1,20 @@ package mdmcrypto import ( + "context" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "errors" + "math/big" "testing" + "time" + "github.com/fleetdm/fleet/v4/server/fleet" + "github.com/fleetdm/fleet/v4/server/mock" "github.com/stretchr/testify/require" ) @@ -11,3 +23,130 @@ func TestSCEPVerifierVerifyEmptyCerts(t *testing.T) { err := v.Verify(nil) require.ErrorContains(t, err, "no certificate provided") } + +func TestVerify(t *testing.T) { + ds := new(mock.Store) + verifier := NewSCEPVerifier(ds) + + // generate a valid root certificate with ExtKeyUsageClientAuth + validRootCertBytes, validRootCert, rootKey := generateRootCertificate(t, []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth}) + _, validClientCert := generateClientCertificate(t, validRootCert, rootKey) + + // generate a root certificate with an unrelated ExtKeyUsage + rootWithOtherUsagesBytes, rootWithOtherUsageCert, rootWithOtherUsageKey := generateRootCertificate(t, []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}) + _, validClientCertFromMultipleUsageRoot := generateClientCertificate(t, rootWithOtherUsageCert, rootWithOtherUsageKey) + + cases := []struct { + name string + rootCert []byte + certToVerify *x509.Certificate + wantErr string + }{ + { + name: "no certificate provided", + rootCert: nil, + certToVerify: nil, + wantErr: "no certificate provided", + }, + { + name: "error loading root cert from database", + rootCert: nil, + certToVerify: validClientCert, + wantErr: "loading existing assets from the database", + }, + { + name: "valid certificate verification succeeds", + rootCert: validRootCertBytes, + certToVerify: validClientCert, + wantErr: "", + }, + { + name: "valid certificate with unrelated key usage in root cert", + rootCert: rootWithOtherUsagesBytes, + certToVerify: validClientCertFromMultipleUsageRoot, + wantErr: "", + }, + { + name: "mismatched certificate presented", + rootCert: rootWithOtherUsagesBytes, + certToVerify: validClientCert, + wantErr: "certificate signed by unknown authority", + }, + } + + for _, tt := range cases { + t.Run(tt.name, func(t *testing.T) { + ds.GetAllMDMConfigAssetsByNameFunc = func(ctx context.Context, assetNames []fleet.MDMAssetName) (map[fleet.MDMAssetName]fleet.MDMConfigAsset, error) { + if tt.rootCert == nil { + return nil, errors.New("test error") + } + + return map[fleet.MDMAssetName]fleet.MDMConfigAsset{ + fleet.MDMAssetCACert: {Value: tt.rootCert}, + }, nil + } + + err := verifier.Verify(tt.certToVerify) + if tt.wantErr == "" { + require.NoError(t, err) + } else { + require.ErrorContains(t, err, tt.wantErr) + } + }) + } +} + +func generateRootCertificate(t *testing.T, extKeyUsages []x509.ExtKeyUsage) ([]byte, *x509.Certificate, *ecdsa.PrivateKey) { + priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + + rootCertTemplate := &x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{ + Organization: []string{"Test Root CA"}, + }, + NotBefore: time.Now(), + NotAfter: time.Now().Add(10 * 365 * 24 * time.Hour), + IsCA: true, + KeyUsage: x509.KeyUsageCertSign | x509.KeyUsageDigitalSignature, + ExtKeyUsage: extKeyUsages, + BasicConstraintsValid: true, + } + + rootCertDER, err := x509.CreateCertificate(rand.Reader, rootCertTemplate, rootCertTemplate, &priv.PublicKey, priv) + require.NoError(t, err) + + rootCertPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: rootCertDER}) + + rootCert, err := x509.ParseCertificate(rootCertDER) + require.NoError(t, err) + + return rootCertPEM, rootCert, priv +} + +func generateClientCertificate(t *testing.T, rootCert *x509.Certificate, rootKey *ecdsa.PrivateKey) ([]byte, *x509.Certificate) { + clientPriv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + + clientCertTemplate := &x509.Certificate{ + SerialNumber: big.NewInt(2), + Subject: pkix.Name{ + Organization: []string{"Test Client"}, + }, + NotBefore: time.Now(), + NotAfter: time.Now().Add(1 * 365 * 24 * time.Hour), + KeyUsage: x509.KeyUsageDigitalSignature, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth}, + BasicConstraintsValid: true, + } + + clientCertDER, err := x509.CreateCertificate(rand.Reader, clientCertTemplate, rootCert, &clientPriv.PublicKey, rootKey) + require.NoError(t, err) + + clientCertPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: clientCertDER}) + + clientCert, err := x509.ParseCertificate(clientCertDER) + require.NoError(t, err) + + return clientCertPEM, clientCert +}