Skip to content

Commit

Permalink
fixes #2154 500 error on mfa enrollment in HA, fixes #2159 HA certs
Browse files Browse the repository at this point in the history
- fixes 500 internal error on MFA enrllment in HA deployments
- fixes HA API Session Certs not working in HA
- adds spiffehlp module to common
- adds tests api session certs w/ spiffe id
- adds tests for SPIFFE IDs in API Session Certs
  • Loading branch information
andrewpmartinez committed Jul 22, 2024
1 parent dfda33f commit 5027c04
Show file tree
Hide file tree
Showing 7 changed files with 300 additions and 36 deletions.
90 changes: 90 additions & 0 deletions common/spiffehlp/spiffe.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
/*
Copyright NetFoundry Inc.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
https://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package spiffehlp

import (
"crypto/tls"
"crypto/x509"
"fmt"
"github.com/pkg/errors"
"net/url"
)

// GetSpiffeIdFromCertChain cycles through a slice of certificates that goes from leaf up CAs. Each certificate
// must contain 0 or 1 spiffe:// URI SAN. The first encountered SPIFFE id looking up the chain back to the root CA is returned.
// If no SPIFFE id is encountered, nil is returned. Errors are returned for parsing and processing errors only.
func GetSpiffeIdFromCertChain(certs []*x509.Certificate) (*url.URL, error) {
var spiffeId *url.URL
for _, cert := range certs {
var err error
spiffeId, err = GetSpiffeIdFromCert(cert)

if err != nil {
return nil, fmt.Errorf("failed to determine SPIFFE ID from x509 certificate chain: %w", err)
}

if spiffeId != nil {
return spiffeId, nil
}
}

return nil, errors.New("failed to determine SPIFFE ID, no spiffe:// URI SANs found in x509 certificate chain")
}

// GetSpiffeIdFromTlsCertChain will search a tls certificate chain for a trust domain encoded as a spiffe:// URI SAN.
// Each certificate must contain 0 or 1 spiffe:// URI SAN. The first SPIFFE id looking up the chain is returned. If
// no SPIFFE id is encountered, nil is returned. Errors are returned for parsing and processing errors only.
func GetSpiffeIdFromTlsCertChain(tlsCerts []*tls.Certificate) (*url.URL, error) {
for _, tlsCert := range tlsCerts {
for i, rawCert := range tlsCert.Certificate {
cert, err := x509.ParseCertificate(rawCert)

if err != nil {
return nil, fmt.Errorf("failed to parse TLS cert at index [%d]: %w", i, err)
}

spiffeId, err := GetSpiffeIdFromCert(cert)

if err != nil {
return nil, fmt.Errorf("failed to determine SPIFFE ID from TLS cert at index [%d]: %w", i, err)
}

if spiffeId != nil {
return spiffeId, nil
}
}
}

return nil, nil
}

// GetSpiffeIdFromCert will search a x509 certificate for a trust domain encoded as a spiffe:// URI SAN.
// Each certificate must contain 0 or 1 spiffe:// URI SAN. The first SPIFFE id looking up the chain is returned. If
// no SPIFFE id is encountered, nil is returned. Errors are returned for parsing and processing errors only.
func GetSpiffeIdFromCert(cert *x509.Certificate) (*url.URL, error) {
var spiffeId *url.URL
for _, uriSan := range cert.URIs {
if uriSan.Scheme == "spiffe" {
if spiffeId != nil {
return nil, fmt.Errorf("multiple URI SAN spiffe:// ids encountered, must only have one, encountered at least two: [%s] and [%s]", spiffeId.String(), uriSan.String())
}
spiffeId = uriSan
}
}

return spiffeId, nil
}
21 changes: 15 additions & 6 deletions controller/internal/routes/current_api_session_router.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"github.com/openziti/edge-api/rest_model"
"github.com/openziti/ziti/controller/env"
"github.com/openziti/ziti/controller/internal/permissions"
"github.com/openziti/ziti/controller/model"
"github.com/openziti/ziti/controller/response"
"net/http"
"time"
Expand Down Expand Up @@ -125,7 +126,15 @@ func (router *CurrentSessionRouter) ListCertificates(ae *env.AppEnv, rc *respons
func (router *CurrentSessionRouter) CreateCertificate(ae *env.AppEnv, rc *response.RequestContext, params clientCurrentApiSession.CreateCurrentAPISessionCertificateParams) {
responder := &ApiSessionCertificateCreateResponder{ae: ae, Responder: rc}
CreateWithResponder(rc, responder, CurrentApiSessionCertificateLinkFactory, func() (string, error) {
return ae.GetManagers().ApiSessionCertificate.CreateFromCSR(rc.ApiSession.Id, 12*time.Hour, []byte(*params.SessionCertificate.Csr), rc.NewChangeContext())
newApiSessionCert, err := ae.GetManagers().ApiSessionCertificate.CreateFromCSR(rc.Identity, rc.ApiSession, rc.IsJwtToken, 12*time.Hour, []byte(*params.SessionCertificate.Csr), rc.NewChangeContext())

if err != nil {
return "", err
}

responder.ApiSessionCertificate = newApiSessionCert

return newApiSessionCert.Id, nil
})
}

Expand Down Expand Up @@ -191,18 +200,18 @@ func (router *CurrentSessionRouter) ListServiceUpdates(ae *env.AppEnv, rc *respo

type ApiSessionCertificateCreateResponder struct {
response.Responder
ae *env.AppEnv
ApiSessionCertificate *model.ApiSessionCertificate
ae *env.AppEnv
}

func (nsr *ApiSessionCertificateCreateResponder) RespondWithCreatedId(id string, _ rest_model.Link) {
sessionCert, _ := nsr.ae.GetManagers().ApiSessionCertificate.Read(id)
certString := sessionCert.PEM
certString := nsr.ApiSessionCertificate.PEM

newSessionEnvelope := &rest_model.CreateCurrentAPISessionCertificateEnvelope{
Data: &rest_model.CurrentAPISessionCertificateCreateResponse{
CreateLocation: rest_model.CreateLocation{
Links: CurrentApiSessionCertificateLinkFactory.Links(sessionCert),
ID: sessionCert.Id,
Links: CurrentApiSessionCertificateLinkFactory.Links(nsr.ApiSessionCertificate),
ID: nsr.ApiSessionCertificate.Id,
},
Certificate: &certString,
Cas: string(nsr.ae.GetConfig().Edge.CaPems()),
Expand Down
10 changes: 6 additions & 4 deletions controller/internal/routes/current_identity_router.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,11 +144,13 @@ func (r *CurrentIdentityRouter) verifyMfa(ae *env.AppEnv, rc *response.RequestCo
return
}

err = ae.Managers.ApiSession.SetMfaPassed(rc.ApiSession, changeCtx)
if !rc.IsJwtToken {
err = ae.Managers.ApiSession.SetMfaPassed(rc.ApiSession, changeCtx)

if err != nil {
rc.RespondWithError(err)
return
if err != nil {
rc.RespondWithError(err)
return
}
}

rc.RespondWithEmptyOk()
Expand Down
40 changes: 32 additions & 8 deletions controller/model/api_session_certificate_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,15 @@ package model

import (
"crypto/x509"
"fmt"
"github.com/openziti/ziti/common/cert"
"github.com/openziti/ziti/common/eid"
"github.com/openziti/ziti/controller/apierror"
"github.com/openziti/ziti/controller/change"
"github.com/openziti/ziti/controller/db"
"github.com/openziti/ziti/controller/models"
"go.etcd.io/bbolt"
"net/url"
"time"
)

Expand All @@ -48,7 +51,7 @@ func (self *ApiSessionCertificateManager) Create(entity *ApiSessionCertificate,
return self.createEntity(entity, ctx.NewMutateContext())
}

func (self *ApiSessionCertificateManager) CreateFromCSR(apiSessionId string, lifespan time.Duration, csrPem []byte, ctx *change.Context) (string, error) {
func (self *ApiSessionCertificateManager) CreateFromCSR(identity *Identity, apiSession *ApiSession, isJwt bool, lifespan time.Duration, csrPem []byte, ctx *change.Context) (*ApiSessionCertificate, error) {
notBefore := time.Now()
notAfter := time.Now().Add(lifespan)

Expand All @@ -58,38 +61,59 @@ func (self *ApiSessionCertificateManager) CreateFromCSR(apiSessionId string, lif
apiErr := apierror.NewCouldNotProcessCsr()
apiErr.Cause = err
apiErr.AppendCause = true
return "", apiErr
return nil, apiErr
}

newId := eid.New()

trustDomain := self.env.GetHostController().GetConfig().SpiffeIdTrustDomain.Hostname()

Check failure on line 69 in controller/model/api_session_certificate_manager.go

View workflow job for this annotation

GitHub Actions / lint

self.env.GetHostController undefined (type Env has no field or method GetHostController)) (typecheck)

Check failure on line 69 in controller/model/api_session_certificate_manager.go

View workflow job for this annotation

GitHub Actions / lint

self.env.GetHostController undefined (type Env has no field or method GetHostController)) (typecheck)

Check failure on line 69 in controller/model/api_session_certificate_manager.go

View workflow job for this annotation

GitHub Actions / lint

self.env.GetHostController undefined (type Env has no field or method GetHostController)) (typecheck)

Check failure on line 69 in controller/model/api_session_certificate_manager.go

View workflow job for this annotation

GitHub Actions / Build Linux binaries

self.env.GetHostController undefined (type Env has no field or method GetHostController)

Check failure on line 69 in controller/model/api_session_certificate_manager.go

View workflow job for this annotation

GitHub Actions / Fablab Smoketest

self.env.GetHostController undefined (type Env has no field or method GetHostController)

Check failure on line 69 in controller/model/api_session_certificate_manager.go

View workflow job for this annotation

GitHub Actions / Build Mac OS binaries

self.env.GetHostController undefined (type Env has no field or method GetHostController)

Check failure on line 69 in controller/model/api_session_certificate_manager.go

View workflow job for this annotation

GitHub Actions / Run Unit and Integration Tests

self.env.GetHostController undefined (type Env has no field or method GetHostController)
spiffeId := &url.URL{
Scheme: "spiffe",
Host: trustDomain,
Path: fmt.Sprintf("identity/%s/apiSession/%s/apiSessionCertificate/%s", identity.Id, apiSession.Id, newId),
}

certRaw, err := self.env.GetApiClientCsrSigner().SignCsr(csr, &cert.SigningOpts{
NotAfter: &notAfter,
NotBefore: &notBefore,
URIs: []*url.URL{
spiffeId,
},
})

if err != nil {
apiErr := apierror.NewCouldNotProcessCsr()
apiErr.Cause = err
apiErr.AppendCause = true
return "", apiErr
return nil, apiErr
}

fp := self.env.GetFingerprintGenerator().FromRaw(certRaw)

certPem, _ := cert.RawToPem(certRaw)

cert, _ := x509.ParseCertificate(certRaw)
newCert, _ := x509.ParseCertificate(certRaw)

entity := &ApiSessionCertificate{
BaseEntity: models.BaseEntity{},
ApiSessionId: apiSessionId,
Subject: cert.Subject.String(),
BaseEntity: models.BaseEntity{
Id: newId,
},
ApiSessionId: apiSession.Id,
Subject: newCert.Subject.String(),
Fingerprint: fp,
ValidAfter: &notBefore,
ValidBefore: &notAfter,
PEM: string(certPem),
}

return self.Create(entity, ctx)
if isJwt {
// can't create if using bearer tokens, the API Session will not exist
return entity, nil
}

entity.Id, err = self.Create(entity, ctx)

return entity, err
}

func (self *ApiSessionCertificateManager) IsUpdated(_ string) bool {
Expand Down
1 change: 1 addition & 0 deletions controller/response/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"github.com/openziti/ziti/common"
"github.com/openziti/ziti/controller/change"
"github.com/openziti/ziti/controller/model"

Check failure on line 25 in controller/response/context.go

View workflow job for this annotation

GitHub Actions / lint

could not import github.com/openziti/ziti/controller/model (-: # github.com/openziti/ziti/controller/model

"net/http"
"time"
)
Expand Down
87 changes: 70 additions & 17 deletions router/xgress_edge/connections.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,17 @@
package xgress_edge

import (
"crypto/x509"
"errors"
"fmt"
"github.com/michaelquigley/pfxlog"
"github.com/openziti/channel/v2"
"github.com/openziti/metrics"
"github.com/openziti/sdk-golang/ziti/edge"
"github.com/openziti/ziti/common/cert"
"github.com/openziti/ziti/common/spiffehlp"
"github.com/openziti/ziti/router/state"
"strings"
)

type sessionConnectionHandler struct {
Expand Down Expand Up @@ -61,7 +64,7 @@ func (handler *sessionConnectionHandler) BindChannel(binding channel.Binding, ed
}

fpg := cert.NewFingerprintGenerator()
fingerprints := fpg.FromCerts(certificates)
fingerprint := fpg.FromCert(certificates[0])

token := string(byteToken)

Expand All @@ -81,38 +84,52 @@ func (handler *sessionConnectionHandler) BindChannel(binding channel.Binding, ed
handler.invalidApiSessionTokenDuringSync.Mark(1)
}

return fmt.Errorf("no api session found for token [%s], fingerprints: [%v], subjects [%v]", token, fingerprints, subjects)
return fmt.Errorf("no api session found for token [%s], fingerprint: [%v], subjects [%v]", token, fingerprint, subjects)
}

edgeConn.apiSession = apiSession

if apiSession.Claims != nil {
token = apiSession.Claims.ApiSessionId
isValid := handler.validateBySpiffeId(apiSession, certificates[0])

if !isValid {
isValid = handler.validateByFingerprint(apiSession, fingerprint)
}

for _, fingerprint := range apiSession.CertFingerprints {
if fingerprints.Contains(fingerprint) {
removeListener := handler.stateManager.AddApiSessionRemovedListener(token, func(token string) {
if !ch.IsClosed() {
if err := ch.Close(); err != nil {
pfxlog.Logger().WithError(err).Error("could not close channel during api session removal")
}
if isValid {
if apiSession.Claims != nil {
token = apiSession.Claims.ApiSessionId
}

removeListener := handler.stateManager.AddApiSessionRemovedListener(token, func(token string) {
if !ch.IsClosed() {
if err := ch.Close(); err != nil {
pfxlog.Logger().WithError(err).Error("could not close channel during api session removal")
}
}

handler.stateManager.RemoveActiveChannel(ch)
})
handler.stateManager.RemoveActiveChannel(ch)
})

handler.stateManager.AddActiveChannel(ch, apiSession)
handler.stateManager.AddConnectedApiSessionWithChannel(token, removeListener, ch)
handler.stateManager.AddActiveChannel(ch, apiSession)
handler.stateManager.AddConnectedApiSessionWithChannel(token, removeListener, ch)

return nil
}
return nil
}

_ = ch.Close()
return errors.New("invalid client certificate for api session")
}

func (handler *sessionConnectionHandler) validateByFingerprint(apiSession *state.ApiSession, clientFingerprint string) bool {
for _, fingerprint := range apiSession.CertFingerprints {
if clientFingerprint == fingerprint {
return true
}
}

return false
}

func (handler *sessionConnectionHandler) HandleClose(ch channel.Channel) {
token := ""
if byteToken, ok := ch.Underlay().Headers()[edge.SessionTokenHeader]; ok {
Expand All @@ -125,3 +142,39 @@ func (handler *sessionConnectionHandler) HandleClose(ch channel.Channel) {
Error("session connection handler encountered a HandleClose that did not have a SessionTokenHeader")
}
}

func (handler *sessionConnectionHandler) validateBySpiffeId(apiSession *state.ApiSession, clientCert *x509.Certificate) bool {
spiffeId, err := spiffehlp.GetSpiffeIdFromCert(clientCert)

if err != nil {
return false
}

if spiffeId == nil {
return false
}

parts := strings.Split(spiffeId.Path, "/")

if len(parts) != 6 {
return false
}

if parts[0] != "identity" {
return false
}

if parts[2] != "apiSession" {
return false
}

if parts[4] != "apiSessionCertificate" {
return false
}

if apiSession.Id == parts[3] {
return true
}

return false
}
Loading

0 comments on commit 5027c04

Please sign in to comment.