Skip to content

Commit

Permalink
Merge pull request #1708 from smallstep/herman/csr-expires-header
Browse files Browse the repository at this point in the history
Add `Expires` header to CRL endpoint
  • Loading branch information
hslatman authored Feb 15, 2024
2 parents bd99db0 + 3dbb4aa commit bb296c9
Show file tree
Hide file tree
Showing 6 changed files with 306 additions and 225 deletions.
2 changes: 1 addition & 1 deletion api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ type Authority interface {
GetRoots() ([]*x509.Certificate, error)
GetFederation() ([]*x509.Certificate, error)
Version() authority.Version
GetCertificateRevocationList() ([]byte, error)
GetCertificateRevocationList() (*authority.CertificateRevocationListInfo, error)
}

// mustAuthority will be replaced on unit tests.
Expand Down
45 changes: 3 additions & 42 deletions api/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ type mockAuthority struct {
getEncryptedKey func(kid string) (string, error)
getRoots func() ([]*x509.Certificate, error)
getFederation func() ([]*x509.Certificate, error)
getCRL func() ([]byte, error)
getCRL func() (*authority.CertificateRevocationListInfo, error)
signSSH func(ctx context.Context, key ssh.PublicKey, opts provisioner.SignSSHOptions, signOpts ...provisioner.SignOption) (*ssh.Certificate, error)
signSSHAddUser func(ctx context.Context, key ssh.PublicKey, cert *ssh.Certificate) (*ssh.Certificate, error)
renewSSH func(ctx context.Context, cert *ssh.Certificate) (*ssh.Certificate, error)
Expand All @@ -214,12 +214,12 @@ type mockAuthority struct {
version func() authority.Version
}

func (m *mockAuthority) GetCertificateRevocationList() ([]byte, error) {
func (m *mockAuthority) GetCertificateRevocationList() (*authority.CertificateRevocationListInfo, error) {
if m.getCRL != nil {
return m.getCRL()
}

return m.ret1.([]byte), m.err
return m.ret1.(*authority.CertificateRevocationListInfo), m.err
}

// TODO: remove once Authorize is deprecated.
Expand Down Expand Up @@ -789,45 +789,6 @@ func (m *mockProvisioner) AuthorizeSSHRekey(ctx context.Context, token string) (
return m.ret1.(*ssh.Certificate), m.ret2.([]provisioner.SignOption), m.err
}

func Test_CRLGeneration(t *testing.T) {
tests := []struct {
name string
err error
statusCode int
expected []byte
}{
{"empty", nil, http.StatusOK, nil},
}

chiCtx := chi.NewRouteContext()
req := httptest.NewRequest("GET", "http://example.com/crl", http.NoBody)
req = req.WithContext(context.WithValue(context.Background(), chi.RouteCtxKey, chiCtx))

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
mockMustAuthority(t, &mockAuthority{ret1: tt.expected, err: tt.err})
w := httptest.NewRecorder()
CRL(w, req)
res := w.Result()

if res.StatusCode != tt.statusCode {
t.Errorf("caHandler.CRL StatusCode = %d, wants %d", res.StatusCode, tt.statusCode)
}

body, err := io.ReadAll(res.Body)
res.Body.Close()
if err != nil {
t.Errorf("caHandler.Root unexpected error = %v", err)
}
if tt.statusCode == 200 {
if !bytes.Equal(bytes.TrimSpace(body), tt.expected) {
t.Errorf("caHandler.Root CRL = %s, wants %s", body, tt.expected)
}
}
})
}
}

