diff --git a/signer/api/api.go b/signer/api/api.go index 393f840177..8f8a7480b9 100644 --- a/signer/api/api.go +++ b/signer/api/api.go @@ -47,15 +47,7 @@ func KeyInfo(sigServices signer.SigningServiceIndex) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { vars := mux.Vars(r) - // TODO(diogo): call resolve method from KeyID -> KeyInfo - sigService := getSigningService(w, RSAAlgorithm, sigServices) - if sigService == nil { - // Error handled inside getSigningService - return - } - - keyID := &pb.KeyID{ID: vars["ID"]} - key, err := sigService.KeyInfo(keyID) + key, _, err := FindKeyByID(sigServices, &pb.KeyID{ID: vars["ID"]}) if err != nil { switch err { // If we received an ErrInvalidKeyID, the key doesn't exist, return 404 @@ -109,11 +101,21 @@ func DeleteKey(sigServices signer.SigningServiceIndex) http.Handler { return } - // TODO(diogo): call resolve method from KeyID -> KeyInfo - sigService := getSigningService(w, RSAAlgorithm, sigServices) - if sigService == nil { - // Error handled inside getSigningService - return + _, sigService, err := FindKeyByID(sigServices, keyID) + + if err != nil { + switch err { + // If we received an ErrInvalidKeyID, the key doesn't exist, return 404 + case keys.ErrInvalidKeyID: + w.WriteHeader(http.StatusNotFound) + w.Write([]byte(err.Error())) + return + // If we received anything else, it is unexpected, and we return a 500 + default: + w.WriteHeader(http.StatusInternalServerError) + w.Write([]byte(err.Error())) + return + } } _, err = sigService.DeleteKey(keyID) @@ -151,15 +153,19 @@ func Sign(sigServices signer.SigningServiceIndex) http.Handler { return } - // TODO(diogo): call resolve method from KeyID -> KeyInfo - keyInfo := &pb.KeyInfo{KeyID: sigRequest.KeyID, Algorithm: &pb.Algorithm{Algorithm: RSAAlgorithm}} - sigService := getSigningService(w, keyInfo.Algorithm.Algorithm, sigServices) - if sigService == nil { - // Error handled inside getSigningService + _, sigService, err := FindKeyByID(sigServices, sigRequest.KeyID) + if err == keys.ErrInvalidKeyID { + w.WriteHeader(http.StatusNotFound) + w.Write([]byte(err.Error())) + return + } else if err != nil { + // We got an unexpected error + w.WriteHeader(http.StatusInternalServerError) + w.Write([]byte(err.Error())) return } - signer, err := sigService.Signer(keyInfo) + signer, err := sigService.Signer(sigRequest.KeyID) if err == keys.ErrInvalidKeyID { w.WriteHeader(http.StatusNotFound) w.Write([]byte(err.Error())) diff --git a/signer/api/ed25519_signing_service.go b/signer/api/ed25519_signing_service.go index 7b6b66d9fb..2b8b6be722 100644 --- a/signer/api/ed25519_signing_service.go +++ b/signer/api/ed25519_signing_service.go @@ -52,9 +52,8 @@ func (s EdDSASigningService) KeyInfo(keyID *pb.KeyID) (*pb.PublicKey, error) { } // Signer returns a Signer for a specific KeyID -func (s EdDSASigningService) Signer(keyInfo *pb.KeyInfo) (signer.Signer, error) { - // TODO(diogo): add verification of keyInfo.Algorithm to be ECDSA - key, err := s.KeyDB.GetKey(keyInfo.KeyID) +func (s EdDSASigningService) Signer(keyID *pb.KeyID) (signer.Signer, error) { + key, err := s.KeyDB.GetKey(keyID) if err != nil { return nil, keys.ErrInvalidKeyID } diff --git a/signer/api/ed25519_signing_service_test.go b/signer/api/ed25519_signing_service_test.go index 24a1c5129e..0f43ada8c1 100644 --- a/signer/api/ed25519_signing_service_test.go +++ b/signer/api/ed25519_signing_service_test.go @@ -84,7 +84,7 @@ func TestSigner(t *testing.T) { sigService := api.NewEdDSASigningService(&m) m.On("GetKey", fakeKeyID).Return(&keys.Key{}, nil).Once() - _, err := sigService.Signer(&pb.KeyInfo{KeyID: &pb.KeyID{ID: fakeKeyID}}) + _, err := sigService.Signer(&pb.KeyID{ID: fakeKeyID}) m.Mock.AssertExpectations(t) assert.Nil(t, err) diff --git a/signer/api/find_key.go b/signer/api/find_key.go new file mode 100644 index 0000000000..be4ee094e7 --- /dev/null +++ b/signer/api/find_key.go @@ -0,0 +1,19 @@ +package api + +import ( + "github.com/docker/notary/signer" + "github.com/docker/notary/signer/keys" + + pb "github.com/docker/notary/proto" +) + +func FindKeyByID(sigServices signer.SigningServiceIndex, keyID *pb.KeyID) (*pb.PublicKey, signer.SigningService, error) { + for _, service := range sigServices { + key, err := service.KeyInfo(keyID) + if err == nil { + return key, service, nil + } + } + + return nil, nil, keys.ErrInvalidKeyID +} diff --git a/signer/api/rpc_api.go b/signer/api/rpc_api.go index ee83b4626d..0e653687ee 100644 --- a/signer/api/rpc_api.go +++ b/signer/api/rpc_api.go @@ -42,22 +42,20 @@ func (s *KeyManagementServer) CreateKey(ctx context.Context, algorithm *pb.Algor //DeleteKey deletes they key associated with a KeyID func (s *KeyManagementServer) DeleteKey(ctx context.Context, keyID *pb.KeyID) (*pb.Void, error) { - // TODO(diogo): call resolve method from KeyID -> KeyInfo - keyInfo := &pb.KeyInfo{KeyID: keyID, Algorithm: &pb.Algorithm{Algorithm: RSAAlgorithm}} - service := s.SigServices[keyInfo.Algorithm.Algorithm] + _, service, err := FindKeyByID(s.SigServices, keyID) - if service == nil { - return nil, fmt.Errorf("algorithm %s not supported for delete key", keyInfo.Algorithm.Algorithm) + if err != nil { + return nil, grpc.Errorf(codes.NotFound, "Invalid keyID: key %s not found", keyID.ID) } - _, err := service.DeleteKey(keyInfo.KeyID) - log.Println("[Notary-signer DeleteKey] : Deleted KeyID ", keyInfo.KeyID.ID) + _, err = service.DeleteKey(keyID) + log.Println("[Notary-signer DeleteKey] : Deleted KeyID ", keyID.ID) if err != nil { switch err { case keys.ErrInvalidKeyID: - return nil, grpc.Errorf(codes.NotFound, "Invalid keyID: key %s not found", keyInfo.KeyID.ID) + return nil, grpc.Errorf(codes.NotFound, "Invalid keyID: key %s not found", keyID.ID) default: - return nil, grpc.Errorf(codes.Internal, "Key deletion for keyID %s failed", keyInfo.KeyID.ID) + return nil, grpc.Errorf(codes.Internal, "Key deletion for keyID %s failed", keyID.ID) } } @@ -66,43 +64,39 @@ func (s *KeyManagementServer) DeleteKey(ctx context.Context, keyID *pb.KeyID) (* //GetKeyInfo returns they PublicKey associated with a KeyID func (s *KeyManagementServer) GetKeyInfo(ctx context.Context, keyID *pb.KeyID) (*pb.PublicKey, error) { - // TODO(diogo): call resolve method from KeyID -> KeyInfo - keyInfo := &pb.KeyInfo{KeyID: keyID, Algorithm: &pb.Algorithm{Algorithm: RSAAlgorithm}} - service := s.SigServices[keyInfo.Algorithm.Algorithm] + _, service, err := FindKeyByID(s.SigServices, keyID) - if service == nil { - return nil, fmt.Errorf("algorithm %s not supported for get key info", keyInfo.Algorithm.Algorithm) - } - - key, err := service.KeyInfo(keyInfo.KeyID) if err != nil { - return nil, grpc.Errorf(codes.NotFound, "Invalid keyID: key %s not found", keyInfo.KeyID.ID) + return nil, grpc.Errorf(codes.NotFound, "Invalid keyID: key %s not found", keyID.ID) } - log.Println("[Notary-signer GetKeyInfo] : Returning PublicKey for KeyID ", keyInfo.KeyID.ID) + + key, err := service.KeyInfo(keyID) + if err != nil { + return nil, grpc.Errorf(codes.NotFound, "Invalid keyID: key %s not found", keyID.ID) + } + log.Println("[Notary-signer GetKeyInfo] : Returning PublicKey for KeyID ", keyID.ID) return key, nil } //Sign signs a message and returns the signature using a private key associate with the KeyID from the SignatureRequest func (s *SignerServer) Sign(ctx context.Context, sr *pb.SignatureRequest) (*pb.Signature, error) { - // TODO(diogo): call resolve method from KeyID -> KeyInfo - keyInfo := &pb.KeyInfo{KeyID: sr.KeyID, Algorithm: &pb.Algorithm{Algorithm: RSAAlgorithm}} - service := s.SigServices[keyInfo.Algorithm.Algorithm] + _, service, err := FindKeyByID(s.SigServices, sr.KeyID) - if service == nil { - return nil, fmt.Errorf("algorithm %s not supported for sign", keyInfo.Algorithm.Algorithm) + if err != nil { + return nil, grpc.Errorf(codes.NotFound, "Invalid keyID: key %s not found", sr.KeyID.ID) } - log.Println("[Notary-signer Sign] : Signing ", string(sr.Content), " with KeyID ", keyInfo.KeyID.ID) - signer, err := service.Signer(keyInfo) + log.Println("[Notary-signer Sign] : Signing ", string(sr.Content), " with KeyID ", sr.KeyID.ID) + signer, err := service.Signer(sr.KeyID) if err == keys.ErrInvalidKeyID { - return nil, grpc.Errorf(codes.NotFound, "Invalid keyID: key not found") + return nil, grpc.Errorf(codes.NotFound, "Invalid keyID: key %s not found", sr.KeyID.ID) } else if err != nil { - return nil, grpc.Errorf(codes.Internal, "Signing failed for keyID %s on hash %s", keyInfo.KeyID.ID, sr.Content) + return nil, grpc.Errorf(codes.Internal, "Signing failed for keyID %s on hash %s", sr.KeyID.ID, sr.Content) } signature, err := signer.Sign(sr) if err != nil { - return nil, grpc.Errorf(codes.Internal, "Signing failed for keyID %s on hash %s", keyInfo.KeyID.ID, sr.Content) + return nil, grpc.Errorf(codes.Internal, "Signing failed for keyID %s on hash %s", sr.KeyID.ID, sr.Content) } return signature, nil diff --git a/signer/api/rsa_signing_service.go b/signer/api/rsa_signing_service.go index f90c5e9dbc..8e4e2c2e4f 100644 --- a/signer/api/rsa_signing_service.go +++ b/signer/api/rsa_signing_service.go @@ -139,9 +139,8 @@ func (s RSASigningService) KeyInfo(keyID *pb.KeyID) (*pb.PublicKey, error) { } // Signer returns a Signer for a specific KeyID -func (s RSASigningService) Signer(keyInfo *pb.KeyInfo) (signer.Signer, error) { - // TODO(diogo): Add verification of keyInfo.Algorithm to be RSA. - key, ok := s.keys[keyInfo.KeyID.ID] +func (s RSASigningService) Signer(keyID *pb.KeyID) (signer.Signer, error) { + key, ok := s.keys[keyID.ID] if !ok { return nil, keys.ErrInvalidKeyID } diff --git a/signer/signer.go b/signer/signer.go index 1f3bac7c0b..6a5de127c6 100644 --- a/signer/signer.go +++ b/signer/signer.go @@ -10,7 +10,7 @@ type SigningService interface { KeyManager // Signer returns a Signer for a given keyID - Signer(keyID *pb.KeyInfo) (Signer, error) + Signer(keyID *pb.KeyID) (Signer, error) } // SigningServiceIndex represents a mapping between a service algorithm string