Repository: TBD54566975/ssi-service
Pull request #579 (2023-07-07 20:03:04)

Implement Revoked check for a key when signing manifest, schema, presentation request.
# Overview

This PR adds `Revoked` checks for a key when signing manifest, schema, presentation request. This is a follow-up to #546, in particular addresses this comment https://github.com/TBD54566975/ssi-service/pull/546#discussion_r1235599242.

# Description

It is related to #451. Previously in #546 revocation check was implemented for credential creation. This PR adds the check to all other relevant places. Please help verify that all applicable scenarios where revocation check is required are covered. 

# How Has This Been Tested?

Extended the existing auto-tests to cover the new code.

# Checklist

Before submitting this PR, please make sure:

- [x] I have read the CONTRIBUTING document.
- [x] My code is consistent with the rest of the project 
- [x] I have tagged the relevant reviewers and/or interested parties
- [x] I have updated the READMEs and other documentation of affected packages

## References

See above.


codecov-commenter: ## [Codecov](https://app.codecov.io/gh/TBD54566975/ssi-service/pull/579?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TBD54566975) Report
> Merging [#579](https://app.codecov.io/gh/TBD54566975/ssi-service/pull/579?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TBD54566975) (288b4a5) into [main](https://app.codecov.io/gh/TBD54566975/ssi-service/commit/e7d184d231cea00d56b5efcd8a0d43b9f864f034?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TBD54566975) (e7d184d) will **increase** coverage by `0.08%`.
> The diff coverage is `100.00%`.

```diff
@@            Coverage Diff             @@
##             main     #579      +/-   ##
==========================================
+ Coverage   24.00%   24.08%   +0.08%     
==========================================
  Files          46       46              
  Lines        5312     5315       +3     
==========================================
+ Hits         1275     1280       +5     
+ Misses       3799     3796       -3     
- Partials      238      239       +1     
```


| [Impacted Files](https://app.codecov.io/gh/TBD54566975/ssi-service/pull/579?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TBD54566975) | Coverage Δ | |
|---|---|---|
| [pkg/service/keystore/service.go](https://app.codecov.io/gh/TBD54566975/ssi-service/pull/579?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TBD54566975#diff-cGtnL3NlcnZpY2Uva2V5c3RvcmUvc2VydmljZS5nbw==) | `44.30% <100.00%> (+2.36%)` | :arrow_up: |



Patch:
diff --git a/pkg/service/keystore/service.go b/pkg/service/keystore/service.go
index 2975a88..a773de0 100644
--- a/pkg/service/keystore/service.go
+++ b/pkg/service/keystore/service.go
@@ -204,6 +204,9 @@ func (s Service) Sign(ctx context.Context, keyID string, data any) (*keyaccess.J
 	if err != nil {
 		return nil, sdkutil.LoggingErrorMsgf(err, "getting key with keyID<%s>", keyID)
 	}
+	if gotKey.Revoked {
+		return nil, sdkutil.LoggingNewErrorf("cannot use revoked key<%s>", gotKey.ID)
+	}
 	keyAccess, err := keyaccess.NewJWKKeyAccess(gotKey.Controller, gotKey.ID, gotKey.Key)
 	if err != nil {
 		return nil, sdkutil.LoggingErrorMsgf(err, "creating key access for keyID<%s>", keyID)
diff --git a/pkg/service/manifest/response.go b/pkg/service/manifest/response.go
index 9822792..04d5b38 100644
--- a/pkg/service/manifest/response.go
+++ b/pkg/service/manifest/response.go
@@ -35,6 +35,9 @@ func (s Service) signCredentialResponse(ctx context.Context, keyStoreID string,
 	if err != nil {
 		return nil, sdkutil.LoggingErrorMsgf(err, "getting key for signing response with key<%s>", keyStoreID)
 	}
+	if gotKey.Revoked {
+		return nil, sdkutil.LoggingNewErrorf("cannot use revoked key<%s>", gotKey.ID)
+	}
 	keyAccess, err := keyaccess.NewJWKKeyAccess(gotKey.Controller, gotKey.ID, gotKey.Key)
 	if err != nil {
 		return nil, sdkutil.LoggingErrorMsgf(err, "creating key access for signing response with key<%s>", gotKey.ID)
diff --git a/pkg/service/schema/service.go b/pkg/service/schema/service.go
index d41b4d7..4ab2771 100644
--- a/pkg/service/schema/service.go
+++ b/pkg/service/schema/service.go
@@ -189,6 +189,9 @@ func (s Service) signCredentialSchema(ctx context.Context, cred credential.Verif
 	if gotKey.Controller != issuer {
 		return nil, sdkutil.LoggingNewErrorf("key controller<%s> does not match credential issuer<%s> for key<%s>", gotKey.Controller, issuer, fullyQualifiedVerificationMethodID)
 	}
+	if gotKey.Revoked {
+		return nil, sdkutil.LoggingNewErrorf("cannot use revoked key<%s>", gotKey.ID)
+	}
 	keyAccess, err := keyaccess.NewJWKKeyAccess(fullyQualifiedVerificationMethodID, gotKey.ID, gotKey.Key)
 	if err != nil {
 		return nil, errors.Wrapf(err, "creating key access for signing credential schema with key<%s>", gotKey.ID)


Test patch:
diff --git a/pkg/server/router/manifest_test.go b/pkg/server/router/manifest_test.go
index 185ccc1..2b92c7e 100644
--- a/pkg/server/router/manifest_test.go
+++ b/pkg/server/router/manifest_test.go
@@ -17,6 +17,9 @@ import (
 	"github.com/tbd54566975/ssi-service/pkg/service/credential"
 	"github.com/tbd54566975/ssi-service/pkg/service/did"
 	"github.com/tbd54566975/ssi-service/pkg/service/framework"
+	"github.com/tbd54566975/ssi-service/pkg/service/keystore"
+	"github.com/tbd54566975/ssi-service/pkg/service/manifest"
+
 	"github.com/tbd54566975/ssi-service/pkg/service/manifest/model"
 	"github.com/tbd54566975/ssi-service/pkg/service/operation/storage"
 	presmodel "github.com/tbd54566975/ssi-service/pkg/service/presentation/model"
@@ -198,38 +201,51 @@ func TestManifestRouter(t *testing.T) {
 					signed, err := signer.SignJSON(applicationRequest)
 					assert.NoError(ttt, err)
 
-					submitApplicationRequest := SubmitApplicationRequest{ApplicationJWT: *signed}
-					sar, err := submitApplicationRequest.toServiceRequest()
-					assert.NoError(ttt, err)
-					createdApplicationResponseOp, err := manifestService.ProcessApplicationSubmission(context.Background(), *sar)
-					assert.NoError(ttt, err)
-					assert.False(ttt, createdApplicationResponseOp.Done)
-
-					createdApplicationResponse, err := manifestService.ReviewApplication(context.Background(), model.ReviewApplicationRequest{
-						ID:       storage.StatusObjectID(createdApplicationResponseOp.ID),
-						Approved: true,
-						Reason:   "ApprovalMan is here",
-						CredentialOverrides: map[string]model.CredentialOverride{
-							"id1": {
-								Data: map[string]any{"licenseType": "Class D"},
-							},
-							"id2": {
-								Data: map[string]any{"licenseType": "Class D"},
-							},
-						},
-					})
+					// submit and review application, the key is valid
+					createdApplicationResponse, err := submitAndReviewApplication(signed, ttt, manifestService)
 					assert.NoError(ttt, err)
 					assert.NotEmpty(ttt, createdManifest)
 					assert.NotEmpty(ttt, createdApplicationResponse.Response.ID)
 					assert.NotEmpty(ttt, createdApplicationResponse.Response.Fulfillment)
 					assert.Empty(ttt, createdApplicationResponse.Response.Denial)
 					assert.Equal(ttt, len(createManifestRequest.OutputDescriptors), len(createdApplicationResponse.Credentials))
+
+					// attempt to submit and review application again, this time with the revoked key
+					err = keyStoreService.RevokeKey(context.Background(), keystore.RevokeKeyRequest{ID: kid})
+					assert.NoError(ttt, err)
+					_, err = submitAndReviewApplication(signed, ttt, manifestService)
+					assert.Error(tt, err)
+					assert.ErrorContains(tt, err, "cannot use revoked key")
 				})
 			})
 		})
 	}
 }
 
+func submitAndReviewApplication(signed *keyaccess.JWT, ttt *testing.T, manifestService *manifest.Service) (*model.SubmitApplicationResponse, error) {
+	submitApplicationRequest := SubmitApplicationRequest{ApplicationJWT: *signed}
+	sar, err := submitApplicationRequest.toServiceRequest()
+	assert.NoError(ttt, err)
+	createdApplicationResponseOp, err := manifestService.ProcessApplicationSubmission(context.Background(), *sar)
+	assert.NoError(ttt, err)
+	assert.False(ttt, createdApplicationResponseOp.Done)
+
+	createdApplicationResponse, err := manifestService.ReviewApplication(context.Background(), model.ReviewApplicationRequest{
+		ID:       storage.StatusObjectID(createdApplicationResponseOp.ID),
+		Approved: true,
+		Reason:   "ApprovalMan is here",
+		CredentialOverrides: map[string]model.CredentialOverride{
+			"id1": {
+				Data: map[string]any{"licenseType": "Class D"},
+			},
+			"id2": {
+				Data: map[string]any{"licenseType": "Class D"},
+			},
+		},
+	})
+	return createdApplicationResponse, err
+}
+
 func getValidManifestRequestRequest(issuerDID *did.CreateDIDResponse, kid string, createdManifest *model.CreateManifestResponse) model.CreateRequestRequest {
 	return model.CreateRequestRequest{
 		ManifestRequest: model.Request{
diff --git a/pkg/server/router/schema_test.go b/pkg/server/router/schema_test.go
index 790d6f0..77e91f9 100644
--- a/pkg/server/router/schema_test.go
+++ b/pkg/server/router/schema_test.go
@@ -5,10 +5,14 @@ import (
 	"testing"
 
 	credschema "github.com/TBD54566975/ssi-sdk/credential/schema"
+	"github.com/TBD54566975/ssi-sdk/crypto"
+	didsdk "github.com/TBD54566975/ssi-sdk/did"
 	"github.com/stretchr/testify/assert"
 
 	"github.com/tbd54566975/ssi-service/config"
+	"github.com/tbd54566975/ssi-service/pkg/service/did"
 	"github.com/tbd54566975/ssi-service/pkg/service/framework"
+	"github.com/tbd54566975/ssi-service/pkg/service/keystore"
 	"github.com/tbd54566975/ssi-service/pkg/service/schema"
 	"github.com/tbd54566975/ssi-service/pkg/testutil"
 )
@@ -58,27 +62,7 @@ func TestSchemaRouter(t *testing.T) {
 				assert.Contains(tt, err.Error(), "error getting schema")
 
 				// create a schema
-				simpleSchema := map[string]any{
-					"$schema": "https://json-schema.org/draft-07/schema",
-					"type":    "object",
-					"properties": map[string]any{
-						"credentialSubject": map[string]any{
-							"type": "object",
-							"properties": map[string]any{
-								"id": map[string]any{
-									"type": "string",
-								},
-								"firstName": map[string]any{
-									"type": "string",
-								},
-								"lastName": map[string]any{
-									"type": "string",
-								},
-							},
-							"required": []any{"firstName", "lastName"},
-						},
-					},
-				}
+				simpleSchema := getSimpleSchema()
 				createdSchema, err := schemaService.CreateSchema(context.Background(), schema.CreateSchemaRequest{Issuer: "me", Name: "simple schema", Schema: simpleSchema})
 				assert.NoError(tt, err)
 				assert.NotEmpty(tt, createdSchema)
@@ -148,27 +132,7 @@ func TestSchemaSigning(t *testing.T) {
 				assert.Equal(tt, framework.StatusReady, schemaService.Status().Status)
 
 				// create a schema and don't sign it
-				simpleSchema := map[string]any{
-					"$schema": "https://json-schema.org/draft-07/schema",
-					"type":    "object",
-					"properties": map[string]any{
-						"credentialSubject": map[string]any{
-							"type": "object",
-							"properties": map[string]any{
-								"id": map[string]any{
-									"type": "string",
-								},
-								"firstName": map[string]any{
-									"type": "string",
-								},
-								"lastName": map[string]any{
-									"type": "string",
-								},
-							},
-							"required": []any{"firstName", "lastName"},
-						},
-					},
-				}
+				simpleSchema := getSimpleSchema()
 				createdSchema, err := schemaService.CreateSchema(context.Background(), schema.CreateSchemaRequest{Issuer: "me", Name: "simple schema", Schema: simpleSchema})
 				assert.NoError(tt, err)
 				assert.NotEmpty(tt, createdSchema)
@@ -176,5 +140,64 @@ func TestSchemaSigning(t *testing.T) {
 				assert.Equal(tt, "simple schema", createdSchema.Schema.Name())
 			})
 		})
+
+		t.Run("Signing schema with revoked key test", func(tt *testing.T) {
+			db := test.ServiceStorage(t)
+			assert.NotEmpty(tt, db)
+
+			serviceConfig := config.SchemaServiceConfig{BaseServiceConfig: &config.BaseServiceConfig{Name: "schema"}}
+			keyStoreService := testKeyStoreService(tt, db)
+			didService := testDIDService(tt, db, keyStoreService)
+			schemaService, err := schema.NewSchemaService(serviceConfig, db, keyStoreService, didService.GetResolver())
+			assert.NoError(tt, err)
+			assert.NotEmpty(tt, schemaService)
+
+			// Create a DID
+			controllerDID, err := didService.CreateDIDByMethod(context.Background(), did.CreateDIDRequest{Method: didsdk.KeyMethod, KeyType: crypto.Ed25519})
+			assert.NoError(tt, err)
+			assert.NotEmpty(tt, controllerDID)
+			didID := controllerDID.DID.ID
+
+			// Create a key controlled by the DID
+			keyID := controllerDID.DID.VerificationMethod[0].ID
+			privateKey := "2dEPd7mA3aiuh2gky8tTPiCkyMwf8tBNUMZwRzeVxVJnJFGTbdLGUBcx51DCNyFWRjTG9bduvyLRStXSCDMFXULY"
+
+			err = keyStoreService.StoreKey(context.Background(), keystore.StoreKeyRequest{ID: keyID, Type: crypto.Ed25519, Controller: didID, PrivateKeyBase58: privateKey})
+			assert.NoError(tt, err)
+
+			// Revoke the key
+			err = keyStoreService.RevokeKey(context.Background(), keystore.RevokeKeyRequest{ID: keyID})
+			assert.NoError(tt, err)
+
+			// create a schema with the revoked key, it fails
+			_, err = schemaService.CreateSchema(context.Background(), schema.CreateSchemaRequest{Issuer: controllerDID.DID.ID, Name: "schema (revoked key)", Schema: getEmailSchema(), FullyQualifiedVerificationMethodID: keyID})
+			assert.Error(tt, err)
+			assert.ErrorContains(tt, err, "cannot use revoked key")
+		})
+	}
+}
+
+func getSimpleSchema() map[string]any {
+	simpleSchema := map[string]any{
+		"$schema": "https://json-schema.org/draft-07/schema",
+		"type":    "object",
+		"properties": map[string]any{
+			"credentialSubject": map[string]any{
+				"type": "object",
+				"properties": map[string]any{
+					"id": map[string]any{
+						"type": "string",
+					},
+					"firstName": map[string]any{
+						"type": "string",
+					},
+					"lastName": map[string]any{
+						"type": "string",
+					},
+				},
+				"required": []any{"firstName", "lastName"},
+			},
+		},
 	}
+	return simpleSchema
 }
diff --git a/pkg/service/keystore/service_test.go b/pkg/service/keystore/service_test.go
index 4595aef..16c28ba 100644
--- a/pkg/service/keystore/service_test.go
+++ b/pkg/service/keystore/service_test.go
@@ -152,6 +152,11 @@ func TestRevokeKey(t *testing.T) {
 	assert.Equal(t, privKey, keyResponse.Key)
 	assert.True(t, keyResponse.Revoked)
 	assert.Equal(t, "2023-06-23T00:00:00Z", keyResponse.RevokedAt)
+
+	// attempt to "Sign()" with the revoked key, ensure it is prohibited
+	_, err = keyStore.Sign(context.Background(), keyID, "sampleDataAsString")
+	assert.Error(t, err)
+	assert.ErrorContains(t, err, "cannot use revoked key")
 }
 
 func createKeyStoreService(t *testing.T) (*Service, error) {