func Test_caHandler_Route(t *testing.T) {
type fields struct {
Authority Authority
Expand Down
20 changes: 17 additions & 3 deletions api/crl.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,30 +3,44 @@ package api
import (
"encoding/pem"
"net/http"
"time"

"github.com/smallstep/certificates/api/render"
"github.com/smallstep/certificates/errs"
)

// CRL is an HTTP handler that returns the current CRL in DER or PEM format
func CRL(w http.ResponseWriter, r *http.Request) {
crlBytes, err := mustAuthority(r.Context()).GetCertificateRevocationList()
crlInfo, err := mustAuthority(r.Context()).GetCertificateRevocationList()
if err != nil {
render.Error(w, err)
return
}

if crlInfo == nil {
render.Error(w, errs.New(http.StatusNotFound, "no CRL available"))
return
}

expires := crlInfo.ExpiresAt
if expires.IsZero() {
expires = time.Now()
}

w.Header().Add("Expires", expires.Format(time.RFC1123))

_, formatAsPEM := r.URL.Query()["pem"]
if formatAsPEM {
w.Header().Add("Content-Type", "application/x-pem-file")
w.Header().Add("Content-Disposition", "attachment; filename=\"crl.pem\"")

_ = pem.Encode(w, &pem.Block{
Type: "X509 CRL",
Bytes: crlBytes,
Bytes: crlInfo.Data,
})
} else {
w.Header().Add("Content-Type", "application/pkix-crl")
w.Header().Add("Content-Disposition", "attachment; filename=\"crl.der\"")
w.Write(crlBytes)
w.Write(crlInfo.Data)
}
}
93 changes: 93 additions & 0 deletions api/crl_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
package api

import (
"bytes"
"context"
"encoding/pem"
"io"
"net/http"
"net/http/httptest"
"testing"
"time"

"github.com/go-chi/chi/v5"
"github.com/pkg/errors"
"github.com/smallstep/certificates/authority"
"github.com/smallstep/certificates/errs"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func Test_CRL(t *testing.T) {
data := []byte{1, 2, 3, 4}
pemData := pem.EncodeToMemory(&pem.Block{
Type: "X509 CRL",
Bytes: data,
})
pemData = bytes.TrimSpace(pemData)
emptyPEMData := pem.EncodeToMemory(&pem.Block{
Type: "X509 CRL",
Bytes: nil,
})
emptyPEMData = bytes.TrimSpace(emptyPEMData)
tests := []struct {
name string
url string
err error
statusCode int
crlInfo *authority.CertificateRevocationListInfo
expectedBody []byte
expectedHeaders http.Header
expectedErrorJSON string
}{
{"ok", "http://example.com/crl", nil, http.StatusOK, &authority.CertificateRevocationListInfo{Data: data}, data, http.Header{"Content-Type": []string{"application/pkix-crl"}, "Content-Disposition": []string{`attachment; filename="crl.der"`}}, ""},
{"ok/pem", "http://example.com/crl?pem=true", nil, http.StatusOK, &authority.CertificateRevocationListInfo{Data: data}, pemData, http.Header{"Content-Type": []string{"application/x-pem-file"}, "Content-Disposition": []string{`attachment; filename="crl.pem"`}}, ""},
{"ok/empty", "http://example.com/crl", nil, http.StatusOK, &authority.CertificateRevocationListInfo{Data: nil}, nil, http.Header{"Content-Type": []string{"application/pkix-crl"}, "Content-Disposition": []string{`attachment; filename="crl.der"`}}, ""},
{"ok/empty-pem", "http://example.com/crl?pem=true", nil, http.StatusOK, &authority.CertificateRevocationListInfo{Data: nil}, emptyPEMData, http.Header{"Content-Type": []string{"application/x-pem-file"}, "Content-Disposition": []string{`attachment; filename="crl.pem"`}}, ""},
{"fail/internal", "http://example.com/crl", errs.Wrap(http.StatusInternalServerError, errors.New("failure"), "authority.GetCertificateRevocationList"), http.StatusInternalServerError, nil, nil, http.Header{}, `{"status":500,"message":"The certificate authority encountered an Internal Server Error. Please see the certificate authority logs for more info."}`},
{"fail/nil", "http://example.com/crl", nil, http.StatusNotFound, nil, nil, http.Header{}, `{"status":404,"message":"no CRL available"}`},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
mockMustAuthority(t, &mockAuthority{ret1: tt.crlInfo, err: tt.err})

chiCtx := chi.NewRouteContext()
req := httptest.NewRequest("GET", tt.url, http.NoBody)
req = req.WithContext(context.WithValue(context.Background(), chi.RouteCtxKey, chiCtx))
w := httptest.NewRecorder()
CRL(w, req)
res := w.Result()

assert.Equal(t, tt.statusCode, res.StatusCode)

body, err := io.ReadAll(res.Body)
res.Body.Close()
require.NoError(t, err)

if tt.statusCode >= 300 {
assert.JSONEq(t, tt.expectedErrorJSON, string(bytes.TrimSpace(body)))
return
}

// check expected header values
for _, h := range []string{"content-type", "content-disposition"} {
v := tt.expectedHeaders.Get(h)
require.NotEmpty(t, v)

actual := res.Header.Get(h)
assert.Equal(t, v, actual)
}

// check expires header value
assert.NotEmpty(t, res.Header.Get("expires"))
t1, err := time.Parse(time.RFC1123, res.Header.Get("expires"))
if assert.NoError(t, err) {
assert.False(t, t1.IsZero())
}

// check body contents
assert.Equal(t, tt.expectedBody, bytes.TrimSpace(body))
})
}
}
17 changes: 15 additions & 2 deletions authority/tls.go
Original file line number Diff line number Diff line change
Expand Up @@ -696,9 +696,17 @@ func (a *Authority) revokeSSH(crt *ssh.Certificate, rci *db.RevokedCertificateIn
return a.db.RevokeSSH(rci)
}

// CertificateRevocationListInfo contains a CRL in DER format and associated metadata.
type CertificateRevocationListInfo struct {
Number int64
ExpiresAt time.Time
Duration time.Duration
Data []byte
}

// GetCertificateRevocationList will return the currently generated CRL from the DB, or a not implemented
// error if the underlying AuthDB does not support CRLs
func (a *Authority) GetCertificateRevocationList() ([]byte, error) {
func (a *Authority) GetCertificateRevocationList() (*CertificateRevocationListInfo, error) {
if !a.config.CRL.IsEnabled() {
return nil, errs.Wrap(http.StatusNotFound, errors.Errorf("Certificate Revocation Lists are not enabled"), "authority.GetCertificateRevocationList")
}
Expand All @@ -713,7 +721,12 @@ func (a *Authority) GetCertificateRevocationList() ([]byte, error) {
return nil, errs.Wrap(http.StatusInternalServerError, err, "authority.GetCertificateRevocationList")
}

return crlInfo.DER, nil
return &CertificateRevocationListInfo{
Number: crlInfo.Number,
ExpiresAt: crlInfo.ExpiresAt,
Duration: crlInfo.Duration,
Data: crlInfo.DER,
}, nil
}

// GenerateCertificateRevocationList generates a DER representation of a signed CRL and stores it in the
Expand Down
Loading

0 comments on commit bb296c9

Please sign in to comment.