Repository: theupdateframework/go-tuf
Pull request #357 (2022-08-17 19:31:56)

feat: Support ecdsa and RSA keys (#270 with backwards compatibility)
Please fill in the fields below to submit a pull request.  The more information that is provided, the better.

Fixes https://github.com/theupdateframework/go-tuf/issues/223
Release Notes: <!-- What comments/remarks should we include in the release notes for this change? -->
* breaking change: ECDSA public key verification now use TUF compliant PEM-encoding keys. For backwards compatibility with hex-encoded keys verification, client libraries must import `pkg/deprecated`
* feat: Adds support for ECDSA and RSA signers 
* feat: Repository CLI command allows a `--scheme` flag to specify key scheme.

**Types of changes**:
<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->
- [x] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [x] Breaking change (fix or feature that would cause existing functionality to not work as expected)

**Description of the changes being introduced by the pull request**:
* Defined new go types for KeyType, KeyScheme and HashAlgorithm, rather than just using strings, and fixed a few places where the key type and scheme had been mixed up.
* Added a new data.PKIXPublicKey type which abstracts away a lot of the boiler plate for marshalling/unmarshalling public keys.
* Completes support for the RSA signer and fixes public key encoding.

**Please verify and check that the pull request fulfills the following requirements**:

- [x] Tests have been added for the bug fix or new feature
- [ ] Docs have been added for the bug fix or new feature

cc @toby-jn: The only changes I made was adding deprecation support and updating your branch!

trishankatdatadog: So does this supersede #270 ?
asraa: > So does this supersede https://github.com/theupdateframework/go-tuf/pull/270 ?

Yes! It's all the changes there with backwards compatibility for just ECDSA verification: i.e. if a current root (sigstore) is using hex-encoded keys, then they will still be able to verify (no hex-encoded signers supported). Adds testing for making sure deprecation still verifies at the key and repo testing level.
asraa: I can also split out just the ECDSA changes. Hoping to get that in ASAP since the next sigstore TUF root to rotate those out relies on it.
toby-jn: Great thanks, LGTM. I did start trying to rebase my changes onto master over the weekend to make these updates, but there were some conflicts that I needed to go read up on and then real life got in the way, so thanks for taking this over.
trishankatdatadog: Thanks very much for all your hard work, Toby! Sorry I didn't get around to reviewing it in time: swamped by work, too...
asraa: @theupdateframework/go-tuf-maintainers could I please get one more review on this? really need this
trishankatdatadog: > @theupdateframework/go-tuf-maintainers could I please get one more review on this? really need this

Yes, sorry, was on PTO, will look tonight
asraa: No worries at all -- didn't realize that! Feel free to do it tomorrow, I hope you had some rest :)
asraa: > So just to double-check: what's the worst that can happen re:https://github.com/theupdateframework/go-tuf/issues/363? We are not able to accept malformed/unusable public keys after unmarshaling, correct?

Good question, and I'll attach my reply to the issue as well. Currently, we use those functions after we create a `verifier` in functionality like `keys.GetVerifier`
or a `signer` through `keys.GetSigner`

https://github.com/theupdateframework/go-tuf/blob/355e39cb2df220fc3961396a6d0e30bcf2c9ac12/pkg/keys/keys.go#L57-L69

So as long as they're verified during `UnmarshalPublicKey` or `UnmarshalPrivateKey` (which now they are), then we will always have a "well-formed" enough public key to `Marshal` or get the `PublicData()` from. 

In case someone accidentally corrupts those keys in the key store or TUF metadata, then we would return errors like `tuf: error unmarshalling key:` on loading them in as `Verifier` or `Signer`. 

So worst case, yes, the key becomes un-usable on load.

That being said, it would always be good practice to re-validate the keys upon marshalling even if they were loaded in a good state, just in case. (for the issue)
znewman01: ACK can look today (sorry, thought review was blocked for some reason)
asraa: > but for repos that already exist, old clients will be around for a long time. Do we have a migration story there? I guess just wait until hopefully all the old clients are gone and then cut over?

Right! So they can still use `pkg/deprecated` for verification, and can still construct . To migrate, they need to re-sign using the new keys, like we are doing in Sigstore. I could add the deprecated signer to `pkg/deprecated`, which would allow them to still `Sign` with the keys in their key store with the deprecated format. I think this is worth it so I'll add.
znewman01: > To migrate, they need to re-sign using the new keys, like we are doing in Sigstore.

Won't this immediately break every old client?

>  I could add the deprecated signer to pkg/deprecated[...]I think this is worth it so I'll add.

+1

> EDIT: I just realized that adding the deprecated signer AND verifier into the same package for compatibility is problematic for users who need to make the transition. E.g. I don't want the signer to ONLY be the deprecated version. So: the deprecated package should actually handle EITHER OR format during the transition. WDYT?

I'm not sure I'm parsing. I think there are the following possible behaviors:

For parsing:
- (default) only accept the new format
- (configurable) accept both old and new formats
- (could be an option, but I don't think we should add it) accept only the old format

This can probably be globally configurable, via the `init()` trick.

For writing new keys:
- (default) write keys in the new format
- (configurable) write keys in the old format

IMO we'd want to configure this on a key-by-key basis, rather than globally.

It'd be nice if we could do *both*, but I don't think that's possible.
asraa: > Won't this immediately break every old client?

Those old clients would need to import `pkg/deprecated` into their binaries. Originally I had deprecated support as a default, but then we opted to move it to an explicit package.

> +1

ECDSA Signing was never supported FYI. I just realized this as I was implementing. So I don't think it breaks old clients. They would have already implemented their own signing logic (as we did).

> (configurable) write keys in the old format

Don't think we need to opt to do this anymore, since it was never supported in the first place. go-tuf only had verification support for keys, which remains on import of `pkg/deprecated`

D: sorry this is confusing!
trishankatdatadog: > Those old clients would need to import `pkg/deprecated` into their binaries. Originally I had deprecated support as a default, but then we opted to move it to an explicit package.

Hold on: old clients that do not import `pkg/deprecated` will not break so long as the server keeps producing these deprecated-style public keys.
asraa: > Hold on: old clients that do not import pkg/deprecated will not break so long as the server keeps producing these deprecated-style public keys.

Maybe we're confused! Old clients that use old go-tuf certainly won't break because they're using an old go-tuf version. Clients that update their go-tuf dependency will break, because now ECDSA verifier expects TUF-compliant keys. Unless they import that deprecated package that understands both. RIght?
trishankatdatadog: > Maybe we're confused! Old clients that use old go-tuf certainly won't break because they're using an old go-tuf version. Clients that update their go-tuf dependency will break, because now ECDSA verifier expects TUF-compliant keys. Unless they import that deprecated package that understands both. RIght?

Oui. Sorry, just need to be super-clear here about what we meant about "old clients" here and so on. Thanks for the clarification!
znewman01: IIUC, serializing keys was never done *directly* by go-tuf. If that's true, then I am okay with 

(I'm a little confused by the discussion about signing and verifying—the signature format was always okay, right? We're just talking about signers/verifiers because that's the part of the codebase where key serialization happened?)

> > > To migrate, they need to re-sign using the new keys, like we are doing in Sigstore.
> >
> > Won't this immediately break every old client?
> 
> Those old clients would need to import `pkg/deprecated` into their binaries. Originally I had deprecated support as a default, but then we opted to move it to an explicit package.

Let me clarify: won't that immediately break every *deployed* client that's still using the old version of go-tuf?

I'm still fuzzy on the migration story. "Wait a long time before cutting over" isn't a great solution IMO; you typically don't want to break clients no matter how old.

I guess an alternative would be to replace the keys with some other key type (say, RSA for the sake of example) whose format was never affected. Then, we can distribute a new `5.root.json` with only RSA signatures (which cuts off the historical ECDSA-hex signatures, and *then* we could actually turn off the `pkg/deprecated` flag for parsing new clients. That brings the repo into compliance with the TUF spec (err, as long as you ignore everything before `5.root.json`).
asraa: > (I'm a little confused by the discussion about signing and verifying—the signature format was always okay, right? We're just talking about signers/verifiers because that's the part of the codebase where key serialization happened?)

Correct, signature format was always correct, it's just the way that the public key was serialized in the root metadata or how the private key was stored in the repo keystore.

> Let me clarify: won't that immediately break every deployed client that's still using the old version of go-tuf?

If they don't update their go-tuf version, they are unaffected: hex ecdsa keys will be the implementation they have in that go-tuf version. Maybe you're asking, if someone is using an old version old client, and the repository owner adds new ECDSA keys their metadata, will the old deployed client break? Yes, but that's akin to using a new feature, right? I'm not sure there's a way to get compatibility here.

> you typically don't want to break clients no matter how old.

I'd say specification deviation and lack of cross-language compatibility is one potential reason :) My feeling though is that this ONLY affected go-tuf clients who were implementing their own ECDSA signers that were compatible with the bad ECDSA verifiers in go-tuf, so they are already aware of managing this implementation.

>  Then, we can distribute a new 5.root.json with only RSA signatures (which cuts off the historical ECDSA-hex signatures, and then we could actually turn off the pkg/deprecated flag for parsing new clients. 

I see. The current plan would have already supported this, since the 5.root.json would have 5 old ECDSA signatures and 5 new ECDSA signatures. Old clients would think that the new keys are bogus, but the old keys are still there as is. There is a danger of failure on parsing though, rather than ignoring.

Because I need this PR this week, I will explore the RSA solution. I just checking in whether Go PIV tools can support RSA and some keys may

znewman01: Note: use `feat!:` for a breaking change
asraa: When I have approval, I'll squash mine and Toby's commits to one and fix the commit to have `feat!`
asraa: Squashed, and commit message fixed, added DCO to fix Toby's old DCO issue

Patch:
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 @@ import (
 
 	"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 @@ form of TUF_{{ROLE}}_PASSPHRASE
 
 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 @@ import (
 	"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 Signature struct {
 }
 
 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 PublicKey struct {
 }
 
 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 @@ require (
 	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 @@ require (
 	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/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/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 @@ import (
 	"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 *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) 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 *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 @@ func (p *p256Verifier) UnmarshalPublicKey(key *data.PublicKey) error {
 		return err
 	}
 
-	curve := elliptic.P256()
+	ecdsaKey, ok := p.PublicKey.PublicKey.(*ecdsa.PublicKey)
+	if !ok {
+		return fmt.Errorf("invalid public key")
+	}
 
-	// Parse as uncompressed marshalled point.
-	x, _ := elliptic.Unmarshal(curve, p.PublicKey)
-	if x == nil {
-		return errors.New("tuf: invalid ecdsa public key point")
+	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/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 @@ import (
 )
 
 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 Ed25519PrivateKeyValue struct {
 
 type ed25519Signer struct {
 	ed25519.PrivateKey
-
-	keyType       string
-	keyScheme     string
-	keyAlgorithms []string
 }
 
 func GenerateEd25519Key() (*ed25519Signer, error) {
@@ -88,19 +84,13 @@ func GenerateEd25519Key() (*ed25519Signer, error) {
 		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 @@ func (e *ed25519Signer) MarshalPrivateKey() (*data.PrivateKey, error) {
 		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 @@ func (e *ed25519Signer) UnmarshalPrivateKey(key *data.PrivateKey) error {
 
 	// 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) UnmarshalPrivateKey(key *data.PrivateKey) error {
 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/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/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 @@ import (
 	"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) MarshalPublicKey() *data.PublicKey {
 }
 
 func (p *rsaVerifier) UnmarshalPublicKey(key *data.PublicKey) error {
-	if err := json.Unmarshal(key.Value, p); err != nil {
-		return err
-	}
-	var err error
-	p.rsaKey, err = parseKey(p.PublicKey)
-	if 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
 	}
-	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")
+	rsaKey, ok := p.PublicKey.PublicKey.(*rsa.PublicKey)
+	if !ok {
+		return fmt.Errorf("invalid public key")
 	}
-	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
+
+	if _, err := x509.MarshalPKIXPublicKey(rsaKey); err != nil {
+		return fmt.Errorf("marshalling to PKIX key: invalid public key")
 	}
-	return nil, errors.New("tuf: error unmarshalling rsa key")
+
+	p.rsaKey = rsaKey
+	p.key = key
+	return nil
 }
 
 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) ContainsID(id string) bool {
 }
 
 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/repo.go b/repo.go
index b4ca62b..603785f 100644
--- a/repo.go
+++ b/repo.go
@@ -309,19 +309,33 @@ func (r *Repo) GenKey(role string) ([]string, error) {
 }
 
 func (r *Repo) GenKeyWithExpires(keyRole string, expires time.Time) (keyids []string, err error) {
-	// Not compatible with delegated targets roles, since delegated targets keys
-	// are associated with a delegation (edge), not a role (node).
+	return r.GenKeyWithSchemeAndExpires(keyRole, expires, data.KeySchemeEd25519)
+}
 
-	signer, err := keys.GenerateEd25519Key()
+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 []string{}, err
+		return nil, err
 	}
 
-	if err = r.AddPrivateKeyWithExpires(keyRole, signer, expires); err != nil {
-		return []string{}, err
+	// Not compatible with delegated targets roles, since delegated targets keys
+	// are associated with a delegation (edge), not a role (node).
+
+	if err = r.AddPrivateKeyWithExpires(role, signer, expires); err != nil {
+		return nil, err
 	}
-	keyids = signer.PublicData().IDs()
-	return
+	return signer.PublicData().IDs(), nil
 }
 
 func (r *Repo) AddPrivateKey(role string, signer keys.Signer) error {
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 NewDBFromDelegations(d *data.Delegations) (*DB, error) {
 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


Test patch:
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 @@ func (s *mockSigner) PublicData() *data.PublicKey {
 	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/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_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 @@ import (
 
 type ECDSASuite struct{}
 
-var _ = Suite(&ECDSASuite{})
+var _ = Suite(ECDSASuite{})
 
-func (ECDSASuite) TestUnmarshalECDSA(c *C) {
-	priv, err := ecdsa.GenerateKey(elliptic.P256(), strings.NewReader("00001-deterministic-buffer-for-key-generation"))
+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")
+}
 
-	// Marshall as non compressed point
-	pub := elliptic.Marshal(elliptic.P256(), priv.X, priv.Y)
+func (ECDSASuite) TestECDSAVerifyMismatchPubKey(c *C) {
+	signer, err := GenerateEcdsaKey()
+	c.Assert(err, IsNil)
+	msg := []byte("foo")
+	sig, err := signer.SignMessage(msg)
+	c.Assert(err, IsNil)
 
-	publicKey, err := json.Marshal(map[string]string{
-		"public": hex.EncodeToString(pub),
-	})
+	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")
+}
 
-	badKey := &data.PublicKey{
+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:      publicKey,
+		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)
+
+	signer := &ecdsaSigner{priv}
+	goodKey := signer.PublicData()
+
 	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/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 @@ package keys
 import (
 	"testing"
 
+	"github.com/theupdateframework/go-tuf/data"
 	. "gopkg.in/check.v1"
 )
 
@@ -32,7 +33,7 @@ func (KeysSuite) TestSignerKeyIDs(c *C) {
 	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_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_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 @@ func (RsaSuite) TestSignVerify(c *C) {
 	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 @@ func (RsaSuite) TestMarshalUnmarshal(c *C) {
 	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_test.go b/repo_test.go
index 87536ee..2f3aebb 100644
--- a/repo_test.go
+++ b/repo_test.go
@@ -3,6 +3,7 @@ package tuf
 import (
 	"bytes"
 	"crypto"
+	"crypto/elliptic"
 	"crypto/rand"
 	"encoding/hex"
 	"encoding/json"
@@ -2705,3 +2706,31 @@ func (rs *RepoSuite) TestTargetMetadataLength(c *C) {
 	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/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 ecdsaSigner struct {
 }
 
 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,