Skip to content

Commit

Permalink
Enable administrative revocation of malformed certs (letsencrypt#5813)
Browse files Browse the repository at this point in the history
Today, the revocation codepaths involve parsing the to-be-revoked
certificate multiple times: inside `admin-revoker` itself, inside the
RA's `AdministrativelyRevokeCertificate` method, and again in its helper
`revokeCertificate`. In addition, we use the fact that we have the full
certificate multiple times: to log various attributes of it, to compute
its `IssuerNameID`, and more. All of this will fail if we ever issue a
cert that is malformed to the point that it cannot be parsed.

Add a new argument to the `AdministrativelyRevokeCertificateRequest`
that allows the certificate to be identified by serial only, instead of
by full certificate bytes. Add support for this in the gRPC handler by
using the serial to construct a dummy in-memory Certificate object.
Support this in the `revokeCertificate` codepath by checking to see if
the passed-in cert has any underlying raw DER bytes, and if not,
triggering the new codepath that does everything via the serial.

In order to support this, unfortunately we have to add a second
in-memory map to the RA, so that it can look up issuer certs by either
name ID or old-style ID, as the IDs gleaned from the database (instead
of from the cert itself) may still be old-style. This will be removed
when the old-style Issuer IDs have aged out.

Fixes letsencrypt#5759
  • Loading branch information
aarongable authored Nov 29, 2021
1 parent 6184ad5 commit 2a26294
Show file tree
Hide file tree
Showing 6 changed files with 298 additions and 123 deletions.
3 changes: 1 addition & 2 deletions akamai/cache-client.go
Original file line number Diff line number Diff line change
Expand Up @@ -392,8 +392,7 @@ func GeneratePurgeURLs(cert, issuer *x509.Certificate) ([]string, error) {
if !strings.HasSuffix(ocspServer, "/") {
ocspServer += "/"
}
// Generate GET url
urls = generateOCSPCacheKeys(req, ocspServer)
urls = append(urls, generateOCSPCacheKeys(req, ocspServer)...)
}
return urls, nil
}
42 changes: 33 additions & 9 deletions cmd/admin-revoker/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,19 +106,30 @@ func (r *revoker) revokeCertificate(ctx context.Context, certObj core.Certificat
if reasonCode < 0 || reasonCode == 7 || reasonCode > 10 {
panic(fmt.Sprintf("Invalid reason code: %d", reasonCode))
}
cert, err := x509.ParseCertificate(certObj.DER)
if err != nil {
return err
}
u, err := user.Current()
if err != nil {
return err
}
_, err = r.rac.AdministrativelyRevokeCertificate(ctx, &rapb.AdministrativelyRevokeCertificateRequest{
Cert: cert.Raw,
Code: int64(reasonCode),
AdminName: u.Username,
})

var req *rapb.AdministrativelyRevokeCertificateRequest
if certObj.DER != nil {
cert, err := x509.ParseCertificate(certObj.DER)
if err != nil {
return err
}
req = &rapb.AdministrativelyRevokeCertificateRequest{
Cert: cert.Raw,
Code: int64(reasonCode),
AdminName: u.Username,
}
} else {
req = &rapb.AdministrativelyRevokeCertificateRequest{
Serial: certObj.Serial,
Code: int64(reasonCode),
AdminName: u.Username,
}
}
_, err = r.rac.AdministrativelyRevokeCertificate(ctx, req)
if err != nil {
return err
}
Expand Down Expand Up @@ -199,6 +210,10 @@ func (r *revoker) revokeByReg(ctx context.Context, regID int64, reasonCode revoc
return nil
}

func (r *revoker) revokeMalformedBySerial(ctx context.Context, serial string, reasonCode revocation.Reason) error {
return r.revokeCertificate(ctx, core.Certificate{Serial: serial}, reasonCode)
}

// This abstraction is needed so that we can use sort.Sort below
type revocationCodes []revocation.Reason

Expand Down Expand Up @@ -270,6 +285,15 @@ func main() {
err = r.revokeByReg(ctx, regID, revocation.Reason(reasonCode))
cmd.FailOnError(err, "Couldn't revoke certificate by registration")

case command == "malformed-revoke" && len(args) == 3:
// 1: serial, 2: reasonCode
serial := args[0]
reasonCode, err := strconv.Atoi(args[1])
cmd.FailOnError(err, "Reason code argument must be an integer")

err = r.revokeMalformedBySerial(ctx, serial, revocation.Reason(reasonCode))
cmd.FailOnError(err, "Couldn't revoke certificate by serial")

case command == "list-reasons":
var codes revocationCodes
for k := range revocation.ReasonToString {
Expand Down
148 changes: 79 additions & 69 deletions ra/proto/ra.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions ra/proto/ra.proto
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ message RevokeCertificateWithRegRequest {

message AdministrativelyRevokeCertificateRequest {
bytes cert = 1;
string serial = 4;
int64 code = 2;
string adminName = 3;
}
Expand Down
Loading

0 comments on commit 2a26294

Please sign in to comment.