crypto/ecdsa: add methods to convert keys to crypto/ecdh format

[pedroalbanese]

Update, Nov 9 2022: Proposal is #56088 (comment)

---

Greetings!

I need Privacy-Enhanced Mail (PEM) support for Curve25519 private keys, more precisely X25519, like OpenSSL. Currently Go's standard libraries only support Ed25519 private keys, not Curve25519 for this task.

Thanks in Advance!

[ianlancetaylor]

CC @golang/security

[seankhliao]

doesn't this already exist https://pkg.go.dev/golang.org/x/crypto/curve25519#X25519

[pedroalbanese]

Hi,

This already exists, but x509 library doesn't support it, so it's not possible to convert private keys to PEM format, more precisely the MarshalPKCS8PrivateKey and MarshalECPrivateKey functions (I don't know which one will be used in this case) doesn't recognize the OID of Curve25519 private keys, I remember being able to convert the public key, but private key is not supported.

package main

import (
	"crypto/x509"
	"encoding/pem"
	"log"
)

func main() {

	pKey := `-----BEGIN PRIVATE KEY-----
MCowBQYDK2VuAyEAfLLsWKkI/7EmTOkSf4fyHuRHDnKk6qNncWDzV8jlIUU=
-----END PRIVATE KEY-----`

	block, _ := pem.Decode([]byte(pKey))

	if block == nil || block.Type != "PRIVATE KEY" {
		log.Fatal("failed to decode PEM block containing private key")

	}

	key, err := x509.ParsePKCS8PrivateKey(block.Bytes)
	if err != nil {
		log.Println("Parse PKI Error:", err)
		return
	}

	log.Println(key)
}

2009/11/10 23:00:00 Parse PKI Error: asn1: structure error: tags don't match (2 vs {class:0 tag:16 length:5 isCompound:true}) {optional:false explicit:false application:false private:false defaultValue:<nil> tag:<nil> stringType:0 timeType:0 set:false omitEmpty:false} int @2

[rsc]

What are the API changes or implementation changes you are proposing in the x509 package?

/cc @golang/security

[rsc]

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

[pedroalbanese]

API changes or implementation changes I'm proposing in the x509 package:

(I don't understand if this question is for me)

x509.MarshalPKCS8PrivateKey() and x509.ParsePKCS8PrivateKey().

I'm afraid the OID of algorithm need to be included.

[FiloSottile]

MarshalECPrivateKey is hardcoded to deal with crypto/ecdsa keys, and Curve25519 is not compatible with that package, or ever used with ECDSA.

MarshalPKCS8PrivateKey and maybe MarshalPKIXPublicKey are more flexible. What we need to support a new key type is a discrete OID to tell the key apart, and a Go type to marshal/parse from/into. RFC 8410, Section 3 gives us dedicated OIDs for Curve25519 keys, and we now have crypto/ecdh.PrivateKey and PublicKey for X25519 keys, so this should be easily doable.

Unfortunately, we can't do the same for crypto/ecdh keys based on NIST curves, because those use the same OIDs as the ECDSA keys on the same curves, regrettably. We could make MarshalPKCS8PrivateKey and MarshalPKIXPublicKey support crypto/ecdh keys, but then the Parse counterparts would produce crypto/ecdsa keys, instead of round-tripping cleanly. This was discussed in #52221 (comment). Maybe that's acceptable if crypto/ecdsa keys have methods to produce crypto/ecdh keys. Opinions?

[rsc]

Maybe that's acceptable if crypto/ecdsa keys have methods to produce crypto/ecdh keys.

That seems like a reasonable path forward. Any objections to doing that?

[rsc]

OK, it sounds like maybe there are no objections. ecdh does not import ecdsa, so we should be able to add methods in ecdsa. I guess it would be

package ecdsa

func (k *PrivateKey) ECDH() *ecdh.PrivateKey
func (k *PublicKey) ECDH() *ecdh.PublicKey

Do I have that right?

[rsc]

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

[gopherbot]

Change https://go.dev/cl/450815 mentions this issue: crypto/x509: add support for PKCS8/PKIX X25519 key encodings

[FiloSottile]

I ended up adding an error return, since crypto/ecdsa keys can be invalid and crypto/ecdh can't.

package ecdsa

func (k *PrivateKey) ECDH() (*ecdh.PrivateKey, error)
func (k *PublicKey) ECDH() (*ecdh.PublicKey, error)

[rsc]

Error returns makes sense to me.

[gopherbot]

Change https://go.dev/cl/450816 mentions this issue: crypto/ecdsa,crypto/x509: add encoding paths for NIST crypto/ecdh keys

[rsc]

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— rsc for the proposal review group

[pedroalbanese]

Thank You very much!