Skip to content

Commit

Permalink
LAPI: support CRL files with multiple PEM blocks (crowdsecurity#3002)
Browse files Browse the repository at this point in the history
  • Loading branch information
@mmetc
mmetc authored May 13, 2024
1 parent e4a8d3b commit e3c6a5b
Show file tree
Hide file tree
Showing 3 changed files with 108 additions and 72 deletions.
63 changes: 33 additions & 30 deletions pkg/apiserver/middlewares/v1/tls_auth.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"crypto"
"crypto/x509"
"encoding/pem"
"errors"
"fmt"
"io"
"net/http"
Expand Down Expand Up @@ -135,31 +136,35 @@ func (ta *TLSAuth) isCRLRevoked(cert *x509.Certificate) (bool, bool) {
return false, false
}

crlBinary, rest := pem.Decode(crlContent)
if len(rest) > 0 {
ta.logger.Warn("CRL file contains more than one PEM block, ignoring the rest")
}
var crlBlock *pem.Block

crl, err := x509.ParseRevocationList(crlBinary.Bytes)
if err != nil {
ta.logger.Errorf("could not parse CRL file, skipping check: %s", err)
return false, false
}
for {
crlBlock, crlContent = pem.Decode(crlContent)
if crlBlock == nil {
break // no more PEM blocks
}

now := time.Now().UTC()
crl, err := x509.ParseRevocationList(crlBlock.Bytes)
if err != nil {
ta.logger.Errorf("could not parse a PEM block in CRL file, skipping: %s", err)
continue
}

if now.After(crl.NextUpdate) {
ta.logger.Warn("CRL has expired, will still validate the cert against it.")
}
now := time.Now().UTC()

if now.Before(crl.ThisUpdate) {
ta.logger.Warn("CRL is not yet valid, will still validate the cert against it.")
}
if now.After(crl.NextUpdate) {
ta.logger.Warn("CRL has expired, will still validate the cert against it.")
}

for _, revoked := range crl.RevokedCertificateEntries {
if revoked.SerialNumber.Cmp(cert.SerialNumber) == 0 {
ta.logger.Warn("client certificate is revoked by CRL")
return true, true
if now.Before(crl.ThisUpdate) {
ta.logger.Warn("CRL is not yet valid, will still validate the cert against it.")
}

for _, revoked := range crl.RevokedCertificateEntries {
if revoked.SerialNumber.Cmp(cert.SerialNumber) == 0 {
ta.logger.Warn("client certificate is revoked by CRL")
return true, true
}
}
}

Expand All @@ -181,9 +186,7 @@ func (ta *TLSAuth) isRevoked(cert *x509.Certificate, issuer *x509.Certificate) (
}

revokedByOCSP, cacheOCSP := ta.isOCSPRevoked(cert, issuer)

revokedByCRL, cacheCRL := ta.isCRLRevoked(cert)

revoked := revokedByOCSP || revokedByCRL

if cacheOCSP && cacheCRL {
Expand All @@ -203,8 +206,8 @@ func (ta *TLSAuth) isInvalid(cert *x509.Certificate, issuer *x509.Certificate) (

revoked, err := ta.isRevoked(cert, issuer)
if err != nil {
//Fail securely, if we can't check the revocation status, let's consider the cert invalid
//We may change this in the future based on users feedback, but this seems the most sensible thing to do
// Fail securely, if we can't check the revocation status, let's consider the cert invalid
// We may change this in the future based on users feedback, but this seems the most sensible thing to do
return true, fmt.Errorf("could not check for client certification revocation status: %w", err)
}

Expand All @@ -213,12 +216,12 @@ func (ta *TLSAuth) isInvalid(cert *x509.Certificate, issuer *x509.Certificate) (

func (ta *TLSAuth) SetAllowedOu(allowedOus []string) error {
for _, ou := range allowedOus {
//disallow empty ou
// disallow empty ou
if ou == "" {
return fmt.Errorf("empty ou isn't allowed")
return errors.New("empty ou isn't allowed")
}

//drop & warn on duplicate ou
// drop & warn on duplicate ou
ok := true

for _, validOu := range ta.AllowedOUs {
Expand All @@ -238,11 +241,11 @@ func (ta *TLSAuth) SetAllowedOu(allowedOus []string) error {
}

func (ta *TLSAuth) ValidateCert(c *gin.Context) (bool, string, error) {
//Checks cert validity, Returns true + CN if client cert matches requested OU
// Checks cert validity, Returns true + CN if client cert matches requested OU
var clientCert *x509.Certificate

if c.Request.TLS == nil || len(c.Request.TLS.PeerCertificates) == 0 {
//do not error if it's not TLS or there are no peer certs
// do not error if it's not TLS or there are no peer certs
return false, "", nil
}

Expand Down Expand Up @@ -279,7 +282,7 @@ func (ta *TLSAuth) ValidateCert(c *gin.Context) (bool, string, error) {
return true, clientCert.Subject.CommonName, nil
}

return false, "", fmt.Errorf("no verified cert in request")
return false, "", errors.New("no verified cert in request")
}

func NewTLSAuth(allowedOus []string, crlPath string, cacheExpiration time.Duration, logger *log.Entry) (*TLSAuth, error) {
Expand Down
50 changes: 33 additions & 17 deletions test/bats/11_bouncers_tls.bats
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,24 +13,37 @@ setup_file() {
CFDIR="${BATS_TEST_DIRNAME}/testdata/cfssl"
export CFDIR

#gen the CA
# Generate the CA
cfssl gencert --initca "${CFDIR}/ca.json" 2>/dev/null | cfssljson --bare "${tmpdir}/ca"
#gen an intermediate

# Generate an intermediate
cfssl gencert --initca "${CFDIR}/intermediate.json" 2>/dev/null | cfssljson --bare "${tmpdir}/inter"
cfssl sign -ca "${tmpdir}/ca.pem" -ca-key "${tmpdir}/ca-key.pem" -config "${CFDIR}/profiles.json" -profile intermediate_ca "${tmpdir}/inter.csr" 2>/dev/null | cfssljson --bare "${tmpdir}/inter"
#gen server cert for crowdsec with the intermediate

# Generate server cert for crowdsec with the intermediate
cfssl gencert -ca "${tmpdir}/inter.pem" -ca-key "${tmpdir}/inter-key.pem" -config "${CFDIR}/profiles.json" -profile=server "${CFDIR}/server.json" 2>/dev/null | cfssljson --bare "${tmpdir}/server"
#gen client cert for the bouncer

# Generate client cert for the bouncer
cfssl gencert -ca "${tmpdir}/inter.pem" -ca-key "${tmpdir}/inter-key.pem" -config "${CFDIR}/profiles.json" -profile=client "${CFDIR}/bouncer.json" 2>/dev/null | cfssljson --bare "${tmpdir}/bouncer"
#gen client cert for the bouncer with an invalid OU

# Genearte client cert for the bouncer with an invalid OU
cfssl gencert -ca "${tmpdir}/inter.pem" -ca-key "${tmpdir}/inter-key.pem" -config "${CFDIR}/profiles.json" -profile=client "${CFDIR}/bouncer_invalid.json" 2>/dev/null | cfssljson --bare "${tmpdir}/bouncer_bad_ou"
#gen client cert for the bouncer directly signed by the CA, it should be refused by crowdsec as uses the intermediate

# Generate client cert for the bouncer directly signed by the CA, it should be refused by crowdsec as uses the intermediate
cfssl gencert -ca "${tmpdir}/ca.pem" -ca-key "${tmpdir}/ca-key.pem" -config "${CFDIR}/profiles.json" -profile=client "${CFDIR}/bouncer.json" 2>/dev/null | cfssljson --bare "${tmpdir}/bouncer_invalid"

cfssl gencert -ca "${tmpdir}/inter.pem" -ca-key "${tmpdir}/inter-key.pem" -config "${CFDIR}/profiles.json" -profile=client "${CFDIR}/bouncer.json" 2>/dev/null | cfssljson --bare "${tmpdir}/bouncer_revoked"
serial="$(openssl x509 -noout -serial -in "${tmpdir}/bouncer_revoked.pem" | cut -d '=' -f2)"
echo "ibase=16; ${serial}" | bc >"${tmpdir}/serials.txt"
cfssl gencrl "${tmpdir}/serials.txt" "${tmpdir}/ca.pem" "${tmpdir}/ca-key.pem" | base64 -d | openssl crl -inform DER -out "${tmpdir}/crl.pem"
# Generate revoked client certs
for cert_name in "revoked_1" "revoked_2"; do
cfssl gencert -ca "${tmpdir}/inter.pem" -ca-key "${tmpdir}/inter-key.pem" -config "${CFDIR}/profiles.json" -profile=client "${CFDIR}/bouncer.json" 2>/dev/null | cfssljson --bare "${tmpdir}/${cert_name}"
serial="$(openssl x509 -noout -serial -in "${tmpdir}/${cert_name}.pem" | cut -d '=' -f2)"
echo "ibase=16; ${serial}" | bc >"${tmpdir}/serials_${cert_name}.txt"
done

# Generate separate CRL blocks and concatenate them
for cert_name in "revoked_1" "revoked_2"; do
cfssl gencrl "${tmpdir}/serials_${cert_name}.txt" "${tmpdir}/ca.pem" "${tmpdir}/ca-key.pem" | base64 -d | openssl crl -inform DER -out "${tmpdir}/crl_${cert_name}.pem"
done
cat "${tmpdir}/crl_revoked_1.pem" "${tmpdir}/crl_revoked_2.pem" >"${tmpdir}/crl.pem"

cat "${tmpdir}/ca.pem" "${tmpdir}/inter.pem" > "${tmpdir}/bundle.pem"

Expand Down Expand Up @@ -90,11 +103,14 @@ teardown() {
}

@test "simulate one bouncer request with a revoked certificate" {
truncate_log
rune -0 curl -i -s --cert "${tmpdir}/bouncer_revoked.pem" --key "${tmpdir}/bouncer_revoked-key.pem" --cacert "${tmpdir}/bundle.pem" https://localhost:8080/v1/decisions\?ip=42.42.42.42
assert_log --partial "client certificate is revoked by CRL"
assert_log --partial "client certificate for CN=localhost OU=[bouncer-ou] is revoked"
assert_output --partial "access forbidden"
rune -0 cscli bouncers list -o json
assert_output "[]"
# we have two certificates revoked by different CRL blocks
for cert_name in "revoked_1" "revoked_2"; do
truncate_log
rune -0 curl -i -s --cert "${tmpdir}/${cert_name}.pem" --key "${tmpdir}/${cert_name}-key.pem" --cacert "${tmpdir}/bundle.pem" https://localhost:8080/v1/decisions\?ip=42.42.42.42
assert_log --partial "client certificate is revoked by CRL"
assert_log --partial "client certificate for CN=localhost OU=[bouncer-ou] is revoked"
assert_output --partial "access forbidden"
rune -0 cscli bouncers list -o json
assert_output "[]"
done
}
67 changes: 42 additions & 25 deletions test/bats/30_machines_tls.bats
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,24 +16,37 @@ setup_file() {
CFDIR="${BATS_TEST_DIRNAME}/testdata/cfssl"
export CFDIR

#gen the CA
# Generate the CA
cfssl gencert --initca "${CFDIR}/ca.json" 2>/dev/null | cfssljson --bare "${tmpdir}/ca"
#gen an intermediate

# Generate an intermediate
cfssl gencert --initca "${CFDIR}/intermediate.json" 2>/dev/null | cfssljson --bare "${tmpdir}/inter"
cfssl sign -ca "${tmpdir}/ca.pem" -ca-key "${tmpdir}/ca-key.pem" -config "${CFDIR}/profiles.json" -profile intermediate_ca "${tmpdir}/inter.csr" 2>/dev/null | cfssljson --bare "${tmpdir}/inter"
#gen server cert for crowdsec with the intermediate

# Generate server cert for crowdsec with the intermediate
cfssl gencert -ca "${tmpdir}/inter.pem" -ca-key "${tmpdir}/inter-key.pem" -config "${CFDIR}/profiles.json" -profile=server "${CFDIR}/server.json" 2>/dev/null | cfssljson --bare "${tmpdir}/server"
#gen client cert for the agent

# Generate client cert for the agent
cfssl gencert -ca "${tmpdir}/inter.pem" -ca-key "${tmpdir}/inter-key.pem" -config "${CFDIR}/profiles.json" -profile=client "${CFDIR}/agent.json" 2>/dev/null | cfssljson --bare "${tmpdir}/agent"
#gen client cert for the agent with an invalid OU

# Genearte client cert for the agent with an invalid OU
cfssl gencert -ca "${tmpdir}/inter.pem" -ca-key "${tmpdir}/inter-key.pem" -config "${CFDIR}/profiles.json" -profile=client "${CFDIR}/agent_invalid.json" 2>/dev/null | cfssljson --bare "${tmpdir}/agent_bad_ou"
#gen client cert for the agent directly signed by the CA, it should be refused by crowdsec as uses the intermediate

# Generate client cert for the bouncer directly signed by the CA, it should be refused by crowdsec as uses the intermediate
cfssl gencert -ca "${tmpdir}/ca.pem" -ca-key "${tmpdir}/ca-key.pem" -config "${CFDIR}/profiles.json" -profile=client "${CFDIR}/agent.json" 2>/dev/null | cfssljson --bare "${tmpdir}/agent_invalid"

cfssl gencert -ca "${tmpdir}/inter.pem" -ca-key "${tmpdir}/inter-key.pem" -config "${CFDIR}/profiles.json" -profile=client "${CFDIR}/agent.json" 2>/dev/null | cfssljson --bare "${tmpdir}/agent_revoked"
serial="$(openssl x509 -noout -serial -in "${tmpdir}/agent_revoked.pem" | cut -d '=' -f2)"
echo "ibase=16; ${serial}" | bc >"${tmpdir}/serials.txt"
cfssl gencrl "${tmpdir}/serials.txt" "${tmpdir}/ca.pem" "${tmpdir}/ca-key.pem" | base64 -d | openssl crl -inform DER -out "${tmpdir}/crl.pem"
# Generate revoked client cert
for cert_name in "revoked_1" "revoked_2"; do
cfssl gencert -ca "${tmpdir}/inter.pem" -ca-key "${tmpdir}/inter-key.pem" -config "${CFDIR}/profiles.json" -profile=client "${CFDIR}/agent.json" 2>/dev/null | cfssljson --bare "${tmpdir}/${cert_name}"
serial="$(openssl x509 -noout -serial -in "${tmpdir}/${cert_name}.pem" | cut -d '=' -f2)"
echo "ibase=16; ${serial}" | bc >"${tmpdir}/serials_${cert_name}.txt"
done

# Generate separate CRL blocks and concatenate them
for cert_name in "revoked_1" "revoked_2"; do
cfssl gencrl "${tmpdir}/serials_${cert_name}.txt" "${tmpdir}/ca.pem" "${tmpdir}/ca-key.pem" | base64 -d | openssl crl -inform DER -out "${tmpdir}/crl_${cert_name}.pem"
done
cat "${tmpdir}/crl_revoked_1.pem" "${tmpdir}/crl_revoked_2.pem" >"${tmpdir}/crl.pem"

cat "${tmpdir}/ca.pem" "${tmpdir}/inter.pem" > "${tmpdir}/bundle.pem"

Expand Down Expand Up @@ -181,19 +194,23 @@ teardown() {
}

@test "revoked cert for agent" {
truncate_log
config_set "${CONFIG_DIR}/local_api_credentials.yaml" '
.ca_cert_path=strenv(tmpdir) + "/bundle.pem" |
.key_path=strenv(tmpdir) + "/agent_revoked-key.pem" |
.cert_path=strenv(tmpdir) + "/agent_revoked.pem" |
.url="https://127.0.0.1:8080"
'

config_set "${CONFIG_DIR}/local_api_credentials.yaml" 'del(.login,.password)'
./instance-crowdsec start
rune -1 cscli lapi status
assert_log --partial "client certificate is revoked by CRL"
assert_log --partial "client certificate for CN=localhost OU=[agent-ou] is revoked"
rune -0 cscli machines list -o json
assert_output '[]'
# we have two certificates revoked by different CRL blocks
for cert_name in "revoked_1" "revoked_2"; do
truncate_log
cert_name="$cert_name" config_set "${CONFIG_DIR}/local_api_credentials.yaml" '
.ca_cert_path=strenv(tmpdir) + "/bundle.pem" |
.key_path=strenv(tmpdir) + "/" + strenv(cert_name) + "-key.pem" |
.cert_path=strenv(tmpdir) + "/" + strenv(cert_name) + ".pem" |
.url="https://127.0.0.1:8080"
'

config_set "${CONFIG_DIR}/local_api_credentials.yaml" 'del(.login,.password)'
./instance-crowdsec start
rune -1 cscli lapi status
assert_log --partial "client certificate is revoked by CRL"
assert_log --partial "client certificate for CN=localhost OU=[agent-ou] is revoked"
rune -0 cscli machines list -o json
assert_output '[]'
./instance-crowdsec stop
done
}

0 comments on commit e3c6a5b

Please sign in to comment.