feat: Support ecdsa and RSA keys (#270 with backwards compatibility) (#357)

* * fix!: ECDSA verifiers now expect PEM-encoded public keys per TUF specification
* feat: ECDSA signers are now implemented
* feat: RSA verifiers and signers are implemented

BREAKING CHANGE: ECDSA verifiers expect PEM-encoded public keys. If you rely
on previous behavior of hex-encoded public keys for verifiers, then you must
import pkg/deprecated/set_ecdsa that will allow a fallback for hex-encoded
ECDSA keys.

Co-authored-by: Asra Ali <asraa@google.com>
Co-authored-by: Toby Bristow <toby.bristow@qush.com>
Signed-off-by: Asra Ali <asraa@google.com>

* add comment

Signed-off-by: Asra Ali <asraa@google.com>

Signed-off-by: Asra Ali <asraa@google.com>
Co-authored-by: Toby Bristow <toby.bristow@qush.com>
diff --git a/cmd/tuf/gen_key.go b/cmd/tuf/gen_key.go
index b994843..bd4334a 100644
--- a/cmd/tuf/gen_key.go
+++ b/cmd/tuf/gen_key.go
@@ -6,11 +6,12 @@
 
 	"github.com/flynn/go-docopt"
 	"github.com/theupdateframework/go-tuf"
+	"github.com/theupdateframework/go-tuf/data"
 )
 
 func init() {
 	register("gen-key", cmdGenKey, `
-usage: tuf gen-key [--expires=<days>] <role>
+usage: tuf gen-key [--expires=<days>] [--scheme=<scheme>] <role>
 
 Generate a new signing key for the given role.
 
@@ -23,28 +24,40 @@
 
 Options:
   --expires=<days>   Set the root metadata file to expire <days> days from now.
+  --scheme=<scheme>      Set the key scheme to use [default: ed25519].
 `)
 }
 
 func cmdGenKey(args *docopt.Args, repo *tuf.Repo) error {
 	role := args.String["<role>"]
 	var keyids []string
+
+	keyScheme := data.KeySchemeEd25519
+	switch t := args.String["--scheme"]; t {
+	case string(data.KeySchemeEd25519),
+		string(data.KeySchemeECDSA_SHA2_P256),
+		string(data.KeySchemeRSASSA_PSS_SHA256):
+		keyScheme = data.KeyScheme(t)
+	default:
+		fmt.Println("Using default key scheme", keyScheme)
+	}
+
 	var err error
+	var expires time.Time
 	if arg := args.String["--expires"]; arg != "" {
-		var expires time.Time
 		expires, err = parseExpires(arg)
 		if err != nil {
 			return err
 		}
-		keyids, err = repo.GenKeyWithExpires(role, expires)
 	} else {
-		keyids, err = repo.GenKey(role)
+		expires = data.DefaultExpires(role)
 	}
+	keyids, err = repo.GenKeyWithSchemeAndExpires(role, expires, keyScheme)
 	if err != nil {
 		return err
 	}
 	for _, id := range keyids {
-		fmt.Println("Generated", role, "key with ID", id)
+		fmt.Println("Generated", role, keyScheme, "key with ID", id)
 	}
 	return nil
 }
diff --git a/data/types.go b/data/types.go
index 0fcc921..d051b76 100644
--- a/data/types.go
+++ b/data/types.go
@@ -15,18 +15,29 @@
 	"github.com/secure-systems-lab/go-securesystemslib/cjson"
 )
 
+type KeyType string
+
+type KeyScheme string
+
+type HashAlgorithm string
+
 const (
-	KeyIDLength                = sha256.Size * 2
-	KeyTypeEd25519             = "ed25519"
-	KeyTypeECDSA_SHA2_P256     = "ecdsa-sha2-nistp256"
-	KeySchemeEd25519           = "ed25519"
-	KeySchemeECDSA_SHA2_P256   = "ecdsa-sha2-nistp256"
-	KeyTypeRSASSA_PSS_SHA256   = "rsa"
-	KeySchemeRSASSA_PSS_SHA256 = "rsassa-pss-sha256"
+	KeyIDLength = sha256.Size * 2
+
+	KeyTypeEd25519           KeyType = "ed25519"
+	KeyTypeECDSA_SHA2_P256   KeyType = "ecdsa-sha2-nistp256"
+	KeyTypeRSASSA_PSS_SHA256 KeyType = "rsa"
+
+	KeySchemeEd25519           KeyScheme = "ed25519"
+	KeySchemeECDSA_SHA2_P256   KeyScheme = "ecdsa-sha2-nistp256"
+	KeySchemeRSASSA_PSS_SHA256 KeyScheme = "rsassa-pss-sha256"
+
+	HashAlgorithmSHA256 HashAlgorithm = "sha256"
+	HashAlgorithmSHA512 HashAlgorithm = "sha512"
 )
 
 var (
-	HashAlgorithms           = []string{"sha256", "sha512"}
+	HashAlgorithms           = []HashAlgorithm{HashAlgorithmSHA256, HashAlgorithmSHA512}
 	ErrPathsAndPathHashesSet = errors.New("tuf: failed validation of delegated target: paths and path_hash_prefixes are both set")
 )
 
@@ -41,9 +52,9 @@
 }
 
 type PublicKey struct {
-	Type       string          `json:"keytype"`
-	Scheme     string          `json:"scheme"`
-	Algorithms []string        `json:"keyid_hash_algorithms,omitempty"`
+	Type       KeyType         `json:"keytype"`
+	Scheme     KeyScheme       `json:"scheme"`
+	Algorithms []HashAlgorithm `json:"keyid_hash_algorithms,omitempty"`
 	Value      json.RawMessage `json:"keyval"`
 
 	ids    []string
@@ -51,9 +62,9 @@
 }
 
 type PrivateKey struct {
-	Type       string          `json:"keytype"`
-	Scheme     string          `json:"scheme,omitempty"`
-	Algorithms []string        `json:"keyid_hash_algorithms,omitempty"`
+	Type       KeyType         `json:"keytype"`
+	Scheme     KeyScheme       `json:"scheme,omitempty"`
+	Algorithms []HashAlgorithm `json:"keyid_hash_algorithms,omitempty"`
 	Value      json.RawMessage `json:"keyval"`
 }
 
diff --git a/go.mod b/go.mod
index 1a838de..a5151da 100644
--- a/go.mod
+++ b/go.mod
@@ -8,6 +8,7 @@
 	github.com/google/gofuzz v1.2.0
 	github.com/secure-systems-lab/go-securesystemslib v0.4.0
 	github.com/stretchr/testify v1.8.0
+	github.com/syndtr/goleveldb v1.0.1-0.20220721030215-126854af5e6d
 	golang.org/x/crypto v0.0.0-20211117183948-ae814b36b871
 	golang.org/x/term v0.0.0-20210927222741-03fcf44c2211
 	gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c
@@ -20,7 +21,6 @@
 	github.com/kr/text v0.1.0 // indirect
 	github.com/onsi/ginkgo v1.16.5 // indirect
 	github.com/pmezard/go-difflib v1.0.0 // indirect
-	github.com/syndtr/goleveldb v1.0.1-0.20220721030215-126854af5e6d // indirect
 	golang.org/x/net v0.0.0-20220607020251-c690dde0001d // indirect
 	golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
diff --git a/internal/signer/sort_test.go b/internal/signer/sort_test.go
index eb9f94d..afda5f8 100644
--- a/internal/signer/sort_test.go
+++ b/internal/signer/sort_test.go
@@ -26,7 +26,7 @@
 	return &data.PublicKey{
 		Type:       "mock",
 		Scheme:     "mock",
-		Algorithms: []string{"mock"},
+		Algorithms: []data.HashAlgorithm{"mock"},
 		Value:      s.value,
 	}
 }
