Skip to content

Revoke Certificates By Role [ISSUE-10733] #11175

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions builtin/logical/pki/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ func Backend(conf *logical.BackendConfig) *backend {
pathFetchValid(&b),
pathFetchListCerts(&b),
pathRevoke(&b),
pathRevokeByRole(&b),
pathTidy(&b),
},

Expand Down
36 changes: 34 additions & 2 deletions builtin/logical/pki/crl_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ type revocationInfo struct {
}

// Revokes a cert, and tries to be smart about error recovery
func revokeCert(ctx context.Context, b *backend, req *logical.Request, serial string, fromLease bool) (*logical.Response, error) {
func revokeCert(ctx context.Context, b *backend, req *logical.Request, serial string, roleName string, fromLease bool) (*logical.Response, error) {
// As this backend is self-contained and this function does not hook into
// third parties to manage users or resources, if the mount is tainted,
// revocation doesn't matter anyways -- the CRL that would be written will
Expand All @@ -45,7 +45,7 @@ func revokeCert(ctx context.Context, b *backend, req *logical.Request, serial st
}
colonSerial := strings.Replace(strings.ToLower(serial), "-", ":", -1)
if colonSerial == certutil.GetHexFormatted(signingBundle.Certificate.SerialNumber.Bytes(), ":") {
return logical.ErrorResponse("adding CA to CRL is not allowed"), nil
return logical.ErrorResponse("adding Root CA to CRL is not allowed"), nil
}

alreadyRevoked := false
Expand Down Expand Up @@ -97,6 +97,12 @@ func revokeCert(ctx context.Context, b *backend, req *logical.Request, serial st
if cert == nil {
return nil, fmt.Errorf("got a nil certificate")
}
// Check if we can do anything based on the roleName
if roleName != "" { // if no roleName was supplied, the user had general revoke permission
if check, err := checkCNrole(ctx, b, req, roleName, cert.Issuer.CommonName); check {
return nil, err
}
}

// Add a little wiggle room because leases are stored with a second
// granularity
Expand Down Expand Up @@ -146,6 +152,32 @@ func revokeCert(ctx context.Context, b *backend, req *logical.Request, serial st
return resp, nil
}

func checkCNrole(ctx context.Context, b *backend, req *logical.Request, roleName string, cn string) (bool, error) {
// Get the role
role, err := b.getRole(ctx, req.Storage, roleName)
if err != nil {
return false, err
}
if role == nil {
return false, errutil.UserError{Err: fmt.Sprintf("unknown role: %s", roleName)}
}

// Create an inputBundle from the role. This is so we can use the preexisting helper
// to validate the CN for revocation.
roleInputBundle := &inputBundle{role: role}

// Check the CN. This ensures that the CN is checked even if it's
// excluded from SANs.
if cn != "" {
badName := validateNames(b, roleInputBundle, []string{cn})
if len(badName) != 0 {
return false, errutil.UserError{Err: fmt.Sprintf(
"common name %s not allowed by this role", badName)}
}
}
return true, nil
}

// Builds a CRL by going through the list of revoked certificates and building
// a new CRL with the stored revocation times and serial numbers.
func buildCRL(ctx context.Context, b *backend, req *logical.Request, forceNew bool) error {
Expand Down
32 changes: 31 additions & 1 deletion builtin/logical/pki/path_revoke.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,30 @@ hyphen-separated octal`,
}
}

func pathRevokeByRole(b *backend) *framework.Path {
return &framework.Path{
Pattern: `revoke/` + framework.GenericNameRegex("name"),
Fields: map[string]*framework.FieldSchema{
"serial_number": &framework.FieldSchema{
Type: framework.TypeString,
Description: `Certificate serial number, in colon- or
hyphen-separated octal`,
},
"name": &framework.FieldSchema{
Type: framework.TypeString,
Description: "Name of the role",
},
},

Callbacks: map[logical.Operation]framework.OperationFunc{
logical.UpdateOperation: b.pathRevokeWrite,
},

HelpSynopsis: pathRevokeHelpSyn,
HelpDescription: pathRevokeHelpDesc,
}
}

func pathRotateCRL(b *backend) *framework.Path {
return &framework.Path{
Pattern: `crl/rotate`,
Expand All @@ -47,6 +71,12 @@ func pathRotateCRL(b *backend) *framework.Path {

func (b *backend) pathRevokeWrite(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) {
serial := data.Get("serial_number").(string)
roleNameRaw, ok := data.GetOk("name")
if !ok {
roleNameRaw = ""
}
roleName := roleNameRaw.(string)

if len(serial) == 0 {
return logical.ErrorResponse("The serial number must be provided"), nil
}
Expand All @@ -62,7 +92,7 @@ func (b *backend) pathRevokeWrite(ctx context.Context, req *logical.Request, dat
b.revokeStorageLock.Lock()
defer b.revokeStorageLock.Unlock()

return revokeCert(ctx, b, req, serial, false)
return revokeCert(ctx, b, req, serial, roleName, false)
}

func (b *backend) pathRotateCRLRead(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) {
Expand Down
3 changes: 2 additions & 1 deletion builtin/logical/pki/secret_certs.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,5 +48,6 @@ func (b *backend) secretCredsRevoke(ctx context.Context, req *logical.Request, d
b.revokeStorageLock.Lock()
defer b.revokeStorageLock.Unlock()

return revokeCert(ctx, b, req, serialInt.(string), true)
// TODO: Come back to this
return revokeCert(ctx, b, req, serialInt.(string), "", true)
}