From 51222e0ea4edcd15a04ac8abf034ac655b3ab9af Mon Sep 17 00:00:00 2001 From: johnabass Date: Wed, 20 Nov 2024 22:33:13 -0800 Subject: [PATCH 1/4] custom trust levels based on the disposition of client certificates --- token/claimBuilder.go | 87 ++++++++++++++++++++++++++++++++------ token/claimBuilder_test.go | 5 +-- token/options.go | 44 +++++++++++++++++++ 3 files changed, 120 insertions(+), 16 deletions(-) diff --git a/token/claimBuilder.go b/token/claimBuilder.go index 3aeec21..84157bc 100644 --- a/token/claimBuilder.go +++ b/token/claimBuilder.go @@ -4,6 +4,7 @@ package token import ( "context" + "crypto/x509" "errors" "fmt" "net/http" @@ -12,6 +13,7 @@ import ( "github.com/xmidt-org/themis/random" "github.com/xmidt-org/themis/xhttp/xhttpclient" + "github.com/xmidt-org/themis/xhttp/xhttpserver" "github.com/go-kit/kit/endpoint" kithttp "github.com/go-kit/kit/transport/http" @@ -178,16 +180,75 @@ func newRemoteClaimBuilder(client xhttpclient.Interface, metadata map[string]int return &remoteClaimBuilder{endpoint: c.Endpoint(), url: r.URL, extra: metadata}, nil } -// enforcePeerCertificate sets a trust of 1000 if and only if at least (1) peer certificate -// was supplied. -func enforcePeerCertificate(_ context.Context, r *Request, target map[string]interface{}) error { - if r.TLS != nil && len(r.TLS.PeerCertificates) > 0 { - target[ClaimTrust] = 1000 - } else { - target[ClaimTrust] = 0 +func newClientCertificateClaimBuiler(cc *ClientCertificates) (cb *clientCertificateClaimBuilder, err error) { + if cc == nil { + return } - return nil + cb = &clientCertificateClaimBuilder{ + trust: cc.Trust, + } + + if len(cc.RootCAFile) > 0 { + cb.roots, err = xhttpserver.ReadCertPool(cc.RootCAFile) + } + + if err == nil && len(cc.IntermediatesFile) > 0 { + cb.intermediates, err = xhttpserver.ReadCertPool(cc.IntermediatesFile) + } + + return +} + +type clientCertificateClaimBuilder struct { + roots *x509.CertPool + intermediates *x509.CertPool + trust Trust +} + +func (cb *clientCertificateClaimBuilder) AddClaims(_ context.Context, r *Request, target map[string]interface{}) (err error) { + // first case: this didn't come from a TLS connection, or it did but the client gave no certificates + if r.TLS == nil || len(r.TLS.PeerCertificates) == 0 { + target[ClaimTrust] = cb.trust.NoCertificates + return + } + + now := time.Now() + for i, pc := range r.TLS.PeerCertificates { + if i < len(r.TLS.VerifiedChains) && len(r.TLS.VerifiedChains[i]) > 0 { + // the TLS layer already verified this certificate, so we're done + target[ClaimTrust] = cb.trust.Trusted + return + } + + // special logic around expired certificates + expired := now.After(pc.NotAfter) + vo := x509.VerifyOptions{ + // always set the current time so that we disambiguate expired + // from untrusted. + CurrentTime: pc.NotAfter.Add(-time.Second), + Roots: cb.roots, + Intermediates: cb.intermediates, + } + + _, verifyErr := pc.Verify(vo) + + switch { + case expired && verifyErr != nil: + target[ClaimTrust] = cb.trust.ExpiredUntrusted + + case !expired && verifyErr != nil: + target[ClaimTrust] = cb.trust.Untrusted + + case expired && verifyErr == nil: + target[ClaimTrust] = cb.trust.ExpiredTrusted + + case !expired && verifyErr == nil: + target[ClaimTrust] = cb.trust.Trusted + } + } + + return } // NewClaimBuilders constructs a ClaimBuilders from configuration. The returned instance is typically @@ -268,10 +329,12 @@ func NewClaimBuilders(n random.Noncer, client xhttpclient.Interface, o Options) }) } - builders = append( - builders, - ClaimBuilderFunc(enforcePeerCertificate), - ) + if cb, err := newClientCertificateClaimBuiler(o.ClientCertificates); cb != nil && err == nil { + builders = append( + builders, + cb, + ) + } return builders, nil } diff --git a/token/claimBuilder_test.go b/token/claimBuilder_test.go index 93a4fa1..eb363b8 100644 --- a/token/claimBuilder_test.go +++ b/token/claimBuilder_test.go @@ -545,7 +545,7 @@ func (suite *NewClaimBuildersTestSuite) TestMinimum() { ) suite.Equal( - map[string]interface{}{"request": 123, "trust": 0}, + map[string]interface{}{"request": 123}, actual, ) } @@ -691,7 +691,6 @@ func (suite *NewClaimBuildersTestSuite) TestStatic() { "static1": suite.rawMessage(-72.5), "static2": suite.rawMessage([]string{"a", "b"}), "request": 123, - "trust": 0, }, actual, ) @@ -738,7 +737,6 @@ func (suite *NewClaimBuildersTestSuite) TestNoRemote() { "iat": suite.expectedNow.UTC().Unix(), "nbf": suite.expectedNow.Add(15 * time.Second).UTC().Unix(), "exp": suite.expectedNow.Add(24 * time.Hour).UTC().Unix(), - "trust": 0, }, actual, ) @@ -823,7 +821,6 @@ func (suite *NewClaimBuildersTestSuite) TestFull() { "iat": suite.expectedNow.UTC().Unix(), "nbf": suite.expectedNow.Add(15 * time.Second).UTC().Unix(), "exp": suite.expectedNow.Add(24 * time.Hour).UTC().Unix(), - "trust": 0, }, actual, ) diff --git a/token/options.go b/token/options.go index 01ac2d0..70eed16 100644 --- a/token/options.go +++ b/token/options.go @@ -93,11 +93,55 @@ type PartnerID struct { Default string } +// Trust describes the various levels of trust based upon client +// certificate state. +type Trust struct { + // NoCertificates is the trust level to set when no client certificates are present. + // This value has no default. If unset, the trust value is zero (0). + NoCertificates int + + // ExpiredUntrusted is the trust level to set when a certificate has both expired + // and is within an CA chain that we do not trust. + ExpiredUntrusted int + + // ExpiredTrusted is the trust level to set when a certificate has both expired + // and IS within a trusted CA chain. + ExpiredTrusted int + + // Untrusted is the trust level to set when a client has an otherwise valid + // certificate, but that certificate is part of an untrusted chain. + Untrusted int + + // Trusted is the trust level to set when a client certificate is part of + // a trusted CA chain. + Trusted int +} + +// ClientCertificates describes how peer certificates are to be handled when +// it comes to issuing tokens. +type ClientCertificates struct { + // RootCAFile is the PEM bundle of certificates used for client certificate verification. + // If unset, the system verifier and/or bundle is used. + RootCAFile string + + // IntermediatesFile is the PEM bundle of certificates used for client certificate verification. + // If unset, no intermediary certificates are considered. + IntermediatesFile string + + // Trust defines the trust levels to set for various situations involving + // client certificates. + Trust Trust +} + // Options holds the configurable information for a token Factory type Options struct { // Alg is the required JWT signing algorithm to use Alg string + // ClientCertificates describes how peer certificates affect the issued tokens. + // If unset, client certificates are not considered when issuing tokens. + ClientCertificates *ClientCertificates + // Key describes the signing key to use Key key.Descriptor From c1a4a10e529262b9a34bbba31b2a495d6599fce5 Mon Sep 17 00:00:00 2001 From: johnabass Date: Wed, 20 Nov 2024 22:41:59 -0800 Subject: [PATCH 2/4] take the highest trust level in situations where a trusted cert isn't found --- token/claimBuilder.go | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/token/claimBuilder.go b/token/claimBuilder.go index 84157bc..c171fc6 100644 --- a/token/claimBuilder.go +++ b/token/claimBuilder.go @@ -206,17 +206,19 @@ type clientCertificateClaimBuilder struct { trust Trust } -func (cb *clientCertificateClaimBuilder) AddClaims(_ context.Context, r *Request, target map[string]interface{}) (err error) { - // first case: this didn't come from a TLS connection, or it did but the client gave no certificates +func (cb *clientCertificateClaimBuilder) getTrust(r *Request, target map[string]interface{}) (err error) { + // simplest case: this didn't come from a TLS connection, or it did but the client gave no certificates if r.TLS == nil || len(r.TLS.PeerCertificates) == 0 { target[ClaimTrust] = cb.trust.NoCertificates return } now := time.Now() + var trust int for i, pc := range r.TLS.PeerCertificates { if i < len(r.TLS.VerifiedChains) && len(r.TLS.VerifiedChains[i]) > 0 { // the TLS layer already verified this certificate, so we're done + // we assume Trusted is the highest trust level target[ClaimTrust] = cb.trust.Trusted return } @@ -235,19 +237,29 @@ func (cb *clientCertificateClaimBuilder) AddClaims(_ context.Context, r *Request switch { case expired && verifyErr != nil: - target[ClaimTrust] = cb.trust.ExpiredUntrusted + if trust < cb.trust.ExpiredUntrusted { + trust = cb.trust.ExpiredUntrusted + } case !expired && verifyErr != nil: - target[ClaimTrust] = cb.trust.Untrusted + if trust < cb.trust.Untrusted { + trust = cb.trust.Untrusted + } case expired && verifyErr == nil: - target[ClaimTrust] = cb.trust.ExpiredTrusted + if trust < cb.trust.ExpiredTrusted { + trust = cb.trust.ExpiredTrusted + } case !expired && verifyErr == nil: + // we assume Trusted is the highest trust level target[ClaimTrust] = cb.trust.Trusted + return } } + // take the highest, non-Trusted level + target[ClaimTrust] = trust return } From 4a1ff4498064d025dcea91c2bfe2f6e0829b1092 Mon Sep 17 00:00:00 2001 From: johnabass Date: Wed, 20 Nov 2024 22:42:36 -0800 Subject: [PATCH 3/4] remove trust from these configurations --- devMode.go | 2 -- themis.yaml | 2 -- 2 files changed, 4 deletions(-) diff --git a/devMode.go b/devMode.go index d1e3a4e..74f0a90 100644 --- a/devMode.go +++ b/devMode.go @@ -78,8 +78,6 @@ token: parameter: uuid - key: iss value: "development" - - key: trust - value: 1000 - key: sub value: "client-supplied" - key: aud diff --git a/themis.yaml b/themis.yaml index e1f069f..e9a540e 100644 --- a/themis.yaml +++ b/themis.yaml @@ -72,8 +72,6 @@ token: parameter: uuid - key: iss value: "development" - - key: trust - value: 1000 - key: sub value: "client-supplied" - key: aud From eb6a4f96c8cdf3e159384d4279f3994e69f6dd80 Mon Sep 17 00:00:00 2001 From: johnabass Date: Wed, 20 Nov 2024 22:52:55 -0800 Subject: [PATCH 4/4] allow sane default trust levels --- token/claimBuilder.go | 4 ++-- token/options.go | 47 ++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/token/claimBuilder.go b/token/claimBuilder.go index c171fc6..60f572a 100644 --- a/token/claimBuilder.go +++ b/token/claimBuilder.go @@ -186,7 +186,7 @@ func newClientCertificateClaimBuiler(cc *ClientCertificates) (cb *clientCertific } cb = &clientCertificateClaimBuilder{ - trust: cc.Trust, + trust: cc.Trust.enforceDefaults(), } if len(cc.RootCAFile) > 0 { @@ -206,7 +206,7 @@ type clientCertificateClaimBuilder struct { trust Trust } -func (cb *clientCertificateClaimBuilder) getTrust(r *Request, target map[string]interface{}) (err error) { +func (cb *clientCertificateClaimBuilder) AddClaims(_ context.Context, r *Request, target map[string]interface{}) (err error) { // simplest case: this didn't come from a TLS connection, or it did but the client gave no certificates if r.TLS == nil || len(r.TLS.PeerCertificates) == 0 { target[ClaimTrust] = cb.trust.NoCertificates diff --git a/token/options.go b/token/options.go index 70eed16..172cdbd 100644 --- a/token/options.go +++ b/token/options.go @@ -9,6 +9,14 @@ import ( "github.com/xmidt-org/themis/key" ) +const ( + DefaultTrustLevelNoCertificates = 0 + DefaultTrustLevelExpiredUntrusted = 100 + DefaultTrustLevelExpiredTrusted = 1000 + DefaultTrustLevelUntrusted = 1000 + DefaultTrustLevelTrusted = 1000 +) + // RemoteClaims describes a remote HTTP endpoint that can produce claims given the // metadata from a token request. type RemoteClaims struct { @@ -97,31 +105,68 @@ type PartnerID struct { // certificate state. type Trust struct { // NoCertificates is the trust level to set when no client certificates are present. - // This value has no default. If unset, the trust value is zero (0). + // If unset, DefaultTrustLevelNoCertificates is used. NoCertificates int // ExpiredUntrusted is the trust level to set when a certificate has both expired // and is within an CA chain that we do not trust. + // + // If unset, DefaultTrustLevelExpiredTrusted is used. ExpiredUntrusted int // ExpiredTrusted is the trust level to set when a certificate has both expired // and IS within a trusted CA chain. + // + // If unset, DefaultTrustLevelExpiredTrusted is used. ExpiredTrusted int // Untrusted is the trust level to set when a client has an otherwise valid // certificate, but that certificate is part of an untrusted chain. + // + // If unset, DefaultTrustLevelUntrusted is used. Untrusted int // Trusted is the trust level to set when a client certificate is part of + // + // If unset, DefaultTrustLevelTrusted is used. // a trusted CA chain. Trusted int } +// enforceDefaults returns a Trust that has ensures any unset values are +// set to their defaults. +func (t Trust) enforceDefaults() (other Trust) { + other = t + if other.NoCertificates <= 0 { + other.NoCertificates = DefaultTrustLevelNoCertificates + } + + if other.ExpiredUntrusted <= 0 { + other.ExpiredUntrusted = DefaultTrustLevelExpiredUntrusted + } + + if other.ExpiredTrusted <= 0 { + other.ExpiredTrusted = DefaultTrustLevelExpiredTrusted + } + + if other.Untrusted <= 0 { + other.Untrusted = DefaultTrustLevelUntrusted + } + + if other.Trusted <= 0 { + other.Trusted = DefaultTrustLevelTrusted + } + + return +} + // ClientCertificates describes how peer certificates are to be handled when // it comes to issuing tokens. type ClientCertificates struct { // RootCAFile is the PEM bundle of certificates used for client certificate verification. // If unset, the system verifier and/or bundle is used. + // + // Generally, this value should be the same as the the mtls.clientCACertificateFile. RootCAFile string // IntermediatesFile is the PEM bundle of certificates used for client certificate verification.