Skip to content

Commit

Permalink
fixup! Verification by CRL
Browse files Browse the repository at this point in the history
  • Loading branch information
Danielius1922 committed Nov 2, 2024
1 parent 4a17e5e commit 89a4370
Show file tree
Hide file tree
Showing 24 changed files with 342 additions and 75 deletions.
1 change: 0 additions & 1 deletion certificate-authority/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ apis:
keyFile: "/secrets/private/cert.key"
certFile: "/secrets/private/cert.crt"
clientCertificateRequired: true

authorization:
ownerClaim: "sub"
audience: ""
Expand Down
4 changes: 2 additions & 2 deletions certificate-authority/service/grpc/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down
5 changes: 3 additions & 2 deletions certificate-authority/service/grpc/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,20 @@ 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"
)

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
}
Expand Down
6 changes: 3 additions & 3 deletions certificate-authority/service/grpc/signer.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import (
"crypto/ecdsa"
"crypto/x509"
"errors"
"strings"
"path"
"time"

"github.com/google/uuid"
Expand Down Expand Up @@ -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 {
Expand Down
64 changes: 59 additions & 5 deletions certificate-authority/service/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
)
Expand Down Expand Up @@ -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 {
Expand All @@ -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)
Expand All @@ -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()
Expand Down
136 changes: 136 additions & 0 deletions certificate-authority/service/service_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
8 changes: 4 additions & 4 deletions coap-gateway/service/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
})
Expand Down Expand Up @@ -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,
Expand All @@ -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
}
Expand Down
2 changes: 0 additions & 2 deletions coap-gateway/service/auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion coap-gateway/service/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}),
)
Expand Down
2 changes: 1 addition & 1 deletion grpc-gateway/service/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion identity-store/service/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
Loading

0 comments on commit 89a4370

Please sign in to comment.