Add early authority policy evaluation to ACME order API

This commit is contained in:
Herman Slatman 2022-03-24 14:55:40 +01:00
parent 613c99f00f
commit 9e0edc7b50
No known key found for this signature in database
GPG key ID: F4D8A44EA0A75A4F
10 changed files with 54 additions and 13 deletions

View file

@ -73,4 +73,3 @@ issues:
- error strings should not be capitalized or end with punctuation or a newline
- Wrapf call needs 1 arg but has 2 args
- cs.NegotiatedProtocolIsMutual is deprecated
- rewrite if-else to switch statement

View file

@ -4,8 +4,9 @@ import (
"context"
"encoding/json"
"github.com/smallstep/certificates/acme"
"go.step.sm/crypto/jose"
"github.com/smallstep/certificates/acme"
)
// ExternalAccountBinding represents the ACME externalAccountBinding JWS

View file

@ -9,10 +9,12 @@ import (
"time"
"github.com/pkg/errors"
"go.step.sm/crypto/jose"
"github.com/smallstep/assert"
"github.com/smallstep/certificates/acme"
"github.com/smallstep/certificates/authority/provisioner"
"go.step.sm/crypto/jose"
)
func Test_keysAreEqual(t *testing.T) {

View file

@ -11,10 +11,12 @@ import (
"time"
"github.com/go-chi/chi"
"go.step.sm/crypto/randutil"
"github.com/smallstep/certificates/acme"
"github.com/smallstep/certificates/api"
"github.com/smallstep/certificates/authority/provisioner"
"go.step.sm/crypto/randutil"
)
// NewOrderRequest represents the body for a NewOrder request.
@ -36,8 +38,8 @@ func (n *NewOrderRequest) Validate() error {
if id.Type == acme.IP && net.ParseIP(id.Value) == nil {
return acme.NewError(acme.ErrorMalformedType, "invalid IP address: %s", id.Value)
}
// TODO: add some validations for DNS domains?
// TODO: combine the errors from this with allow/deny policy, like example error in https://datatracker.ietf.org/doc/html/rfc8555#section-6.7.1
// TODO(hs): add some validations for DNS domains?
// TODO(hs): combine the errors from this with allow/deny policy, like example error in https://datatracker.ietf.org/doc/html/rfc8555#section-6.7.1
}
return nil
}
@ -99,23 +101,29 @@ func (h *Handler) NewOrder(w http.ResponseWriter, r *http.Request) {
return
}
// TODO(hs): this should also verify rules set in the Account (i.e. allowed/denied
// DNS and IPs; it's probably good to connect those to the EAB credentials and management? Or
// TODO(hs): the policy evaluation below should also verify rules set in the Account (i.e. allowed/denied
// DNS and IPs). It's probably good to connect those to the EAB credentials and management? Or
// should we do it fully properly and connect them to the Account directly? The latter would allow
// management of allowed/denied names based on just the name, without having bound to EAB. Still,
// EAB is not illogical, because that's the way Accounts are connected to an external system and
// thus make sense to also set the allowed/denied names based on that info.
// TODO: also perform check on the authority level here already, so that challenges are not performed
// and after that the CA fails to sign it. (i.e. h.ca function?)
// TODO(hs): gather all errors, so that we can build one response with subproblems; include the nor.Validate()
// error here too, like in example?
for _, identifier := range nor.Identifiers {
// TODO: gather all errors, so that we can build subproblems; include the nor.Validate() error here too, like in example?
// evaluate the provisioner level policy
orderIdentifier := provisioner.ACMEIdentifier{Type: provisioner.ACMEIdentifierType(identifier.Type), Value: identifier.Value}
err = prov.AuthorizeOrderIdentifier(ctx, orderIdentifier)
if err != nil {
api.WriteError(w, acme.WrapError(acme.ErrorRejectedIdentifierType, err, "not authorized"))
return
}
// evaluate the authority level policy
err = h.ca.AreSANsAllowed(ctx, []string{identifier.Value})
if err != nil {
api.WriteError(w, acme.WrapError(acme.ErrorRejectedIdentifierType, err, "not authorized"))
return
}
}
now := clock.Now()

