Skip to content

Commit

Permalink
[FAB-7620] Return err when revoking revoked cert
Browse files Browse the repository at this point in the history
Currently, a revoked certificate can be revoked.
This will update the revoked_at value in the database,
losing the date and time when the certificate was
revoked initially. This change will return an error
if the certificate is already revoked.

Change-Id: Id395fd76b6e84b08ee8dc3804ee7a2bc773c3a31
Signed-off-by: Anil Ambati <[email protected]>
  • Loading branch information
Anil Ambati committed Jan 18, 2018
1 parent 673faef commit ad88250
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 0 deletions.
13 changes: 13 additions & 0 deletions cmd/fabric-ca-client/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1571,6 +1571,11 @@ func testRevoke(t *testing.T) {
t.Fatalf("Failed to enroll testRegister5 user: %s", err)
}

testRegister5Serial, testRegister5AKI, err := getSerialAKIByID("testRegister5")
if err != nil {
t.Fatalf("Failed to get serial and aki of the enrollment certificate of the user 'testRegister5': %s", err)
}

// Revoke testRegister5 without --gencrl option, so it does not create a CRL
err = RunMain([]string{cmdName, "revoke", "-u", serverURL, "--revoke.name",
"testRegister5", "--revoke.serial", "", "--revoke.aki", ""})
Expand All @@ -1580,6 +1585,14 @@ func testRevoke(t *testing.T) {
_, err = os.Stat(filepath.Join(clientHome, "msp/crls/crl.pem"))
assert.Error(t, err, "CRL should not be created when revoke is called without --gencrl parameter")

// Revoke testRegister5 certificate that was revoked by the above revoke command, we expect
// an error in this case
err = RunMain([]string{cmdName, "revoke", "-u", serverURL,
"--revoke.serial", testRegister5Serial, "--revoke.aki", testRegister5AKI})
if err == nil {
t.Error("Revoke of testRegister5's certificate should have failed as it was already revoked")
}

err = RunMain([]string{cmdName, "enroll", "-d", "-u", "http://admin3:adminpw3@localhost:7090"})
if err != nil {
t.Errorf("client enroll -u failed: %s", err)
Expand Down
2 changes: 2 additions & 0 deletions lib/servererror.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,8 @@ const (
ErrUpdateConfigModifyAff = 64
// Error occured while deleting user
ErrDBDeleteUser = 65
// Certificate that is being revoked has already been revoked
ErrCertAlreadyRevoked = 66
)

// Construct a new HTTP error.
Expand Down
15 changes: 15 additions & 0 deletions lib/serverrevoke.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,16 @@ type revocationResponseNet struct {
CRL string
}

// CertificateStatus represents status of an enrollment certificate
type CertificateStatus string

const (
// Revoked is the status of a revoked certificate
Revoked CertificateStatus = "revoked"
// Good is the status of a active certificate
Good = "good"
)

func newRevokeEndpoint(s *Server) *serverEndpoint {
return &serverEndpoint{
Methods: []string{"POST"},
Expand Down Expand Up @@ -82,6 +92,11 @@ func revokeHandler(ctx *serverRequestContext) (interface{}, error) {
req.Serial, req.AKI, err)
}

if certificate.Status == string(Revoked) {
return nil, newHTTPErr(404, ErrCertAlreadyRevoked, "Certificate with serial %s and AKI %s was already revoked",
req.Serial, req.AKI)
}

if req.Name != "" && req.Name != certificate.ID {
return nil, newHTTPErr(400, ErrCertWrongOwner, "Certificate with serial %s and AKI %s is not owned by %s",
req.Serial, req.AKI, req.Name)
Expand Down

0 comments on commit ad88250

Please sign in to comment.