Add Expires header to CRL endpoint

api/api.go

@@ -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.

api/api_test.go

@@ -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)
@@ -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.
@@ -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

api/crl.go

@@ -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)
 	}
 }

api/crl_test.go

@@ -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))
+		})
+	}
+}

authority/tls.go

@@ -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")
 	}
@@ -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