View file

@ -282,6 +282,10 @@ func (m *mockCA) Sign(cr *x509.CertificateRequest, opts provisioner.SignOptions,
return nil, nil
}
func (m *mockCA) AreSANsAllowed(ctx context.Context, sans []string) error {
return nil
}
func (m *mockCA) IsRevoked(sn string) (bool, error) {
if m.MockIsRevoked != nil {
return m.MockIsRevoked(sn)

View file

@ -12,6 +12,7 @@ import (
// CertificateAuthority is the interface implemented by a CA authority.
type CertificateAuthority interface {
Sign(cr *x509.CertificateRequest, opts provisioner.SignOptions, signOpts ...provisioner.SignOption) ([]*x509.Certificate, error)
AreSANsAllowed(ctx context.Context, sans []string) error
IsRevoked(sn string) (bool, error)
Revoke(context.Context, *authority.RevokeOptions) error
LoadProvisionerByName(string) (provisioner.Interface, error)

View file

@ -268,6 +268,7 @@ func TestOrder_UpdateStatus(t *testing.T) {
type mockSignAuth struct {
sign func(csr *x509.CertificateRequest, signOpts provisioner.SignOptions, extraOpts ...provisioner.SignOption) ([]*x509.Certificate, error)
areSANsAllowed func(ctx context.Context, sans []string) error
loadProvisionerByName func(string) (provisioner.Interface, error)
ret1, ret2 interface{}
err error
@ -282,6 +283,13 @@ func (m *mockSignAuth) Sign(csr *x509.CertificateRequest, signOpts provisioner.S
return []*x509.Certificate{m.ret1.(*x509.Certificate), m.ret2.(*x509.Certificate)}, m.err
}
func (m *mockSignAuth) AreSANsAllowed(ctx context.Context, sans []string) error {
if m.areSANsAllowed != nil {
return m.areSANsAllowed(ctx, sans)
}
return m.err
}
func (m *mockSignAuth) LoadProvisionerByName(name string) (provisioner.Interface, error) {
if m.loadProvisionerByName != nil {
return m.loadProvisionerByName(name)

View file

@ -2,10 +2,9 @@ package authority
import (
"context"
"errors"
"fmt"
"github.com/pkg/errors"
"go.step.sm/linkedca"
"github.com/smallstep/certificates/authority/admin"

View file

@ -3,6 +3,7 @@ package provisioner
import (
"context"
"crypto/x509"
"fmt"
"net"
"time"
@ -126,6 +127,8 @@ func (p *ACME) AuthorizeOrderIdentifier(ctx context.Context, identifier ACMEIden
_, err = p.x509Policy.IsIPAllowed(net.ParseIP(identifier.Value))
case DNS:
_, err = p.x509Policy.IsDNSAllowed(identifier.Value)
default:
err = fmt.Errorf("invalid ACME identifier type '%s' provided", identifier.Type)
}
return err

View file

@ -245,6 +245,22 @@ func (a *Authority) isAllowedToSign(cert *x509.Certificate) (bool, error) {
return a.x509Policy.AreCertificateNamesAllowed(cert)
}
// AreSANsAllowed evaluates the provided sans against the
// authority X.509 policy.
func (a *Authority) AreSANsAllowed(ctx context.Context, sans []string) error {
// no policy configured; return early
if a.x509Policy == nil {
return nil
}
// evaluate the X.509 policy for the provided sans
var err error
_, err = a.x509Policy.AreSANsAllowed(sans)
return err
}
// Renew creates a new Certificate identical to the old certificate, except
// with a validity window that begins 'now'.
func (a *Authority) Renew(oldCert *x509.Certificate) ([]*x509.Certificate, error) {