diff --git a/pkg/deprecated/deprecated_repo_test.go b/pkg/deprecated/deprecated_repo_test.go
new file mode 100644
index 0000000..6d194ab
--- /dev/null
+++ b/pkg/deprecated/deprecated_repo_test.go
@@ -0,0 +1,82 @@
+package deprecated
+
+import (
+	"crypto"
+	"crypto/elliptic"
+	"crypto/rand"
+	"crypto/sha256"
+	"encoding/json"
+	"testing"
+
+	"github.com/secure-systems-lab/go-securesystemslib/cjson"
+	repo "github.com/theupdateframework/go-tuf"
+	"github.com/theupdateframework/go-tuf/data"
+	_ "github.com/theupdateframework/go-tuf/pkg/deprecated/set_ecdsa"
+	"github.com/theupdateframework/go-tuf/pkg/keys"
+	. "gopkg.in/check.v1"
+)
+
+func Test(t *testing.T) { TestingT(t) }
+
+type RepoSuite struct{}
+
+var _ = Suite(&RepoSuite{})
+
+func genKey(c *C, r *repo.Repo, role string) []string {
+	keyids, err := r.GenKey(role)
+	c.Assert(err, IsNil)
+	c.Assert(len(keyids) > 0, Equals, true)
+	return keyids
+}
+
+// Deprecated ecdsa key support: Support verification against roots that were
+// signed with hex-encoded ecdsa keys.
+func (rs *RepoSuite) TestDeprecatedHexEncodedKeysSucceed(c *C) {
+	files := map[string][]byte{"foo.txt": []byte("foo")}
+	local := repo.MemoryStore(make(map[string]json.RawMessage), files)
+	r, err := repo.NewRepo(local)
+	c.Assert(err, IsNil)
+
+	r.Init(false)
+	// Add a root key with hex-encoded ecdsa format
+	signer, err := keys.GenerateEcdsaKey()
+	c.Assert(err, IsNil)
+	type deprecatedP256Verifier struct {
+		PublicKey data.HexBytes `json:"public"`
+	}
+	pub := signer.PublicKey
+	keyValBytes, err := json.Marshal(&deprecatedP256Verifier{PublicKey: elliptic.Marshal(pub.Curve, pub.X, pub.Y)})
+	c.Assert(err, IsNil)
+	publicData := &data.PublicKey{
+		Type:       data.KeyTypeECDSA_SHA2_P256,
+		Scheme:     data.KeySchemeECDSA_SHA2_P256,
+		Algorithms: data.HashAlgorithms,
+		Value:      keyValBytes,
+	}
+	err = r.AddVerificationKey("root", publicData)
+	c.Assert(err, IsNil)
+	// Add other keys as normal
+	genKey(c, r, "targets")
+	genKey(c, r, "snapshot")
+	genKey(c, r, "timestamp")
+	c.Assert(r.AddTarget("foo.txt", nil), IsNil)
+
+	// Sign the root role manually
+	rootMeta, err := r.SignedMeta("root.json")
+	c.Assert(err, IsNil)
+	rootCanonical, err := cjson.EncodeCanonical(rootMeta.Signed)
+	c.Assert(err, IsNil)
+	hash := sha256.Sum256(rootCanonical)
+	rootSig, err := signer.PrivateKey.Sign(rand.Reader, hash[:], crypto.SHA256)
+	c.Assert(err, IsNil)
+	for _, id := range publicData.IDs() {
+		c.Assert(r.AddOrUpdateSignature("root.json", data.Signature{
+			KeyID:     id,
+			Signature: rootSig}), IsNil)
+	}
+
+	// Committing should succeed because the deprecated key pkg is added.
+	c.Assert(r.Snapshot(), IsNil)
+	c.Assert(r.Timestamp(), IsNil)
+	c.Assert(r.Commit(), IsNil)
+}
diff --git a/pkg/deprecated/set_ecdsa/set_ecdsa.go b/pkg/deprecated/set_ecdsa/set_ecdsa.go
new file mode 100644
index 0000000..de3771e
--- /dev/null
+++ b/pkg/deprecated/set_ecdsa/set_ecdsa.go
@@ -0,0 +1,23 @@
+package set_ecdsa
+
+import (
+	"errors"
+
+	"github.com/theupdateframework/go-tuf/data"
+	"github.com/theupdateframework/go-tuf/pkg/keys"
+)
+
+/*
+	Importing this package will allow support for both hex-encoded ECDSA
+	verifiers and PEM-encoded ECDSA verifiers.
+	Note that this package imports "github.com/theupdateframework/go-tuf/pkg/keys"
+	and overrides the ECDSA verifier loaded at init time in that package.
+*/
+
+func init() {
+	_, ok := keys.VerifierMap.Load(data.KeyTypeECDSA_SHA2_P256)
+	if !ok {
+		panic(errors.New("expected to override previously loaded PEM-only ECDSA verifier"))
+	}
+	keys.VerifierMap.Store(data.KeyTypeECDSA_SHA2_P256, keys.NewDeprecatedEcdsaVerifier)
+}
diff --git a/pkg/keys/deprecated_ecdsa.go b/pkg/keys/deprecated_ecdsa.go
new file mode 100644
index 0000000..4a8f151
--- /dev/null
+++ b/pkg/keys/deprecated_ecdsa.go
@@ -0,0 +1,103 @@
+package keys
+
+import (
+	"bytes"
+	"crypto/ecdsa"
+	"crypto/elliptic"
+	"crypto/sha256"
+	"encoding/json"
+	"errors"
+	"fmt"
+	"io"
+	"os"
+
+	"github.com/theupdateframework/go-tuf/data"
+)
+
+func NewDeprecatedEcdsaVerifier() Verifier {
+	return &ecdsaVerifierWithDeprecatedSupport{}
+}
+
+type ecdsaVerifierWithDeprecatedSupport struct {
+	key *data.PublicKey
+	// This will switch based on whether this is a PEM-encoded key
+	// or a deprecated hex-encoded key.
+	Verifier
+}
+
+func (p *ecdsaVerifierWithDeprecatedSupport) UnmarshalPublicKey(key *data.PublicKey) error {
+	p.key = key
+	pemVerifier := &EcdsaVerifier{}
+	if err := pemVerifier.UnmarshalPublicKey(key); err != nil {
+		// Try the deprecated hex-encoded verifier
+		hexVerifier := &deprecatedP256Verifier{}
+		if err := hexVerifier.UnmarshalPublicKey(key); err != nil {
+			return err
+		}
+		p.Verifier = hexVerifier
+		return nil
+	}
+	p.Verifier = pemVerifier
+	return nil
+}
+
+/*
+   Deprecated ecdsaVerifier that used hex-encoded public keys.
+   This MAY be used to verify existing metadata that used this
+   old format. This will be deprecated soon, ensure that repositories
+   are re-signed and clients receieve a fully compliant root.
+*/
+
+type deprecatedP256Verifier struct {
+	PublicKey data.HexBytes `json:"public"`
+	key       *data.PublicKey
+}
+
+func (p *deprecatedP256Verifier) Public() string {
+	return p.PublicKey.String()
+}
+
+func (p *deprecatedP256Verifier) Verify(msg, sigBytes []byte) error {
+	x, y := elliptic.Unmarshal(elliptic.P256(), p.PublicKey)
+	k := &ecdsa.PublicKey{
+		Curve: elliptic.P256(),
+		X:     x,
+		Y:     y,
+	}
+
+	hash := sha256.Sum256(msg)
+
+	if !ecdsa.VerifyASN1(k, hash[:], sigBytes) {
+		return errors.New("tuf: deprecated ecdsa signature verification failed")
+	}
+	return nil
+}
+
+func (p *deprecatedP256Verifier) MarshalPublicKey() *data.PublicKey {
+	return p.key
+}
+
+func (p *deprecatedP256Verifier) UnmarshalPublicKey(key *data.PublicKey) error {
+	// Prepare decoder limited to 512Kb
+	dec := json.NewDecoder(io.LimitReader(bytes.NewReader(key.Value), MaxJSONKeySize))
+
+	// Unmarshal key value
+	if err := dec.Decode(p); err != nil {
+		if errors.Is(err, io.EOF) || errors.Is(err, io.ErrUnexpectedEOF) {
+			return fmt.Errorf("tuf: the public key is truncated or too large: %w", err)
+		}
+		return err
+	}
+
+	curve := elliptic.P256()
+
+	// Parse as uncompressed marshalled point.
+	x, _ := elliptic.Unmarshal(curve, p.PublicKey)
+	if x == nil {
+		return errors.New("tuf: invalid ecdsa public key point")
+	}
+
+	p.key = key
+	fmt.Fprintln(os.Stderr, "tuf: warning using deprecated ecdsa hex-encoded keys")
+	return nil
+}
diff --git a/pkg/keys/deprecated_ecdsa_test.go b/pkg/keys/deprecated_ecdsa_test.go
new file mode 100644
index 0000000..ddfaa84
--- /dev/null
+++ b/pkg/keys/deprecated_ecdsa_test.go
@@ -0,0 +1,129 @@
+package keys
+
+import (
+	"crypto"
+	"crypto/ecdsa"
+	"crypto/elliptic"
+	"crypto/rand"
+	"crypto/sha256"
+	"encoding/json"
+	"errors"
+
+	"github.com/theupdateframework/go-tuf/data"
+	. "gopkg.in/check.v1"
+)
+
+type DeprecatedECDSASuite struct{}
+
+var _ = Suite(DeprecatedECDSASuite{})
+
+type deprecatedEcdsaSigner struct {
+	*ecdsa.PrivateKey
+}
+
+type deprecatedEcdsaPublic struct {
+	PublicKey data.HexBytes `json:"public"`
+}
+
+func (s deprecatedEcdsaSigner) PublicData() *data.PublicKey {
+	pub := s.Public().(*ecdsa.PublicKey)
+	keyValBytes, _ := json.Marshal(deprecatedEcdsaPublic{
+		PublicKey: elliptic.Marshal(pub.Curve, pub.X, pub.Y)})
+	return &data.PublicKey{
+		Type:       data.KeyTypeECDSA_SHA2_P256,
+		Scheme:     data.KeySchemeECDSA_SHA2_P256,
+		Algorithms: data.HashAlgorithms,
+		Value:      keyValBytes,
+	}
+}
+
+func (s deprecatedEcdsaSigner) SignMessage(message []byte) ([]byte, error) {
+	hash := sha256.Sum256(message)
+	return s.PrivateKey.Sign(rand.Reader, hash[:], crypto.SHA256)
+}
+
+func (s deprecatedEcdsaSigner) ContainsID(id string) bool {
+	return s.PublicData().ContainsID(id)
+}
+
+func (deprecatedEcdsaSigner) MarshalPrivateKey() (*data.PrivateKey, error) {
+	return nil, errors.New("not implemented for test")
+}
+
+func (deprecatedEcdsaSigner) UnmarshalPrivateKey(key *data.PrivateKey) error {
+	return errors.New("not implemented for test")
+}
+
+func generatedDeprecatedSigner() (*deprecatedEcdsaSigner, error) {
+	privkey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	if err != nil {
+		return nil, err
+	}
+	return &deprecatedEcdsaSigner{privkey}, nil
+}
+
+func (DeprecatedECDSASuite) TestSignVerifyDeprecatedFormat(c *C) {
+	// Create an ecdsa key with a deprecated format.
+	signer, err := generatedDeprecatedSigner()
+	c.Assert(err, IsNil)
+	msg := []byte("foo")
+	sig, err := signer.SignMessage(msg)
+	c.Assert(err, IsNil)
+
+	pub := signer.PublicKey
+
+	keyValBytes, err := json.Marshal(&deprecatedP256Verifier{PublicKey: elliptic.Marshal(pub.Curve, pub.X, pub.Y)})
+	c.Assert(err, IsNil)
+	publicData := &data.PublicKey{
+		Type:       data.KeyTypeECDSA_SHA2_P256,
+		Scheme:     data.KeySchemeECDSA_SHA2_P256,
+		Algorithms: data.HashAlgorithms,
+		Value:      keyValBytes,
+	}
+
+	deprecatedEcdsa := NewDeprecatedEcdsaVerifier()
+	err = deprecatedEcdsa.UnmarshalPublicKey(publicData)
+	c.Assert(err, IsNil)
+	c.Assert(deprecatedEcdsa.Verify(msg, sig), IsNil)
+}
+
+func (DeprecatedECDSASuite) TestECDSAVerifyMismatchMessage(c *C) {
+	signer, err := generatedDeprecatedSigner()
+	c.Assert(err, IsNil)
+	msg := []byte("foo")
+	sig, err := signer.SignMessage(msg)
+	c.Assert(err, IsNil)
+	publicData := signer.PublicData()
+	deprecatedEcdsa := NewDeprecatedEcdsaVerifier()
+	err = deprecatedEcdsa.UnmarshalPublicKey(publicData)
+	c.Assert(err, IsNil)
+	c.Assert(deprecatedEcdsa.Verify([]byte("notfoo"), sig), ErrorMatches, "tuf: deprecated ecdsa signature verification failed")
+}
+
+func (DeprecatedECDSASuite) TestECDSAVerifyMismatchPubKey(c *C) {
+	signer, err := generatedDeprecatedSigner()
+	c.Assert(err, IsNil)
+	msg := []byte("foo")
+	sig, err := signer.SignMessage(msg)
+	c.Assert(err, IsNil)
+
+	signerNew, err := generatedDeprecatedSigner()
+	c.Assert(err, IsNil)
+	deprecatedEcdsa := NewDeprecatedEcdsaVerifier()
+	err = deprecatedEcdsa.UnmarshalPublicKey(signerNew.PublicData())
+	c.Assert(err, IsNil)
+	c.Assert(deprecatedEcdsa.Verify([]byte("notfoo"), sig), ErrorMatches, "tuf: deprecated ecdsa signature verification failed")
+}
+
+func (DeprecatedECDSASuite) TestMarshalUnmarshalPublicKey(c *C) {
+	signer, err := generatedDeprecatedSigner()
+	c.Assert(err, IsNil)
+
+	pub := signer.PublicData()
+
+	deprecatedEcdsa := NewDeprecatedEcdsaVerifier()
+	err = deprecatedEcdsa.UnmarshalPublicKey(pub)
+	c.Assert(err, IsNil)
+
+	c.Assert(deprecatedEcdsa.MarshalPublicKey(), DeepEquals, pub)
+}
diff --git a/pkg/keys/ecdsa.go b/pkg/keys/ecdsa.go
index 22ed12b..ee93e33 100644
--- a/pkg/keys/ecdsa.go
+++ b/pkg/keys/ecdsa.go
@@ -4,64 +4,65 @@
 	"bytes"
 	"crypto/ecdsa"
 	"crypto/elliptic"
