From 89a43705154416e5ceacd9d5c8870c5fbac30652 Mon Sep 17 00:00:00 2001 From: Daniel Adam Date: Sat, 2 Nov 2024 09:42:24 +0100 Subject: [PATCH] fixup! Verification by CRL --- certificate-authority/config.yaml | 1 - .../service/grpc/config_test.go | 4 +- certificate-authority/service/grpc/service.go | 5 +- certificate-authority/service/grpc/signer.go | 6 +- certificate-authority/service/service.go | 64 ++++++++- certificate-authority/service/service_test.go | 136 ++++++++++++++++++ coap-gateway/service/auth.go | 8 +- coap-gateway/service/auth_test.go | 2 - coap-gateway/service/service.go | 2 +- grpc-gateway/service/service.go | 2 +- identity-store/service/service.go | 2 +- m2m-oauth-server/service/grpc/service.go | 2 +- pkg/net/grpc/server/server.go | 6 +- .../certManager/client/certManager.go | 8 +- .../certManager/general/certManager.go | 74 ++++++---- .../certManager/general/clientCertManager.go | 8 +- pkg/security/certManager/general/options.go | 15 ++ .../certManager/server/certManager.go | 6 +- pkg/security/jwt/validator/validator.go | 19 ++- pkg/security/x509/verify.go | 39 ++++- resource-aggregate/service/service.go | 2 +- resource-directory/service/service.go | 2 +- snippet-service/service/grpc/service.go | 2 +- tools/grpc-reflection/service/service.go | 2 +- 24 files changed, 342 insertions(+), 75 deletions(-) create mode 100644 certificate-authority/service/service_test.go create mode 100644 pkg/security/certManager/general/options.go diff --git a/certificate-authority/config.yaml b/certificate-authority/config.yaml index 15f6ebfe4..6cff2ac34 100644 --- a/certificate-authority/config.yaml +++ b/certificate-authority/config.yaml @@ -29,7 +29,6 @@ apis: keyFile: "/secrets/private/cert.key" certFile: "/secrets/private/cert.crt" clientCertificateRequired: true - authorization: ownerClaim: "sub" audience: "" diff --git a/certificate-authority/service/grpc/config_test.go b/certificate-authority/service/grpc/config_test.go index 560a62434..4dfe2589b 100644 --- a/certificate-authority/service/grpc/config_test.go +++ b/certificate-authority/service/grpc/config_test.go @@ -29,10 +29,10 @@ func TestCRLConfigValidate(t *testing.T) { }, }, { - name: "Enabled CRLConfig with ExpiresIn less than 1 minute", + name: "Enabled CRLConfig with ExpiresIn less than 10 seconds", input: grpc.CRLConfig{ Enabled: true, - ExpiresIn: 30 * time.Second, + ExpiresIn: 9 * time.Second, }, wantErr: true, }, diff --git a/certificate-authority/service/grpc/service.go b/certificate-authority/service/grpc/service.go index d28f36b2f..452a9cfc2 100644 --- a/certificate-authority/service/grpc/service.go +++ b/certificate-authority/service/grpc/service.go @@ -8,6 +8,7 @@ import ( "github.com/plgd-dev/hub/v2/pkg/log" "github.com/plgd-dev/hub/v2/pkg/net/grpc/server" "github.com/plgd-dev/hub/v2/pkg/security/jwt/validator" + pkgX509 "github.com/plgd-dev/hub/v2/pkg/security/x509" "go.opentelemetry.io/otel/trace" ) @@ -15,12 +16,12 @@ type Service struct { *server.Server } -func New(config Config, clientApplicationServer *CertificateAuthorityServer, validator *validator.Validator, fileWatcher *fsnotify.Watcher, logger log.Logger, tracerProvider trace.TracerProvider) (*Service, error) { +func New(config Config, clientApplicationServer *CertificateAuthorityServer, validator *validator.Validator, fileWatcher *fsnotify.Watcher, logger log.Logger, tracerProvider trace.TracerProvider, customDistributionPointCRLVerification pkgX509.CustomDistributionPointVerification) (*Service, error) { opts, err := server.MakeDefaultOptions(server.NewAuth(validator), logger, tracerProvider) if err != nil { return nil, fmt.Errorf("cannot create grpc server options: %w", err) } - server, err := server.New(config.BaseConfig, fileWatcher, logger, tracerProvider, opts...) + server, err := server.New(config.BaseConfig, fileWatcher, logger, tracerProvider, customDistributionPointCRLVerification, opts...) if err != nil { return nil, err } diff --git a/certificate-authority/service/grpc/signer.go b/certificate-authority/service/grpc/signer.go index f77168391..724d48f9d 100644 --- a/certificate-authority/service/grpc/signer.go +++ b/certificate-authority/service/grpc/signer.go @@ -5,7 +5,7 @@ import ( "crypto/ecdsa" "crypto/x509" "errors" - "strings" + "path" "time" "github.com/google/uuid" @@ -181,8 +181,8 @@ func (s *Signer) sign(ctx context.Context, isIdentityCertificate bool, csr []byt }), } if s.IsCRLEnabled() { - dp := []string{s.crl.serverAddress, uri.SigningRevocationListBase, s.issuerID} - opts = append(opts, certificateSigner.WithCRLDistributionPoints([]string{strings.Join(dp, "/")})) + dp := s.crl.serverAddress + path.Join(uri.SigningRevocationListBase, s.issuerID) + opts = append(opts, certificateSigner.WithCRLDistributionPoints([]string{dp})) } signer, err := s.newCertificateSigner(isIdentityCertificate, opts...) if err != nil { diff --git a/certificate-authority/service/service.go b/certificate-authority/service/service.go index 8e54df5ee..c4ea99ee1 100644 --- a/certificate-authority/service/service.go +++ b/certificate-authority/service/service.go @@ -2,13 +2,17 @@ package service import ( "context" + "crypto/x509" "errors" "fmt" + "math/big" + "strings" "time" "github.com/go-co-op/gocron/v2" grpcService "github.com/plgd-dev/hub/v2/certificate-authority/service/grpc" httpService "github.com/plgd-dev/hub/v2/certificate-authority/service/http" + "github.com/plgd-dev/hub/v2/certificate-authority/service/uri" "github.com/plgd-dev/hub/v2/certificate-authority/store" storeConfig "github.com/plgd-dev/hub/v2/certificate-authority/store/config" "github.com/plgd-dev/hub/v2/certificate-authority/store/cqldb" @@ -17,9 +21,11 @@ import ( "github.com/plgd-dev/hub/v2/pkg/fn" "github.com/plgd-dev/hub/v2/pkg/fsnotify" "github.com/plgd-dev/hub/v2/pkg/log" + pkgHttpUri "github.com/plgd-dev/hub/v2/pkg/net/http/uri" "github.com/plgd-dev/hub/v2/pkg/net/listener" otelClient "github.com/plgd-dev/hub/v2/pkg/opentelemetry/collector/client" "github.com/plgd-dev/hub/v2/pkg/security/jwt/validator" + pkgX509 "github.com/plgd-dev/hub/v2/pkg/security/x509" "github.com/plgd-dev/hub/v2/pkg/service" "go.opentelemetry.io/otel/trace" ) @@ -84,6 +90,45 @@ func newStore(ctx context.Context, config StorageConfig, fileWatcher *fsnotify.W return db, fl.ToFunction(), nil } +func getVerifyCertificateByCRLFromStorage(s store.Store) func(ctx context.Context, certificate *x509.Certificate, endpoint string) (bool, error) { + return func(ctx context.Context, certificate *x509.Certificate, endpoint string) (bool, error) { + // get issuer from endpoint /certificate-authority/api/v1/signing/crl/{$issuerID} + prefix := uri.SigningRevocationListBase + "/" + var issuerID string + if strings.HasPrefix(endpoint, prefix) { + issuerID = strings.TrimPrefix(endpoint, prefix) + // ignore everything after next "/" + if len(issuerID) > 36 && issuerID[36] == '/' { + issuerID = issuerID[:36] + } + } + // uuid string is 36 chars long + if len(issuerID) != 36 { + return false, fmt.Errorf("invalid issuerID(%s)", issuerID) + } + rl, err := s.GetRevocationList(ctx, issuerID, false) + if err != nil { + if errors.Is(err, store.ErrNotFound) { + return false, nil + } + return false, err + } + + for _, revoked := range rl.Certificates { + serial := &big.Int{} + serial, ok := serial.SetString(revoked.Serial, 10) + if !ok { + continue + } + + if certificate.SerialNumber.Cmp(serial) == 0 { + return true, nil + } + } + return false, nil + } +} + func New(ctx context.Context, config Config, fileWatcher *fsnotify.Watcher, logger log.Logger) (*service.Service, error) { otelClient, err := otelClient.New(ctx, config.Clients.OpenTelemetryCollector, "certificate-authority", fileWatcher, logger) if err != nil { @@ -100,13 +145,22 @@ func New(ctx context.Context, config Config, fileWatcher *fsnotify.Watcher, logg } closerFn.AddFunc(closeStore) - ca, err := grpcService.NewCertificateAuthorityServer(config.APIs.GRPC.Authorization.OwnerClaim, config.HubID, config.APIs.HTTP.ExternalAddress, config.Signer, dbStorage, fileWatcher, logger) + externalAddress := pkgHttpUri.CanonicalURI(config.APIs.HTTP.ExternalAddress) + ca, err := grpcService.NewCertificateAuthorityServer(config.APIs.GRPC.Authorization.OwnerClaim, config.HubID, externalAddress, config.Signer, dbStorage, fileWatcher, logger) if err != nil { closerFn.Execute() return nil, fmt.Errorf("cannot create grpc certificate authority server: %w", err) } closerFn.AddFunc(ca.Close) - httpValidator, err := validator.New(ctx, config.APIs.GRPC.Authorization.Config, fileWatcher, logger, tracerProvider) + + var customDistributionPointCRLVerification pkgX509.CustomDistributionPointVerification + crlEnabled := externalAddress != "" && dbStorage.SupportsRevocationList() + if crlEnabled { + customDistributionPointCRLVerification = pkgX509.CustomDistributionPointVerification{ + externalAddress: getVerifyCertificateByCRLFromStorage(dbStorage), + } + } + httpValidator, err := validator.New(ctx, config.APIs.GRPC.Authorization.Config, fileWatcher, logger, tracerProvider, validator.WithCustomDistributionPointVerification(customDistributionPointCRLVerification)) if err != nil { closerFn.Execute() return nil, fmt.Errorf("cannot create http validator: %w", err) @@ -119,19 +173,19 @@ func New(ctx context.Context, config Config, fileWatcher *fsnotify.Watcher, logg }, Authorization: config.APIs.GRPC.Authorization.Config, Server: config.APIs.HTTP.Server, - CRLEnabled: config.Signer.CRL.Enabled, + CRLEnabled: crlEnabled, }, dbStorage, ca, httpValidator, fileWatcher, logger, tracerProvider) if err != nil { closerFn.Execute() return nil, fmt.Errorf("cannot create http service: %w", err) } - grpcValidator, err := validator.New(ctx, config.APIs.GRPC.Authorization.Config, fileWatcher, logger, tracerProvider) + grpcValidator, err := validator.New(ctx, config.APIs.GRPC.Authorization.Config, fileWatcher, logger, tracerProvider, validator.WithCustomDistributionPointVerification(customDistributionPointCRLVerification)) if err != nil { closerFn.Execute() _ = httpService.Close() return nil, fmt.Errorf("cannot create grpc validator: %w", err) } - grpcService, err := grpcService.New(config.APIs.GRPC, ca, grpcValidator, fileWatcher, logger, tracerProvider) + grpcService, err := grpcService.New(config.APIs.GRPC, ca, grpcValidator, fileWatcher, logger, tracerProvider, customDistributionPointCRLVerification) if err != nil { closerFn.Execute() _ = httpService.Close() diff --git a/certificate-authority/service/service_test.go b/certificate-authority/service/service_test.go new file mode 100644 index 000000000..646b8b162 --- /dev/null +++ b/certificate-authority/service/service_test.go @@ -0,0 +1,136 @@ +package service_test + +import ( + "context" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/tls" + "crypto/x509" + "encoding/pem" + "testing" + "time" + + "github.com/plgd-dev/device/v2/pkg/security/generateCertificate" + pbCA "github.com/plgd-dev/hub/v2/certificate-authority/pb" + caTest "github.com/plgd-dev/hub/v2/certificate-authority/test" + "github.com/plgd-dev/hub/v2/pkg/config/database" + pkgGrpc "github.com/plgd-dev/hub/v2/pkg/net/grpc" + "github.com/plgd-dev/hub/v2/test" + "github.com/plgd-dev/hub/v2/test/config" + oauthTest "github.com/plgd-dev/hub/v2/test/oauth-server/test" + testService "github.com/plgd-dev/hub/v2/test/service" + "github.com/stretchr/testify/require" + "google.golang.org/grpc" + "google.golang.org/grpc/credentials" +) + +func getSigningRecords(ctx context.Context, t *testing.T, addr string, certificates []tls.Certificate) error { + conn, err := grpc.NewClient(addr, grpc.WithTransportCredentials( + credentials.NewTLS(&tls.Config{ + RootCAs: test.GetRootCertificatePool(t), + Certificates: certificates, + })), + ) + require.NoError(t, err) + caClient := pbCA.NewCertificateAuthorityClient(conn) + _, err = caClient.GetSigningRecords(ctx, &pbCA.GetSigningRecordsRequest{}) + return err +} + +func getNewCertificate(ctx context.Context, t *testing.T, addr string, pk *ecdsa.PrivateKey, certificates []tls.Certificate) ([]byte, error) { + conn, err := grpc.NewClient(addr, grpc.WithTransportCredentials( + credentials.NewTLS(&tls.Config{ + RootCAs: test.GetRootCertificatePool(t), + Certificates: certificates, + })), + ) + require.NoError(t, err) + caClient := pbCA.NewCertificateAuthorityClient(conn) + + var cfg generateCertificate.Configuration + cfg.Subject.CommonName = "test" + csr, err := generateCertificate.GenerateCSR(cfg, pk) + require.NoError(t, err) + + resp, err := caClient.SignCertificate(ctx, &pbCA.SignCertificateRequest{CertificateSigningRequest: csr}) + if err != nil { + return nil, err + } + return resp.GetCertificate(), nil +} + +func TestGetSigningRecords(t *testing.T) { + if config.ACTIVE_DATABASE() == database.CqlDB { + t.Skip("revocation list not supported for CqlDB") + } + + ctx, cancel := context.WithTimeout(context.Background(), time.Second*30) + defer cancel() + shutdown := testService.SetUpServices(ctx, t, testService.SetUpServicesOAuth|testService.SetUpServicesMachine2MachineOAuth) + defer shutdown() + + // start insecure ca + caCfg := caTest.MakeConfig(t) + // CRL list should be valid for 10 sec after it is issued + caCfg.Signer.CRL.Enabled = true + caCfg.Signer.CRL.ExpiresIn = time.Hour + err := caCfg.Validate() + require.NoError(t, err) + caShutdown := caTest.New(t, caCfg) + defer caShutdown() + + pk, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + // get certificate - insecure + ctx = pkgGrpc.CtxWithToken(ctx, oauthTest.GetDefaultAccessToken(t)) + certData1, err := getNewCertificate(ctx, t, config.CERTIFICATE_AUTHORITY_HOST, pk, nil) + require.NoError(t, err) + caShutdown() + b, err := x509.MarshalECPrivateKey(pk) + require.NoError(t, err) + key := pem.EncodeToMemory(&pem.Block{Type: "EC PRIVATE KEY", Bytes: b}) + crt1, err := tls.X509KeyPair(certData1, key) + require.NoError(t, err) + + // start secure ca + caCfg.APIs.GRPC.TLS.ClientCertificateRequired = true + caCfg.APIs.GRPC.TLS.CRL.Enabled = true + httpClientConfig := config.MakeHttpClientConfig() + caCfg.APIs.GRPC.TLS.CRL.HTTP = &httpClientConfig + caCfg.Signer.CRL.Enabled = false // generate cert without distribution point + err = caCfg.Validate() + require.NoError(t, err) + caShutdown = caTest.New(t, caCfg) + defer caShutdown() + + // second ca on different port + const ca_addr = "localhost:30011" + caCfg2 := caTest.MakeConfig(t) + caCfg2.APIs.GRPC = config.MakeGrpcServerConfig(ca_addr) + caCfg2.APIs.GRPC.TLS.ClientCertificateRequired = true + caCfg2.APIs.GRPC.TLS.CRL.Enabled = true + caCfg2.APIs.GRPC.TLS.CRL.HTTP = &httpClientConfig + caCfg2.APIs.HTTP.ExternalAddress = "https://localhost:30012" + caCfg2.APIs.HTTP.Addr = "localhost:30012" + err = caCfg2.Validate() + require.NoError(t, err) + caShutdown2 := caTest.New(t, caCfg2) + defer caShutdown2() + + err = getSigningRecords(ctx, t, ca_addr, []tls.Certificate{crt1}) + require.NoError(t, err) + + certData2, err := getNewCertificate(ctx, t, config.CERTIFICATE_AUTHORITY_HOST, pk, []tls.Certificate{crt1}) + require.NoError(t, err) + crt2, err := tls.X509KeyPair(certData2, key) + require.NoError(t, err) + + // use crt2 without distribution point + _, err = getNewCertificate(ctx, t, config.CERTIFICATE_AUTHORITY_HOST, pk, []tls.Certificate{crt2}) + require.NoError(t, err) + + // try use revoked crt1 + err = getSigningRecords(ctx, t, config.CERTIFICATE_AUTHORITY_HOST, []tls.Certificate{crt1}) + require.Error(t, err) +} diff --git a/coap-gateway/service/auth.go b/coap-gateway/service/auth.go index a22c694f6..6fcd497d1 100644 --- a/coap-gateway/service/auth.go +++ b/coap-gateway/service/auth.go @@ -81,8 +81,8 @@ func (s *Service) VerifyAndResolveDeviceID(tlsDeviceID, paramDeviceID string, cl return deviceID, nil } -func verifyChain(ctx context.Context, chain []*x509.Certificate, capool *x509.CertPool, identityPropertiesRequired, crlVerificationEnabled bool, verifyByCRL pkgX509.VerifyByCRL) error { - err := pkgX509.VerifyChain(ctx, chain, capool, pkgX509.CRLVerification{ +func verifyChain(chain []*x509.Certificate, capool *x509.CertPool, identityPropertiesRequired, crlVerificationEnabled bool, verifyByCRL pkgX509.VerifyByCRL) error { + err := pkgX509.VerifyChain(chain, capool, pkgX509.CRLVerification{ Enabled: crlVerificationEnabled, Verify: verifyByCRL, }) @@ -117,7 +117,7 @@ func verifyChain(ctx context.Context, chain []*x509.Certificate, capool *x509.Ce return nil } -func MakeGetConfigForClient(ctx context.Context, tlsCfg *tls.Config, identityPropertiesRequired, crlVerificationEnabled bool, verifyByCRL pkgX509.VerifyByCRL) tls.Config { +func MakeGetConfigForClient(tlsCfg *tls.Config, identityPropertiesRequired, crlVerificationEnabled bool, verifyByCRL pkgX509.VerifyByCRL) tls.Config { return tls.Config{ GetCertificate: tlsCfg.GetCertificate, MinVersion: tlsCfg.MinVersion, @@ -126,7 +126,7 @@ func MakeGetConfigForClient(ctx context.Context, tlsCfg *tls.Config, identityPro VerifyPeerCertificate: func(_ [][]byte, chains [][]*x509.Certificate) error { var errs *multierror.Error for _, chain := range chains { - err := verifyChain(ctx, chain, tlsCfg.ClientCAs, identityPropertiesRequired, crlVerificationEnabled, verifyByCRL) + err := verifyChain(chain, tlsCfg.ClientCAs, identityPropertiesRequired, crlVerificationEnabled, verifyByCRL) if err == nil { return nil } diff --git a/coap-gateway/service/auth_test.go b/coap-gateway/service/auth_test.go index e089f0e3b..2d66c0af0 100644 --- a/coap-gateway/service/auth_test.go +++ b/coap-gateway/service/auth_test.go @@ -22,7 +22,6 @@ import ( oauthTest "github.com/plgd-dev/hub/v2/test/oauth-server/test" testService "github.com/plgd-dev/hub/v2/test/service" "github.com/stretchr/testify/require" - "go.uber.org/zap" "google.golang.org/grpc" "google.golang.org/grpc/credentials" ) @@ -36,7 +35,6 @@ func TestCertificateWithCRL(t *testing.T) { caCfg.Signer.CRL.Enabled = true caCfg.Signer.CRL.ExpiresIn = time.Second * 10 coapgwCfg := coapgwTest.MakeConfig(t) - coapgwCfg.Log.Level = zap.DebugLevel coapgwCfg.APIs.COAP.TLS.Enabled = new(bool) *coapgwCfg.APIs.COAP.TLS.Enabled = true coapgwCfg.APIs.COAP.TLS.Embedded.ClientCertificateRequired = true diff --git a/coap-gateway/service/service.go b/coap-gateway/service/service.go index f410a7f8f..ed892706d 100644 --- a/coap-gateway/service/service.go +++ b/coap-gateway/service/service.go @@ -622,7 +622,7 @@ func (s *Service) createServices(fileWatcher *fsnotify.Watcher, logger log.Logge coapService.WithOnInactivityConnection(s.onInactivityConnection), coapService.WithMessagePool(s.messagePool), coapService.WithOverrideTLS(func(cfg *tls.Config, verifyByCRL pkgX509.VerifyByCRL) *tls.Config { - tlsCfg := MakeGetConfigForClient(s.ctx, cfg, s.config.APIs.COAP.InjectedCOAPConfig.TLSConfig.IdentityPropertiesRequired, s.config.APIs.COAP.Config.TLS.Embedded.CRL.Enabled, verifyByCRL) + tlsCfg := MakeGetConfigForClient(cfg, s.config.APIs.COAP.InjectedCOAPConfig.TLSConfig.IdentityPropertiesRequired, s.config.APIs.COAP.Config.TLS.Embedded.CRL.Enabled, verifyByCRL) return &tlsCfg }), ) diff --git a/grpc-gateway/service/service.go b/grpc-gateway/service/service.go index a7fae809c..96619de5f 100644 --- a/grpc-gateway/service/service.go +++ b/grpc-gateway/service/service.go @@ -34,7 +34,7 @@ func New(ctx context.Context, config Config, fileWatcher *fsnotify.Watcher, logg validator.Close() return nil, fmt.Errorf("cannot create grpc server options: %w", err) } - server, err := server.New(config.APIs.GRPC.Config.BaseConfig, fileWatcher, logger, tracerProvider, opts...) + server, err := server.New(config.APIs.GRPC.Config.BaseConfig, fileWatcher, logger, tracerProvider, nil, opts...) if err != nil { validator.Close() otelClient.Close() diff --git a/identity-store/service/service.go b/identity-store/service/service.go index ae3241d4c..7f1971196 100644 --- a/identity-store/service/service.go +++ b/identity-store/service/service.go @@ -75,7 +75,7 @@ func NewService(persistence Persistence, publisher *publisher.Publisher, ownerCl } func NewServer(ctx context.Context, cfg Config, fileWatcher *fsnotify.Watcher, logger log.Logger, tracerProvider trace.TracerProvider, publisher *publisher.Publisher, grpcOpts ...grpc.ServerOption) (*Server, error) { - grpcServer, err := server.New(cfg.APIs.GRPC.BaseConfig, fileWatcher, logger, tracerProvider, grpcOpts...) + grpcServer, err := server.New(cfg.APIs.GRPC.BaseConfig, fileWatcher, logger, tracerProvider, nil, grpcOpts...) if err != nil { return nil, fmt.Errorf("cannot create grpc listener: %w", err) } diff --git a/m2m-oauth-server/service/grpc/service.go b/m2m-oauth-server/service/grpc/service.go index 184212fc2..a5bd1c0f6 100644 --- a/m2m-oauth-server/service/grpc/service.go +++ b/m2m-oauth-server/service/grpc/service.go @@ -20,7 +20,7 @@ func New(config Config, m2mOAuthServiceServer *M2MOAuthServiceServer, validator if err != nil { return nil, fmt.Errorf("cannot create grpc server options: %w", err) } - server, err := server.New(config.BaseConfig, fileWatcher, logger, tracerProvider, opts...) + server, err := server.New(config.BaseConfig, fileWatcher, logger, tracerProvider, nil, opts...) if err != nil { return nil, err } diff --git a/pkg/net/grpc/server/server.go b/pkg/net/grpc/server/server.go index e0ffc410d..15cf71d30 100644 --- a/pkg/net/grpc/server/server.go +++ b/pkg/net/grpc/server/server.go @@ -5,18 +5,20 @@ import ( "github.com/plgd-dev/hub/v2/pkg/fsnotify" "github.com/plgd-dev/hub/v2/pkg/log" + "github.com/plgd-dev/hub/v2/pkg/security/certManager/general" "github.com/plgd-dev/hub/v2/pkg/security/certManager/server" + pkgX509 "github.com/plgd-dev/hub/v2/pkg/security/x509" "go.opentelemetry.io/otel/trace" "google.golang.org/grpc" "google.golang.org/grpc/credentials" ) -func New(config BaseConfig, fileWatcher *fsnotify.Watcher, logger log.Logger, tracerProvider trace.TracerProvider, opts ...grpc.ServerOption) (*Server, error) { +func New(config BaseConfig, fileWatcher *fsnotify.Watcher, logger log.Logger, tracerProvider trace.TracerProvider, customDPVerify pkgX509.CustomDistributionPointVerification, opts ...grpc.ServerOption) (*Server, error) { err := config.Validate() if err != nil { return nil, fmt.Errorf("invalid config: %w", err) } - tls, err := server.New(config.TLS, fileWatcher, logger, tracerProvider) + tls, err := server.New(config.TLS, fileWatcher, logger, tracerProvider, general.WithCustomDistributionPointVerification(customDPVerify)) if err != nil { return nil, fmt.Errorf("cannot create cert manager %w", err) } diff --git a/pkg/security/certManager/client/certManager.go b/pkg/security/certManager/client/certManager.go index 817ae778a..6d87005b7 100644 --- a/pkg/security/certManager/client/certManager.go +++ b/pkg/security/certManager/client/certManager.go @@ -14,10 +14,10 @@ type Config = pkgTls.ClientConfig // CertManager holds certificates from filesystem watched for changes type CertManager = general.ClientCertManager -func New(config Config, fileWatcher *fsnotify.Watcher, logger log.Logger, tracerProvider trace.TracerProvider) (*CertManager, error) { - return general.NewClientCertManager(config, fileWatcher, logger, tracerProvider) +func New(config Config, fileWatcher *fsnotify.Watcher, logger log.Logger, tracerProvider trace.TracerProvider, opts ...general.SetOption) (*CertManager, error) { + return general.NewClientCertManager(config, fileWatcher, logger, tracerProvider, opts...) } -func NewHTTPClient(config pkgTls.HTTPConfigurer, fileWatcher *fsnotify.Watcher, logger log.Logger, tracerProvider trace.TracerProvider) (*client.Client, error) { - return general.NewHTTPClient(config, fileWatcher, logger, tracerProvider) +func NewHTTPClient(config pkgTls.HTTPConfigurer, fileWatcher *fsnotify.Watcher, logger log.Logger, tracerProvider trace.TracerProvider, opts ...general.SetOption) (*client.Client, error) { + return general.NewHTTPClient(config, fileWatcher, logger, tracerProvider, opts...) } diff --git a/pkg/security/certManager/general/certManager.go b/pkg/security/certManager/general/certManager.go index 2e44f913f..9da8825f4 100644 --- a/pkg/security/certManager/general/certManager.go +++ b/pkg/security/certManager/general/certManager.go @@ -8,6 +8,7 @@ import ( "crypto/x509" "errors" "fmt" + "net/url" "strings" "sync" "time" @@ -57,12 +58,13 @@ func (c Config) Validate() error { type CertManager struct { config Config - fileWatcher *fsnotify.Watcher - verifyClientCertificate tls.ClientAuthType - logger log.Logger - onFileChangeFunc func(event fsnotify.Event) - done atomic.Bool - crlCache *CRLCache + fileWatcher *fsnotify.Watcher + verifyClientCertificate tls.ClientAuthType + logger log.Logger + onFileChangeFunc func(event fsnotify.Event) + done atomic.Bool + crlCache *CRLCache + customDistributionPointVerification pkgX509.CustomDistributionPointVerification // override CRL verification for given host private struct { mutex sync.Mutex @@ -87,13 +89,14 @@ func tryToWatchFile(file urischeme.URIScheme, fileWatcher *fsnotify.Watcher, rem return nil } -func newCertManager(config Config, fileWatcher *fsnotify.Watcher, logger log.Logger, verifyClientCertificate tls.ClientAuthType, crlCache *CRLCache, cleanUpOnError fn.FuncList) (*CertManager, error) { +func newCertManager(config Config, fileWatcher *fsnotify.Watcher, logger log.Logger, verifyClientCertificate tls.ClientAuthType, crlCache *CRLCache, dpVerify pkgX509.CustomDistributionPointVerification, cleanUpOnError fn.FuncList) (*CertManager, error) { c := &CertManager{ - fileWatcher: fileWatcher, - config: config, - verifyClientCertificate: verifyClientCertificate, - logger: logger, - crlCache: crlCache, + fileWatcher: fileWatcher, + config: config, + verifyClientCertificate: verifyClientCertificate, + logger: logger, + crlCache: crlCache, + customDistributionPointVerification: dpVerify, } _, err := c.loadCAs() if err != nil { @@ -122,7 +125,12 @@ func newCertManager(config Config, fileWatcher *fsnotify.Watcher, logger log.Log } // New creates a new certificate manager which watches for certs in a filesystem -func New(config Config, fileWatcher *fsnotify.Watcher, logger log.Logger, tracerProvider trace.TracerProvider) (*CertManager, error) { +func New(config Config, fileWatcher *fsnotify.Watcher, logger log.Logger, tracerProvider trace.TracerProvider, opts ...SetOption) (*CertManager, error) { + options := Options{} + for _, opt := range opts { + opt(&options) + } + verifyClientCertificate := tls.RequireAndVerifyClientCert if !config.ClientCertificateRequired { verifyClientCertificate = tls.NoClientCert @@ -139,7 +147,7 @@ func New(config Config, fileWatcher *fsnotify.Watcher, logger log.Logger, tracer cleanUpOnError.AddFunc(crlCache.Close) } - c, err := newCertManager(config, fileWatcher, logger, verifyClientCertificate, crlCache, cleanUpOnError) + c, err := newCertManager(config, fileWatcher, logger, verifyClientCertificate, crlCache, options.CustomDistributionPointVerification, cleanUpOnError) if err != nil { cleanUpOnError.Execute() return nil, err @@ -170,9 +178,7 @@ func (a *CertManager) GetServerTLSConfig() *tls.Config { ClientAuth: a.verifyClientCertificate, } if a.config.CRL.Enabled { - // TODO: propagate context - ctx := context.Background() - cfg.VerifyPeerCertificate = pkgX509.VerifyChains(ctx, a.GetCertificateAuthorities(), pkgX509.CRLVerification{ + cfg.VerifyPeerCertificate = pkgX509.VerifyChains(a.GetCertificateAuthorities(), pkgX509.CRLVerification{ Enabled: true, Verify: a.VerifyByCRL, }) @@ -189,9 +195,7 @@ func (a *CertManager) GetClientTLSConfig() *tls.Config { MinVersion: tls.VersionTLS12, } if a.config.CRL.Enabled { - // TODO: propagate context - ctx := context.Background() - cfg.VerifyPeerCertificate = pkgX509.VerifyChains(ctx, a.GetCertificateAuthorities(), pkgX509.CRLVerification{ + cfg.VerifyPeerCertificate = pkgX509.VerifyChains(a.GetCertificateAuthorities(), pkgX509.CRLVerification{ Enabled: true, Verify: a.VerifyByCRL, }) @@ -412,22 +416,40 @@ func (a *CertManager) onFileChange(event fsnotify.Event) { } } -func (a *CertManager) VerifyByCRL(ctx context.Context, certificate *x509.Certificate, cdps []string) error { +func (a *CertManager) getHostAndEndpoint(distributionPoint string) (string, string, bool) { + u, err := url.ParseRequestURI(distributionPoint) + if err != nil { + a.logger.Errorf("invalid distribution point(%v): %v", distributionPoint, err) + return "", "", false + } + return u.Scheme + "://" + u.Host, u.Path, true +} + +func (a *CertManager) VerifyByCRL(ctx context.Context, certificate *x509.Certificate, cdps []string) (bool, error) { if !a.config.CRL.Enabled { - return nil + return false, nil } for _, dp := range cdps { + if a.customDistributionPointVerification != nil { + host, ep, ok := a.getHostAndEndpoint(dp) + if ok { + verify, ok := a.customDistributionPointVerification[host] + if ok { + a.logger.Debugf("custom distribution point(%s) CRL verification for certificate(serialNumber=%s)", dp, certificate.SerialNumber.String()) + return verify(ctx, certificate, ep) + } + } + } crl, err := a.crlCache.GetRevocationList(ctx, dp) if err != nil { a.logger.Errorf("failed to download CRL from distribution point(%v): %v", dp, err) continue } if pkgX509.IsRevoked(certificate, crl) { - return fmt.Errorf("certificate(serialNumber=%s) was revoked by CRL(%s)", certificate.SerialNumber.String(), crl.Issuer.String()) + return true, nil } - return nil - + return false, nil } - return fmt.Errorf("failed to verify certificate(serialNumber=%s) by CRL: all distribution points failed", certificate.SerialNumber.String()) + return false, fmt.Errorf("failed to verify certificate(serialNumber=%s) by CRL: all distribution points failed", certificate.SerialNumber.String()) } diff --git a/pkg/security/certManager/general/clientCertManager.go b/pkg/security/certManager/general/clientCertManager.go index d2eb0454f..e313dad67 100644 --- a/pkg/security/certManager/general/clientCertManager.go +++ b/pkg/security/certManager/general/clientCertManager.go @@ -43,7 +43,7 @@ func (c *ClientCertManager) Close() { } // New creates a new certificate manager which watches for certs in a filesystem -func NewClientCertManager(config pkgTls.ClientConfig, fileWatcher *fsnotify.Watcher, logger log.Logger, tracerProvider trace.TracerProvider) (*ClientCertManager, error) { +func NewClientCertManager(config pkgTls.ClientConfig, fileWatcher *fsnotify.Watcher, logger log.Logger, tracerProvider trace.TracerProvider, opts ...SetOption) (*ClientCertManager, error) { if err := config.Validate(); err != nil { return nil, err } @@ -53,7 +53,7 @@ func NewClientCertManager(config pkgTls.ClientConfig, fileWatcher *fsnotify.Watc if err := cfg.Validate(); err != nil { return nil, err } - c, err := New(cfg, fileWatcher, ClientLogger(logger), tracerProvider) + c, err := New(cfg, fileWatcher, ClientLogger(logger), tracerProvider, opts...) if err != nil { return nil, err } @@ -62,8 +62,8 @@ func NewClientCertManager(config pkgTls.ClientConfig, fileWatcher *fsnotify.Watc }, nil } -func NewHTTPClient(config pkgTls.HTTPConfigurer, fileWatcher *fsnotify.Watcher, logger log.Logger, tracerProvider trace.TracerProvider) (*client.Client, error) { - cm, err := NewClientCertManager(config.GetTLS(), fileWatcher, logger, tracerProvider) +func NewHTTPClient(config pkgTls.HTTPConfigurer, fileWatcher *fsnotify.Watcher, logger log.Logger, tracerProvider trace.TracerProvider, opts ...SetOption) (*client.Client, error) { + cm, err := NewClientCertManager(config.GetTLS(), fileWatcher, logger, tracerProvider, opts...) if err != nil { return nil, fmt.Errorf("cannot create cert manager %w", err) } diff --git a/pkg/security/certManager/general/options.go b/pkg/security/certManager/general/options.go new file mode 100644 index 000000000..291ba1015 --- /dev/null +++ b/pkg/security/certManager/general/options.go @@ -0,0 +1,15 @@ +package general + +import pkgX509 "github.com/plgd-dev/hub/v2/pkg/security/x509" + +type Options struct { + CustomDistributionPointVerification pkgX509.CustomDistributionPointVerification +} + +type SetOption = func(cfg *Options) + +func WithCustomDistributionPointVerification(customDistributionPointVerification pkgX509.CustomDistributionPointVerification) SetOption { + return func(o *Options) { + o.CustomDistributionPointVerification = customDistributionPointVerification + } +} diff --git a/pkg/security/certManager/server/certManager.go b/pkg/security/certManager/server/certManager.go index f9336866a..d0ad9ea26 100644 --- a/pkg/security/certManager/server/certManager.go +++ b/pkg/security/certManager/server/certManager.go @@ -71,7 +71,7 @@ func (c *CertManager) GetTLSConfig() *tls.Config { return c.c.GetServerTLSConfig() } -func (c *CertManager) VerifyByCRL(ctx context.Context, certificate *x509.Certificate, cdp []string) error { +func (c *CertManager) VerifyByCRL(ctx context.Context, certificate *x509.Certificate, cdp []string) (bool, error) { return c.c.VerifyByCRL(ctx, certificate, cdp) } @@ -81,7 +81,7 @@ func (c *CertManager) Close() { } // New creates a new certificate manager which watches for certs in a filesystem -func New(config Config, fileWatcher *fsnotify.Watcher, logger log.Logger, tracerProvider trace.TracerProvider) (*CertManager, error) { +func New(config Config, fileWatcher *fsnotify.Watcher, logger log.Logger, tracerProvider trace.TracerProvider, opts ...general.SetOption) (*CertManager, error) { if !config.validated { if err := config.Validate(); err != nil { return nil, err @@ -99,7 +99,7 @@ func New(config Config, fileWatcher *fsnotify.Watcher, logger log.Logger, tracer if err := cfg.Validate(); err != nil { return nil, err } - c, err := general.New(cfg, fileWatcher, logger.With(log.CertManagerKey, "server"), tracerProvider) + c, err := general.New(cfg, fileWatcher, logger.With(log.CertManagerKey, "server"), tracerProvider, opts...) if err != nil { return nil, err } diff --git a/pkg/security/jwt/validator/validator.go b/pkg/security/jwt/validator/validator.go index 2b7cb833c..ab482c3c4 100644 --- a/pkg/security/jwt/validator/validator.go +++ b/pkg/security/jwt/validator/validator.go @@ -12,8 +12,10 @@ import ( "github.com/plgd-dev/hub/v2/pkg/log" pkgHttpUri "github.com/plgd-dev/hub/v2/pkg/net/http/uri" cmClient "github.com/plgd-dev/hub/v2/pkg/security/certManager/client" + "github.com/plgd-dev/hub/v2/pkg/security/certManager/general" jwtValidator "github.com/plgd-dev/hub/v2/pkg/security/jwt" "github.com/plgd-dev/hub/v2/pkg/security/openid" + pkgX509 "github.com/plgd-dev/hub/v2/pkg/security/x509" "go.opentelemetry.io/otel/trace" ) @@ -44,8 +46,9 @@ func (v *Validator) GetParser() *jwtValidator.Validator { type GetOpenIDConfigurationFunc func(ctx context.Context, c *http.Client, authority string) (openid.Config, error) type Options struct { - getOpenIDConfiguration GetOpenIDConfigurationFunc - customTokenIssuerClients map[string]jwtValidator.TokenIssuerClient + getOpenIDConfiguration GetOpenIDConfigurationFunc + customTokenIssuerClients map[string]jwtValidator.TokenIssuerClient + customDistributionPointVerification pkgX509.CustomDistributionPointVerification } func WithGetOpenIDConfiguration(f GetOpenIDConfigurationFunc) func(o *Options) { @@ -60,6 +63,12 @@ func WithCustomTokenIssuerClients(clients map[string]jwtValidator.TokenIssuerCli } } +func WithCustomDistributionPointVerification(dpVerification pkgX509.CustomDistributionPointVerification) func(o *Options) { + return func(o *Options) { + o.customDistributionPointVerification = dpVerification + } +} + func New(ctx context.Context, config Config, fileWatcher *fsnotify.Watcher, logger log.Logger, tracerProvider trace.TracerProvider, opts ...func(o *Options)) (*Validator, error) { options := Options{ getOpenIDConfiguration: openid.GetConfiguration, @@ -78,12 +87,16 @@ func New(ctx context.Context, config Config, fileWatcher *fsnotify.Watcher, logg return nil, errors.New("no endpoints") } + cmOptions := []general.SetOption{} + if options.customDistributionPointVerification != nil { + cmOptions = append(cmOptions, general.WithCustomDistributionPointVerification(options.customDistributionPointVerification)) + } keys := jwtValidator.NewMultiKeyCache() var onClose fn.FuncList openIDConfigurations := make([]openid.Config, 0, len(config.Endpoints)) clients := make(map[string]jwtValidator.TokenIssuerClient, len(config.Endpoints)) for _, authority := range config.Endpoints { - httpClient, err := cmClient.NewHTTPClient(&authority.HTTP, fileWatcher, logger, tracerProvider) + httpClient, err := cmClient.NewHTTPClient(&authority.HTTP, fileWatcher, logger, tracerProvider, cmOptions...) if err != nil { onClose.Execute() return nil, fmt.Errorf("cannot create client cert manager: %w", err) diff --git a/pkg/security/x509/verify.go b/pkg/security/x509/verify.go index 6b0654ebf..583cfa115 100644 --- a/pkg/security/x509/verify.go +++ b/pkg/security/x509/verify.go @@ -5,12 +5,28 @@ import ( "context" "crypto/x509" "errors" + "fmt" "time" "github.com/hashicorp/go-multierror" ) -type VerifyByCRL = func(context.Context, *x509.Certificate, []string) error +type ( + VerifyByCRL = func(context.Context, *x509.Certificate, []string) (bool, error) + VerifyForDistributionPoint = func(context.Context, *x509.Certificate, string) (bool, error) + CustomDistributionPointVerification = map[string]VerifyForDistributionPoint + Options struct { + CustomDistributionPointVerification CustomDistributionPointVerification + } + + SetOption = func(cfg *Options) +) + +func WithCustomDistributionPointVerification(customDistributionPointVerification CustomDistributionPointVerification) SetOption { + return func(o *Options) { + o.CustomDistributionPointVerification = customDistributionPointVerification + } +} func IsRootCA(cert *x509.Certificate) bool { return cert.IsCA && bytes.Equal(cert.RawIssuer, cert.RawSubject) && cert.CheckSignature(cert.SignatureAlgorithm, cert.RawTBSCertificate, cert.Signature) == nil @@ -67,10 +83,11 @@ func Verify(certificates []*x509.Certificate, certificateAuthorities []*x509.Cer type CRLVerification struct { Enabled bool + Ctx context.Context Verify VerifyByCRL } -func VerifyChain(ctx context.Context, chain []*x509.Certificate, capool *x509.CertPool, crlVerify CRLVerification) error { +func VerifyChain(chain []*x509.Certificate, capool *x509.CertPool, crlVerify CRLVerification) error { if len(chain) == 0 { return errors.New("certificate chain is empty") } @@ -92,18 +109,28 @@ func VerifyChain(ctx context.Context, chain []*x509.Certificate, capool *x509.Ce if crlVerify.Verify == nil { return errors.New("cannot verify certificate validity by CRL: verification function not provided") } - if err = crlVerify.Verify(ctx, certificate, certificate.CRLDistributionPoints); err != nil { - return err + ctx := crlVerify.Ctx + if ctx == nil { + var cancel context.CancelFunc + ctx, cancel = context.WithTimeout(context.Background(), time.Second*5) + defer cancel() + } + revoked, err := crlVerify.Verify(ctx, certificate, certificate.CRLDistributionPoints) + if err != nil { + return fmt.Errorf("cannot verify certificate validity by CRL: %w", err) + } + if revoked { + return errors.New("certificate revoked by CRL") } } return nil } -func VerifyChains(ctx context.Context, capool *x509.CertPool, crlVerify CRLVerification) func(rawCerts [][]byte, verifiedChains [][]*x509.Certificate) error { +func VerifyChains(capool *x509.CertPool, crlVerify CRLVerification) func(rawCerts [][]byte, verifiedChains [][]*x509.Certificate) error { return func(_ [][]byte, chains [][]*x509.Certificate) error { var errs *multierror.Error for _, chain := range chains { - err := VerifyChain(ctx, chain, capool, crlVerify) + err := VerifyChain(chain, capool, crlVerify) if err == nil { return nil } diff --git a/resource-aggregate/service/service.go b/resource-aggregate/service/service.go index 8f2eb6669..0c8447e04 100644 --- a/resource-aggregate/service/service.go +++ b/resource-aggregate/service/service.go @@ -110,7 +110,7 @@ func newGrpcServer(ctx context.Context, config GRPCConfig, fileWatcher *fsnotify return nil, fmt.Errorf("cannot create grpc server options: %w", err) } - grpcServer, err := server.New(config.BaseConfig, fileWatcher, logger, tracerProvider, opts...) + grpcServer, err := server.New(config.BaseConfig, fileWatcher, logger, tracerProvider, nil, opts...) if err != nil { validator.Close() return nil, fmt.Errorf("cannot create grpc server: %w", err) diff --git a/resource-directory/service/service.go b/resource-directory/service/service.go index d8931e584..fd1e549d1 100644 --- a/resource-directory/service/service.go +++ b/resource-directory/service/service.go @@ -38,7 +38,7 @@ func New(ctx context.Context, config Config, fileWatcher *fsnotify.Watcher, logg validator.Close() return nil, fmt.Errorf("cannot create grpc server options: %w", err) } - server, err := server.New(config.APIs.GRPC.BaseConfig, fileWatcher, logger, tracerProvider, opts...) + server, err := server.New(config.APIs.GRPC.BaseConfig, fileWatcher, logger, tracerProvider, nil, opts...) if err != nil { otelClient.Close() validator.Close() diff --git a/snippet-service/service/grpc/service.go b/snippet-service/service/grpc/service.go index 7b1bf7c15..ca5d1279d 100644 --- a/snippet-service/service/grpc/service.go +++ b/snippet-service/service/grpc/service.go @@ -20,7 +20,7 @@ func New(config Config, snippetServiceServer *SnippetServiceServer, validator *v if err != nil { return nil, fmt.Errorf("cannot create grpc server options: %w", err) } - server, err := server.New(config.BaseConfig, fileWatcher, logger, tracerProvider, opts...) + server, err := server.New(config.BaseConfig, fileWatcher, logger, tracerProvider, nil, opts...) if err != nil { return nil, err } diff --git a/tools/grpc-reflection/service/service.go b/tools/grpc-reflection/service/service.go index 418373818..a5ad5c5f7 100644 --- a/tools/grpc-reflection/service/service.go +++ b/tools/grpc-reflection/service/service.go @@ -25,7 +25,7 @@ func New(config Config, fileWatcher *fsnotify.Watcher, logger log.Logger) (*serv if err != nil { return nil, fmt.Errorf("cannot create grpc server options: %w", err) } - server, err := server.New(config.APIs.GRPC.BaseConfig, fileWatcher, logger, noop.NewTracerProvider(), opts...) + server, err := server.New(config.APIs.GRPC.BaseConfig, fileWatcher, logger, noop.NewTracerProvider(), nil, opts...) if err != nil { return nil, err }