+	"crypto/rand"
 	"crypto/sha256"
-	"encoding/asn1"
+	"crypto/x509"
 	"encoding/json"
+	"encoding/pem"
 	"errors"
 	"fmt"
 	"io"
-	"math/big"
 
 	"github.com/theupdateframework/go-tuf/data"
 )
 
 func init() {
-	VerifierMap.Store(data.KeyTypeECDSA_SHA2_P256, NewEcdsaVerifier)
+	// Note: we use LoadOrStore here to prevent accidentally overriding the
+	// an explicit deprecated ECDSA verifier.
+	// TODO: When deprecated ECDSA is removed, this can switch back to Store.
+	VerifierMap.LoadOrStore(data.KeyTypeECDSA_SHA2_P256, NewEcdsaVerifier)
+	SignerMap.Store(data.KeyTypeECDSA_SHA2_P256, newEcdsaSigner)
 }
 
 func NewEcdsaVerifier() Verifier {
-	return &p256Verifier{}
+	return &EcdsaVerifier{}
 }
 
-type ecdsaSignature struct {
-	R, S *big.Int
+func newEcdsaSigner() Signer {
+	return &ecdsaSigner{}
 }
 
-type p256Verifier struct {
-	PublicKey data.HexBytes `json:"public"`
+type EcdsaVerifier struct {
+	PublicKey *PKIXPublicKey `json:"public"`
+	ecdsaKey  *ecdsa.PublicKey
 	key       *data.PublicKey
 }
 
-func (p *p256Verifier) Public() string {
-	return p.PublicKey.String()
+func (p *EcdsaVerifier) Public() string {
+	// This is already verified to succeed when unmarshalling a public key.
+	r, err := x509.MarshalPKIXPublicKey(p.ecdsaKey)
+	if err != nil {
+		// TODO: Gracefully handle these errors.
+		// See https://github.com/theupdateframework/go-tuf/issues/363
+		panic(err)
+	}
+	return string(r)
 }
 
-func (p *p256Verifier) Verify(msg, sigBytes []byte) error {
-	x, y := elliptic.Unmarshal(elliptic.P256(), p.PublicKey)
-	k := &ecdsa.PublicKey{
-		Curve: elliptic.P256(),
-		X:     x,
-		Y:     y,
-	}
-
-	var sig ecdsaSignature
-	if _, err := asn1.Unmarshal(sigBytes, &sig); err != nil {
-		return err
-	}
-
+func (p *EcdsaVerifier) Verify(msg, sigBytes []byte) error {
 	hash := sha256.Sum256(msg)
 
-	if !ecdsa.Verify(k, hash[:], sig.R, sig.S) {
+	if !ecdsa.VerifyASN1(p.ecdsaKey, hash[:], sigBytes) {
 		return errors.New("tuf: ecdsa signature verification failed")
 	}
 	return nil
 }
 
-func (p *p256Verifier) MarshalPublicKey() *data.PublicKey {
+func (p *EcdsaVerifier) MarshalPublicKey() *data.PublicKey {
 	return p.key
 }
 
-func (p *p256Verifier) UnmarshalPublicKey(key *data.PublicKey) error {
+func (p *EcdsaVerifier) UnmarshalPublicKey(key *data.PublicKey) error {
 	// Prepare decoder limited to 512Kb
 	dec := json.NewDecoder(io.LimitReader(bytes.NewReader(key.Value), MaxJSONKeySize))
 
@@ -73,14 +74,98 @@
 		return err
 	}
 
-	curve := elliptic.P256()
-
-	// Parse as uncompressed marshalled point.
-	x, _ := elliptic.Unmarshal(curve, p.PublicKey)
-	if x == nil {
-		return errors.New("tuf: invalid ecdsa public key point")
+	ecdsaKey, ok := p.PublicKey.PublicKey.(*ecdsa.PublicKey)
+	if !ok {
+		return fmt.Errorf("invalid public key")
 	}
 
+	if _, err := x509.MarshalPKIXPublicKey(ecdsaKey); err != nil {
+		return fmt.Errorf("marshalling to PKIX key: invalid public key")
+	}
+
+	p.ecdsaKey = ecdsaKey
 	p.key = key
 	return nil
 }
+
+type ecdsaSigner struct {
+	*ecdsa.PrivateKey
+}
+
+type ecdsaPrivateKeyValue struct {
+	Private string         `json:"private"`
+	Public  *PKIXPublicKey `json:"public"`
+}
+
+func (s *ecdsaSigner) PublicData() *data.PublicKey {
+	// This uses a trusted public key JSON format with a trusted Public value.
+	keyValBytes, _ := json.Marshal(EcdsaVerifier{PublicKey: &PKIXPublicKey{PublicKey: s.Public()}})
+	return &data.PublicKey{
+		Type:       data.KeyTypeECDSA_SHA2_P256,
+		Scheme:     data.KeySchemeECDSA_SHA2_P256,
+		Algorithms: data.HashAlgorithms,
+		Value:      keyValBytes,
+	}
+}
+
+func (s *ecdsaSigner) SignMessage(message []byte) ([]byte, error) {
+	hash := sha256.Sum256(message)
+	return ecdsa.SignASN1(rand.Reader, s.PrivateKey, hash[:])
+}
+
+func (s *ecdsaSigner) MarshalPrivateKey() (*data.PrivateKey, error) {
+	priv, err := x509.MarshalECPrivateKey(s.PrivateKey)
+	if err != nil {
+		return nil, err
+	}
+	pemKey := pem.EncodeToMemory(&pem.Block{Type: "EC PRIVATE KEY", Bytes: priv})
+	val, err := json.Marshal(ecdsaPrivateKeyValue{
+		Private: string(pemKey),
+		Public:  &PKIXPublicKey{PublicKey: s.Public()},
+	})
+	if err != nil {
+		return nil, err
+	}
+	return &data.PrivateKey{
+		Type:       data.KeyTypeECDSA_SHA2_P256,
+		Scheme:     data.KeySchemeECDSA_SHA2_P256,
+		Algorithms: data.HashAlgorithms,
+		Value:      val,
+	}, nil
+}
+
+func (s *ecdsaSigner) UnmarshalPrivateKey(key *data.PrivateKey) error {
+	val := ecdsaPrivateKeyValue{}
+	if err := json.Unmarshal(key.Value, &val); err != nil {
+		return err
+	}
+	block, _ := pem.Decode([]byte(val.Private))
+	if block == nil {
+		return errors.New("invalid PEM value")
+	}
+	if block.Type != "EC PRIVATE KEY" {
+		return fmt.Errorf("invalid block type: %s", block.Type)
+	}
+	k, err := x509.ParseECPrivateKey(block.Bytes)
+	if err != nil {
+		return err
+	}
+	if k.Curve != elliptic.P256() {
+		return errors.New("unsupported ecdsa curve")
+	}
+	if _, err := json.Marshal(EcdsaVerifier{
+		PublicKey: &PKIXPublicKey{PublicKey: k.Public()}}); err != nil {
+		return fmt.Errorf("invalid public key: %s", err)
+	}
+
+	s.PrivateKey = k
+	return nil
+}
+
+func GenerateEcdsaKey() (*ecdsaSigner, error) {
+	privkey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	if err != nil {
+		return nil, err
+	}
+	return &ecdsaSigner{privkey}, nil
+}
diff --git a/pkg/keys/ecdsa_test.go b/pkg/keys/ecdsa_test.go
index a141d58..2fe6348 100644
--- a/pkg/keys/ecdsa_test.go
+++ b/pkg/keys/ecdsa_test.go
@@ -17,28 +17,99 @@
 
 type ECDSASuite struct{}
 
-var _ = Suite(&ECDSASuite{})
+var _ = Suite(ECDSASuite{})
+
+func (ECDSASuite) TestSignVerify(c *C) {
+	signer, err := GenerateEcdsaKey()
+	c.Assert(err, IsNil)
+	msg := []byte("foo")
+	sig, err := signer.SignMessage(msg)
+	c.Assert(err, IsNil)
+	publicData := signer.PublicData()
+	pubKey, err := GetVerifier(publicData)
+	c.Assert(err, IsNil)
+	c.Assert(pubKey.Verify(msg, sig), IsNil)
+}
+
+func (ECDSASuite) TestECDSAVerifyMismatchMessage(c *C) {
+	signer, err := GenerateEcdsaKey()
+	c.Assert(err, IsNil)
+	msg := []byte("foo")
+	sig, err := signer.SignMessage(msg)
+	c.Assert(err, IsNil)
+	publicData := signer.PublicData()
+	pubKey, err := GetVerifier(publicData)
+	c.Assert(err, IsNil)
+	c.Assert(pubKey.Verify([]byte("notfoo"), sig), ErrorMatches, "tuf: ecdsa signature verification failed")
+}
+
+func (ECDSASuite) TestECDSAVerifyMismatchPubKey(c *C) {
+	signer, err := GenerateEcdsaKey()
+	c.Assert(err, IsNil)
+	msg := []byte("foo")
+	sig, err := signer.SignMessage(msg)
+	c.Assert(err, IsNil)
+
+	signerNew, err := GenerateEcdsaKey()
+	c.Assert(err, IsNil)
+	pubKey, err := GetVerifier(signerNew.PublicData())
+	c.Assert(err, IsNil)
+	c.Assert(pubKey.Verify([]byte("notfoo"), sig), ErrorMatches, "tuf: ecdsa signature verification failed")
+}
+
+func (ECDSASuite) TestSignVerifyDeprecatedFails(c *C) {
+	// Create an ecdsa key with a deprecated format.
+	signer, err := GenerateEcdsaKey()
+	c.Assert(err, IsNil)
+
+	type deprecatedP256Verifier struct {
+		PublicKey data.HexBytes `json:"public"`
+	}
+	pub := signer.PublicKey
+	keyValBytes, err := json.Marshal(&deprecatedP256Verifier{PublicKey: elliptic.Marshal(pub.Curve, pub.X, pub.Y)})
+	c.Assert(err, IsNil)
+	publicData := &data.PublicKey{
+		Type:       data.KeyTypeECDSA_SHA2_P256,
+		Scheme:     data.KeySchemeECDSA_SHA2_P256,
+		Algorithms: data.HashAlgorithms,
+		Value:      keyValBytes,
+	}
+
+	_, err = GetVerifier(publicData)
+	c.Assert(err, ErrorMatches, "tuf: error unmarshalling key: invalid PEM value")
+}
+
+func (ECDSASuite) TestMarshalUnmarshalPublicKey(c *C) {
+	signer, err := GenerateEcdsaKey()
+	c.Assert(err, IsNil)
+	publicData := signer.PublicData()
+	pubKey, err := GetVerifier(publicData)
+	c.Assert(err, IsNil)
+	c.Assert(pubKey.MarshalPublicKey(), DeepEquals, publicData)
+}
+
+func (ECDSASuite) TestMarshalUnmarshalPrivateKey(c *C) {
+	signer, err := GenerateEcdsaKey()
+	c.Assert(err, IsNil)
+	privateData, err := signer.MarshalPrivateKey()
+	c.Assert(err, IsNil)
+	c.Assert(privateData.Type, Equals, data.KeyTypeECDSA_SHA2_P256)
+	c.Assert(privateData.Scheme, Equals, data.KeySchemeECDSA_SHA2_P256)
+	c.Assert(privateData.Algorithms, DeepEquals, data.HashAlgorithms)
+	s, err := GetSigner(privateData)
+	c.Assert(err, IsNil)
+	c.Assert(s, DeepEquals, signer)
+}
 
 func (ECDSASuite) TestUnmarshalECDSA(c *C) {
 	priv, err := ecdsa.GenerateKey(elliptic.P256(), strings.NewReader("00001-deterministic-buffer-for-key-generation"))
 	c.Assert(err, IsNil)
 
-	// Marshall as non compressed point
-	pub := elliptic.Marshal(elliptic.P256(), priv.X, priv.Y)
+	signer := &ecdsaSigner{priv}
+	goodKey := signer.PublicData()
 
-	publicKey, err := json.Marshal(map[string]string{
-		"public": hex.EncodeToString(pub),
-	})
-	c.Assert(err, IsNil)
-
-	badKey := &data.PublicKey{
-		Type:       data.KeyTypeECDSA_SHA2_P256,
-		Scheme:     data.KeySchemeECDSA_SHA2_P256,
-		Algorithms: data.HashAlgorithms,
-		Value:      publicKey,
-	}
 	verifier := NewEcdsaVerifier()
-	c.Assert(verifier.UnmarshalPublicKey(badKey), IsNil)
+	c.Assert(verifier.UnmarshalPublicKey(goodKey), IsNil)
 }
 
 func (ECDSASuite) TestUnmarshalECDSA_Invalid(c *C) {
diff --git a/pkg/keys/ed25519.go b/pkg/keys/ed25519.go
index 82178f5..1e4c66c 100644
--- a/pkg/keys/ed25519.go
+++ b/pkg/keys/ed25519.go
@@ -15,8 +15,8 @@
 )
 
 func init() {
-	SignerMap.Store(data.KeySchemeEd25519, NewEd25519Signer)
-	VerifierMap.Store(data.KeySchemeEd25519, NewEd25519Verifier)
+	SignerMap.Store(data.KeyTypeEd25519, NewEd25519Signer)
+	VerifierMap.Store(data.KeyTypeEd25519, NewEd25519Verifier)
 }
 
 func NewEd25519Signer() Signer {
@@ -73,10 +73,6 @@
 
 type ed25519Signer struct {
 	ed25519.PrivateKey
-
-	keyType       string
-	keyScheme     string
-	keyAlgorithms []string
 }
 
 func GenerateEd25519Key() (*ed25519Signer, error) {
@@ -88,19 +84,13 @@
 		return nil, err
 	}
 	return &ed25519Signer{
-		PrivateKey:    ed25519.PrivateKey(data.HexBytes(private)),
-		keyType:       data.KeyTypeEd25519,
-		keyScheme:     data.KeySchemeEd25519,
-		keyAlgorithms: data.HashAlgorithms,
+		PrivateKey: ed25519.PrivateKey(data.HexBytes(private)),
 	}, nil
 }
 
 func NewEd25519SignerFromKey(keyValue Ed25519PrivateKeyValue) *ed25519Signer {
 	return &ed25519Signer{
-		PrivateKey:    ed25519.PrivateKey(data.HexBytes(keyValue.Private)),
-		keyType:       data.KeyTypeEd25519,
-		keyScheme:     data.KeySchemeEd25519,
-		keyAlgorithms: data.HashAlgorithms,
+		PrivateKey: ed25519.PrivateKey(data.HexBytes(keyValue.Private)),
 	}
 }
 
@@ -117,9 +107,9 @@
 		return nil, err
 	}
 	return &data.PrivateKey{
-		Type:       e.keyType,
-		Scheme:     e.keyScheme,
-		Algorithms: e.keyAlgorithms,
+		Type:       data.KeyTypeEd25519,
+		Scheme:     data.KeySchemeEd25519,
+		Algorithms: data.HashAlgorithms,
 		Value:      valueBytes,
 	}, nil
 }
@@ -155,10 +145,7 @@
 
 	// Prepare signer
 	*e = ed25519Signer{
-		PrivateKey:    ed25519.PrivateKey(data.HexBytes(keyValue.Private)),
-		keyType:       key.Type,
-		keyScheme:     key.Scheme,
-		keyAlgorithms: key.Algorithms,
+		PrivateKey: ed25519.PrivateKey(data.HexBytes(keyValue.Private)),
 	}
 	return nil
 }
@@ -166,9 +153,9 @@
 func (e *ed25519Signer) PublicData() *data.PublicKey {
 	keyValBytes, _ := json.Marshal(ed25519Verifier{PublicKey: []byte(e.PrivateKey.Public().(ed25519.PublicKey))})
 	return &data.PublicKey{
-		Type:       e.keyType,
-		Scheme:     e.keyScheme,
-		Algorithms: e.keyAlgorithms,
+		Type:       data.KeyTypeEd25519,
+		Scheme:     data.KeySchemeEd25519,
+		Algorithms: data.HashAlgorithms,
 		Value:      keyValBytes,
 	}
 }
diff --git a/pkg/keys/keys_test.go b/pkg/keys/keys_test.go
index 956a318..c1a7d01 100644
--- a/pkg/keys/keys_test.go
+++ b/pkg/keys/keys_test.go
@@ -3,6 +3,7 @@
 import (
 	"testing"
 
+	"github.com/theupdateframework/go-tuf/data"
 	. "gopkg.in/check.v1"
 )
 
@@ -32,7 +33,7 @@
 	c.Assert(err, IsNil)
 	privKey, err = signer.MarshalPrivateKey()
 	c.Assert(err, IsNil)
-	privKey.Algorithms = []string{}
+	privKey.Algorithms = []data.HashAlgorithm{}
 	err = signer.UnmarshalPrivateKey(privKey)
 	c.Assert(err, IsNil)
 }
diff --git a/pkg/keys/pkix.go b/pkg/keys/pkix.go
new file mode 100644
index 0000000..e58d4c9
--- /dev/null
+++ b/pkg/keys/pkix.go
@@ -0,0 +1,56 @@
+package keys
+
+import (
+	"bytes"
+	"crypto"
+	"crypto/x509"
+	"encoding/json"
+	"encoding/pem"
+	"errors"
+	"fmt"
+	"io"
+)
+
+type PKIXPublicKey struct {
+	crypto.PublicKey
+}
+
+func (p *PKIXPublicKey) MarshalJSON() ([]byte, error) {
+	bytes, err := x509.MarshalPKIXPublicKey(p.PublicKey)
+	if err != nil {
+		return nil, err
+	}
+	pemBytes := pem.EncodeToMemory(&pem.Block{
+		Type:  "PUBLIC KEY",
+		Bytes: bytes,
+	})
+	return json.Marshal(string(pemBytes))
+}
+
+func (p *PKIXPublicKey) UnmarshalJSON(b []byte) error {
+	var pemValue string
+	// Prepare decoder limited to 512Kb
+	dec := json.NewDecoder(io.LimitReader(bytes.NewReader(b), MaxJSONKeySize))
+
+	// Unmarshal key value
+	if err := dec.Decode(&pemValue); err != nil {
+		if errors.Is(err, io.EOF) || errors.Is(err, io.ErrUnexpectedEOF) {
+			return fmt.Errorf("tuf: the public key is truncated or too large: %w", err)
+		}
+		return err
+	}
+
+	block, _ := pem.Decode([]byte(pemValue))
+	if block == nil {
+		return errors.New("invalid PEM value")
+	}
+	if block.Type != "PUBLIC KEY" {
+		return fmt.Errorf("invalid block type: %s", block.Type)
+	}
+	pub, err := x509.ParsePKIXPublicKey(block.Bytes)
+	if err != nil {
+		return err
+	}
+	p.PublicKey = pub
+	return nil
+}
diff --git a/pkg/keys/pkix_test.go b/pkg/keys/pkix_test.go
new file mode 100644
index 0000000..4debdde
--- /dev/null
+++ b/pkg/keys/pkix_test.go
@@ -0,0 +1,62 @@
+package keys
+
+import (
+	"crypto/ecdsa"
+	"crypto/rand"
+	"crypto/x509"
+	"encoding/json"
+	"encoding/pem"
+	"errors"
+	"io"
+
+	. "gopkg.in/check.v1"
+)
+
+const ecdsaKey = `-----BEGIN PUBLIC KEY-----
+MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEftgasQA68yvumeXZmcOTSIHKfbmx
+WT1oYuRF0Un3tKxnzip6xAYwlz0Dt96DUh+0P7BruHH2O6s4MiRR9/TuNw==
+-----END PUBLIC KEY-----
+`
+
+type PKIXSuite struct{}
+
+var _ = Suite(&PKIXSuite{})
+
+func (PKIXSuite) TestMarshalJSON(c *C) {
+	block, _ := pem.Decode([]byte(ecdsaKey))
+	key, err := x509.ParsePKIXPublicKey(block.Bytes)
+	c.Assert(err, IsNil)
+	k := PKIXPublicKey{PublicKey: key}
+	buf, err := json.Marshal(&k)
+	c.Assert(err, IsNil)
+	var val string
+	err = json.Unmarshal(buf, &val)
+	c.Assert(err, IsNil)
+	c.Assert(val, Equals, ecdsaKey)
+}
+
+func (PKIXSuite) TestUnmarshalJSON(c *C) {
+	buf, err := json.Marshal(ecdsaKey)
+	c.Assert(err, IsNil)
+	var k PKIXPublicKey
+	err = json.Unmarshal(buf, &k)
+	c.Assert(err, IsNil)
+	c.Assert(k.PublicKey, FitsTypeOf, (*ecdsa.PublicKey)(nil))
+}
+
+func (PKIXSuite) TestUnmarshalPKIX_TooLongContent(c *C) {
+	randomSeed := make([]byte, MaxJSONKeySize)
+	_, err := io.ReadFull(rand.Reader, randomSeed)
+	c.Assert(err, IsNil)
+
+	pemBytes := pem.EncodeToMemory(&pem.Block{
+		Type:  "PUBLIC KEY",
+		Bytes: randomSeed,
+	})
+	tooLongPayload, err := json.Marshal(string(pemBytes))
+	c.Assert(err, IsNil)
+
+	var k PKIXPublicKey
+	err = json.Unmarshal(tooLongPayload, &k)
+	c.Assert(errors.Is(err, io.ErrUnexpectedEOF), Equals, true)
+}
diff --git a/pkg/keys/rsa.go b/pkg/keys/rsa.go
index 28c82d1..618f104 100644
--- a/pkg/keys/rsa.go
+++ b/pkg/keys/rsa.go
@@ -1,6 +1,7 @@
 package keys
 
 import (
+	"bytes"
 	"crypto"
 	"crypto/rand"
 	"crypto/rsa"
@@ -9,36 +10,38 @@
 	"encoding/json"
 	"encoding/pem"
 	"errors"
+	"fmt"
+	"io"
 
 	"github.com/theupdateframework/go-tuf/data"
 )
 
 func init() {
-	VerifierMap.Store(data.KeyTypeRSASSA_PSS_SHA256, NewRsaVerifier)
-	SignerMap.Store(data.KeyTypeRSASSA_PSS_SHA256, NewRsaSigner)
+	VerifierMap.Store(data.KeyTypeRSASSA_PSS_SHA256, newRsaVerifier)
+	SignerMap.Store(data.KeyTypeRSASSA_PSS_SHA256, newRsaSigner)
 }
 
-func NewRsaVerifier() Verifier {
+func newRsaVerifier() Verifier {
 	return &rsaVerifier{}
 }
 
-func NewRsaSigner() Signer {
+func newRsaSigner() Signer {
 	return &rsaSigner{}
 }
 
 type rsaVerifier struct {
-	PublicKey string `json:"public"`
+	PublicKey *PKIXPublicKey `json:"public"`
 	rsaKey    *rsa.PublicKey
 	key       *data.PublicKey
 }
 
 func (p *rsaVerifier) Public() string {
-	// Unique public key identifier, use a uniform encodng
+	// This is already verified to succeed when unmarshalling a public key.
 	r, err := x509.MarshalPKIXPublicKey(p.rsaKey)
 	if err != nil {
-		// This shouldn't happen with a valid rsa key, but fallback on the
-		// JSON public key string
-		return string(p.PublicKey)
+		// TODO: Gracefully handle these errors.
+		// See https://github.com/theupdateframework/go-tuf/issues/363
+		panic(err)
 	}
 	return string(r)
 }
@@ -54,56 +57,42 @@
 }
 
 func (p *rsaVerifier) UnmarshalPublicKey(key *data.PublicKey) error {
-	if err := json.Unmarshal(key.Value, p); err != nil {
+	// Prepare decoder limited to 512Kb
+	dec := json.NewDecoder(io.LimitReader(bytes.NewReader(key.Value), MaxJSONKeySize))
+
+	// Unmarshal key value
+	if err := dec.Decode(p); err != nil {
+		if errors.Is(err, io.EOF) || errors.Is(err, io.ErrUnexpectedEOF) {
+			return fmt.Errorf("tuf: the public key is truncated or too large: %w", err)
+		}
 		return err
 	}
-	var err error
-	p.rsaKey, err = parseKey(p.PublicKey)
-	if err != nil {
-		return err
+
+	rsaKey, ok := p.PublicKey.PublicKey.(*rsa.PublicKey)
+	if !ok {
+		return fmt.Errorf("invalid public key")
 	}
+
+	if _, err := x509.MarshalPKIXPublicKey(rsaKey); err != nil {
+		return fmt.Errorf("marshalling to PKIX key: invalid public key")
+	}
+
+	p.rsaKey = rsaKey
 	p.key = key
 	return nil
 }
 
-// parseKey tries to parse a PEM []byte slice by attempting PKCS1 and PKIX in order.
-func parseKey(data string) (*rsa.PublicKey, error) {
-	block, _ := pem.Decode([]byte(data))
-	if block == nil {
-		return nil, errors.New("tuf: pem decoding public key failed")
-	}
-	rsaPub, err := x509.ParsePKCS1PublicKey(block.Bytes)
-	if err == nil {
-		return rsaPub, nil
-	}
-	key, err := x509.ParsePKIXPublicKey(block.Bytes)
-	if err == nil {
-		rsaPub, ok := key.(*rsa.PublicKey)
-		if !ok {
-			return nil, errors.New("tuf: invalid rsa key")
-		}
-		return rsaPub, nil
-	}
-	return nil, errors.New("tuf: error unmarshalling rsa key")
-}
-
 type rsaSigner struct {
 	*rsa.PrivateKey
 }
 
-type rsaPublic struct {
-	// PEM encoded public key.
-	PublicKey string `json:"public"`
+type rsaPrivateKeyValue struct {
+	Private string         `json:"private"`
+	Public  *PKIXPublicKey `json:"public"`
 }
 
 func (s *rsaSigner) PublicData() *data.PublicKey {
-	pub, _ := x509.MarshalPKIXPublicKey(s.Public().(*rsa.PublicKey))
-	pubBytes := pem.EncodeToMemory(&pem.Block{
-		Type:  "RSA PUBLIC KEY",
-		Bytes: pub,
-	})
-
-	keyValBytes, _ := json.Marshal(rsaPublic{PublicKey: string(pubBytes)})
+	keyValBytes, _ := json.Marshal(rsaVerifier{PublicKey: &PKIXPublicKey{PublicKey: s.Public()}})
 	return &data.PublicKey{
 		Type:       data.KeyTypeRSASSA_PSS_SHA256,
 		Scheme:     data.KeySchemeRSASSA_PSS_SHA256,
@@ -122,11 +111,46 @@
 }
 
 func (s *rsaSigner) MarshalPrivateKey() (*data.PrivateKey, error) {
-	return nil, errors.New("not implemented for test")
+	priv := x509.MarshalPKCS1PrivateKey(s.PrivateKey)
+	pemKey := pem.EncodeToMemory(&pem.Block{Type: "RSA PRIVATE KEY", Bytes: priv})
+	val, err := json.Marshal(rsaPrivateKeyValue{
+		Private: string(pemKey),
+		Public:  &PKIXPublicKey{PublicKey: s.Public()},
+	})
+	if err != nil {
+		return nil, err
+	}
+	return &data.PrivateKey{
+		Type:       data.KeyTypeRSASSA_PSS_SHA256,
+		Scheme:     data.KeySchemeRSASSA_PSS_SHA256,
+		Algorithms: data.HashAlgorithms,
+		Value:      val,
+	}, nil
 }
 
 func (s *rsaSigner) UnmarshalPrivateKey(key *data.PrivateKey) error {
-	return errors.New("not implemented for test")
+	val := rsaPrivateKeyValue{}
+	if err := json.Unmarshal(key.Value, &val); err != nil {
+		return err
+	}
+	block, _ := pem.Decode([]byte(val.Private))
+	if block == nil {
+		return errors.New("invalid PEM value")
+	}
+	if block.Type != "RSA PRIVATE KEY" {
+		return fmt.Errorf("invalid block type: %s", block.Type)
+	}
+	k, err := x509.ParsePKCS1PrivateKey(block.Bytes)
+	if err != nil {
+		return err
+	}
+	if _, err := json.Marshal(rsaVerifier{
+		PublicKey: &PKIXPublicKey{PublicKey: k.Public()}}); err != nil {
+		return fmt.Errorf("invalid public key: %s", err)
+	}
+
+	s.PrivateKey = k
+	return nil
 }
 
 func GenerateRsaKey() (*rsaSigner, error) {
diff --git a/pkg/keys/rsa_test.go b/pkg/keys/rsa_test.go
index d0e3e86..7352000 100644
--- a/pkg/keys/rsa_test.go
+++ b/pkg/keys/rsa_test.go
@@ -1,6 +1,13 @@
 package keys
 
 import (
+	"crypto/rand"
+	"encoding/hex"
+	"encoding/json"
+	"errors"
+	"io"
+
+	"github.com/theupdateframework/go-tuf/data"
 	. "gopkg.in/check.v1"
 )
 
@@ -20,7 +27,34 @@
 	c.Assert(pubKey.Verify(msg, sig), IsNil)
 }
 
-func (RsaSuite) TestMarshalUnmarshal(c *C) {
+func (RsaSuite) TestRSAVerifyMismatchMessage(c *C) {
+	signer, err := GenerateRsaKey()
+	c.Assert(err, IsNil)
+	msg := []byte("foo")
+	sig, err := signer.SignMessage(msg)
+	c.Assert(err, IsNil)
+	publicData := signer.PublicData()
+	pubKey, err := GetVerifier(publicData)
+	c.Assert(err, IsNil)
+	c.Assert(pubKey.Verify([]byte("notfoo"), sig), ErrorMatches, "crypto/rsa: verification error")
+}
+
+func (RsaSuite) TestRSAVerifyMismatchPubKey(c *C) {
+	signer, err := GenerateRsaKey()
+	c.Assert(err, IsNil)
+	msg := []byte("foo")
+	sig, err := signer.SignMessage(msg)
+	c.Assert(err, IsNil)
+
+	signerNew, err := GenerateRsaKey()
+	c.Assert(err, IsNil)
+
+	pubKey, err := GetVerifier(signerNew.PublicData())
+	c.Assert(err, IsNil)
+	c.Assert(pubKey.Verify([]byte("notfoo"), sig), ErrorMatches, "crypto/rsa: verification error")
+}
+
+func (RsaSuite) TestMarshalUnmarshalPublicKey(c *C) {
 	signer, err := GenerateRsaKey()
 	c.Assert(err, IsNil)
 	publicData := signer.PublicData()
@@ -28,3 +62,64 @@
 	c.Assert(err, IsNil)
 	c.Assert(pubKey.MarshalPublicKey(), DeepEquals, publicData)
 }
+
+func (RsaSuite) TestMarshalUnmarshalPrivateKey(c *C) {
+	signer, err := GenerateRsaKey()
+	c.Assert(err, IsNil)
+	privateData, err := signer.MarshalPrivateKey()
+	c.Assert(err, IsNil)
+	c.Assert(privateData.Type, Equals, data.KeyTypeRSASSA_PSS_SHA256)
+	c.Assert(privateData.Scheme, Equals, data.KeySchemeRSASSA_PSS_SHA256)
+	c.Assert(privateData.Algorithms, DeepEquals, data.HashAlgorithms)
+	s, err := GetSigner(privateData)
+	c.Assert(err, IsNil)
+	c.Assert(s, DeepEquals, signer)
+}
+
+func (ECDSASuite) TestUnmarshalRSA_Invalid(c *C) {
+	badKeyValue, err := json.Marshal(true)
+	c.Assert(err, IsNil)
+
+	badKey := &data.PublicKey{
+		Type:       data.KeyTypeECDSA_SHA2_P256,
+		Scheme:     data.KeySchemeECDSA_SHA2_P256,
+		Algorithms: data.HashAlgorithms,
+		Value:      badKeyValue,
+	}
+	verifier := NewEcdsaVerifier()
+	c.Assert(verifier.UnmarshalPublicKey(badKey), ErrorMatches, "json: cannot unmarshal.*")
+}
+
+func (ECDSASuite) TestUnmarshalRSAPublicKey(c *C) {
+	priv, err := GenerateRsaKey()
+	c.Assert(err, IsNil)
+
+	signer := &rsaSigner{priv.PrivateKey}
+	goodKey := signer.PublicData()
+
+	verifier := newRsaVerifier()
+	c.Assert(verifier.UnmarshalPublicKey(goodKey), IsNil)
+}
+
+func (ECDSASuite) TestUnmarshalRSA_TooLongContent(c *C) {
+	randomSeed := make([]byte, MaxJSONKeySize)
+	_, err := io.ReadFull(rand.Reader, randomSeed)
+	c.Assert(err, IsNil)
+
+	tooLongPayload, err := json.Marshal(
+		&ed25519Verifier{
+			PublicKey: data.HexBytes(hex.EncodeToString(randomSeed)),
+		},
+	)
+	c.Assert(err, IsNil)
+
+	badKey := &data.PublicKey{
+		Type:       data.KeyTypeECDSA_SHA2_P256,
+		Scheme:     data.KeySchemeECDSA_SHA2_P256,
+		Algorithms: data.HashAlgorithms,
+		Value:      tooLongPayload,
+	}
+	verifier := newRsaVerifier()
+	err = verifier.UnmarshalPublicKey(badKey)
+	c.Assert(errors.Is(err, io.ErrUnexpectedEOF), Equals, true)
+}
diff --git a/repo.go b/repo.go
index b4ca62b..603785f 100644
--- a/repo.go
+++ b/repo.go
@@ -309,19 +309,33 @@
 }
 
 func (r *Repo) GenKeyWithExpires(keyRole string, expires time.Time) (keyids []string, err error) {
+	return r.GenKeyWithSchemeAndExpires(keyRole, expires, data.KeySchemeEd25519)
+}
+
+func (r *Repo) GenKeyWithSchemeAndExpires(role string, expires time.Time, keyScheme data.KeyScheme) ([]string, error) {
+	var signer keys.Signer
+	var err error
+	switch keyScheme {
+	case data.KeySchemeEd25519:
+		signer, err = keys.GenerateEd25519Key()
+	case data.KeySchemeECDSA_SHA2_P256:
+		signer, err = keys.GenerateEcdsaKey()
+	case data.KeySchemeRSASSA_PSS_SHA256:
+		signer, err = keys.GenerateRsaKey()
+	default:
+		return nil, errors.New("unknown key type")
+	}
+	if err != nil {
+		return nil, err
+	}
+
 	// Not compatible with delegated targets roles, since delegated targets keys
 	// are associated with a delegation (edge), not a role (node).
 
-	signer, err := keys.GenerateEd25519Key()
-	if err != nil {
-		return []string{}, err
+	if err = r.AddPrivateKeyWithExpires(role, signer, expires); err != nil {
+		return nil, err
 	}
-
-	if err = r.AddPrivateKeyWithExpires(keyRole, signer, expires); err != nil {
-		return []string{}, err
-	}
-	keyids = signer.PublicData().IDs()
-	return
+	return signer.PublicData().IDs(), nil
 }
 
 func (r *Repo) AddPrivateKey(role string, signer keys.Signer) error {
diff --git a/repo_test.go b/repo_test.go
index 87536ee..2f3aebb 100644
--- a/repo_test.go
+++ b/repo_test.go
@@ -3,6 +3,7 @@
 import (
 	"bytes"
 	"crypto"
+	"crypto/elliptic"
 	"crypto/rand"
 	"encoding/hex"
 	"encoding/json"
@@ -2705,3 +2706,31 @@
 	c.Assert(json.Unmarshal(lengthMsg, &length), IsNil)
 	c.Assert(length, Equals, int64(0))
 }
+
+func (rs *RepoSuite) TestDeprecatedHexEncodedKeysFails(c *C) {
+	files := map[string][]byte{"foo.txt": []byte("foo")}
+	local := MemoryStore(make(map[string]json.RawMessage), files)
+	r, err := NewRepo(local)
+	c.Assert(err, IsNil)
+
+	r.Init(false)
+	// Add a root key with hex-encoded ecdsa format
+	signer, err := keys.GenerateEcdsaKey()
+	c.Assert(err, IsNil)
+	type deprecatedP256Verifier struct {
+		PublicKey data.HexBytes `json:"public"`
+	}
+	pub := signer.PublicKey
+	keyValBytes, err := json.Marshal(&deprecatedP256Verifier{PublicKey: elliptic.Marshal(pub.Curve, pub.X, pub.Y)})
+	c.Assert(err, IsNil)
+	publicData := &data.PublicKey{
+		Type:       data.KeyTypeECDSA_SHA2_P256,
+		Scheme:     data.KeySchemeECDSA_SHA2_P256,
+		Algorithms: data.HashAlgorithms,
+		Value:      keyValBytes,
+	}
+	err = r.AddVerificationKey("root", publicData)
+	c.Assert(err, IsNil)
+	// TODO: AddVerificationKey does no validation, so perform a sign operation.
+	c.Assert(r.Sign("root.json"), ErrorMatches, "tuf: error unmarshalling key: invalid PEM value")
+}
diff --git a/verify/db.go b/verify/db.go
index b67ff26..04f5bf1 100644
--- a/verify/db.go
+++ b/verify/db.go
@@ -55,7 +55,7 @@
 func (db *DB) AddKey(id string, k *data.PublicKey) error {
 	verifier, err := keys.GetVerifier(k)
 	if err != nil {
-		return ErrInvalidKey
+		return err // ErrInvalidKey
 	}
 
 	// TUF is considering in TAP-12 removing the
diff --git a/verify/verify_test.go b/verify/verify_test.go
index 4d9d379..afbf79d 100644
--- a/verify/verify_test.go
+++ b/verify/verify_test.go
@@ -31,12 +31,11 @@
 }
 
 type ecdsaPublic struct {
-	PublicKey data.HexBytes `json:"public"`
+	PublicKey *keys.PKIXPublicKey `json:"public"`
 }
 
 func (s ecdsaSigner) PublicData() *data.PublicKey {
-	pub := s.Public().(*ecdsa.PublicKey)
-	keyValBytes, _ := json.Marshal(ecdsaPublic{PublicKey: elliptic.Marshal(pub.Curve, pub.X, pub.Y)})
+	keyValBytes, _ := json.Marshal(ecdsaPublic{PublicKey: &keys.PKIXPublicKey{PublicKey: s.Public()}})
 	return &data.PublicKey{
 		Type:       data.KeyTypeECDSA_SHA2_P256,
 		Scheme:     data.KeySchemeECDSA_SHA2_P256,