Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/crypto/ssh: please provide unified parameter types for ed25519.PrivateKey #51974

Open
dacapoday opened this issue Mar 27, 2022 · 5 comments
Open
Labels
NeedsInvestigation Unfortunate
Milestone

Comments

@dacapoday
Copy link

@dacapoday dacapoday commented Mar 27, 2022

Hi,
When I want to use go to do some format cleaning for PEM file found that ssh.ParseRawPrivateKey and x509.MarshalPKCS8PrivateKey have different data type for ed25519.PrivateKey.
ssh.ParseRawPrivateKey actually output a Pointer, x509.MarshalPKCS8PrivateKey only receiver Value.
But other algorithms are both pointers. So I had to do type assertion for ed25519.PrivateKey.

example code:

package main

import (
	"crypto/ed25519"
	"crypto/x509"
	"encoding/base64"
	"encoding/pem"
	"fmt"

	"golang.org/x/crypto/ssh"
)

var pemfile []byte

func main() {
	prikey, err := ssh.ParseRawPrivateKey(pemfile)
	if err != nil {
		panic(err)
	}

	// workaround for golang.org/x/crypto/ssh/ed25519
	ed25519key, ok := prikey.(*ed25519.PrivateKey)
	if ok {
		prikey = *ed25519key
	}

	prikeycontent, err := x509.MarshalPKCS8PrivateKey(prikey)
	if err != nil {
		panic(err)
	}
	block := &pem.Block{
		Type:  "PRIVATE KEY",
		Bytes: prikeycontent,
	}
	prikeypem := pem.EncodeToMemory(block)
	fmt.Printf("prikey: \n%s\n", prikeypem)
}

func init() {
	var err error
	pemfile, err = base64.StdEncoding.DecodeString(samplePem)
	if err != nil {
		panic(err)
	}
}

var samplePem = `LS0tLS1CRUdJTiBPUEVOU1NIIFBSSVZBVEUgS0VZLS0tLS0KYjNCbGJuTnphQzFyWlhrdGRqRUFBQUFBQkc1dmJtVUFBQUFFYm05dVpRQUFBQUFBQUFBQkFBQUFNd0FBQUF0emMyZ3RaVwpReU5UVXhPUUFBQUNDc1pUcEpXZ0ljajFqcFR0OVJHQ2ovUktCTlJZSTFiWmZDZFJXNCtmNmJ2UUFBQUlqWExVaTYxeTFJCnVnQUFBQXR6YzJndFpXUXlOVFV4T1FBQUFDQ3NaVHBKV2dJY2oxanBUdDlSR0NqL1JLQk5SWUkxYlpmQ2RSVzQrZjZidlEKQUFBRUNPUm12SUdjaDVWMzRqSUpWSGFiU3FQbGl3RGJsL3diamNtNVpmeWZhRjdxeGxPa2xhQWh5UFdPbE8zMUVZS1A5RQpvRTFGZ2pWdGw4SjFGYmo1L3B1OUFBQUFBbmg0QVFJRAotLS0tLUVORCBPUEVOU1NIIFBSSVZBVEUgS0VZLS0tLS0K`

I think it's not necessary. Perhaps we can make x509.MarshalPKCS8PrivateKey receiver ed25519.PrivateKey Pointer and Value at same time or make ssh.ParseRawPrivateKey output Value because ed25519.PrivateKey underlying type is []byte.

@gopherbot gopherbot added this to the Unreleased milestone Mar 27, 2022
@crazysouthern

This comment was marked as off-topic.

@crazysouthern

This comment was marked as off-topic.

@seankhliao
Copy link
Member

@seankhliao seankhliao commented Mar 28, 2022

cc @golang/security

@seankhliao seankhliao added the NeedsInvestigation label Mar 28, 2022
@FiloSottile
Copy link
Contributor

@FiloSottile FiloSottile commented Jun 29, 2022

It's even a little worse than this: ParseRawPrivateKey returns a value for PKCS8 encodings (because it calls x509.ParsePKCS8PrivateKey) and a pointer for the OpenSSH encoding.

The correct type is definitely the value type, which is what everything else expects, but it might be too late to change ParseRawPrivateKey, since most if not all Ed25519 keys are encoded in the OpenSSH encoding, so applications are probably expecting pointers there.

@FiloSottile
Copy link
Contributor

@FiloSottile FiloSottile commented Jun 29, 2022

It goes all the way back to 2016, so applications have surely come to rely on it: https://go-review.googlesource.com/22512

See also golang/crypto#119.

FiloSottile added a commit to FiloSottile/age that referenced this issue Jul 3, 2022
OpenSSH never generated them (unencrypted, and golang.org/x/crypto/ssh
doesn't support encrypted PKCS#8 for now, so the encrypted_keys.go
change is technically superfluous) but there are other systems that
produce them (for example, 1Password). Unfortunately, ParseRawPrivateKey
returns a value type for PKCS#8 and a pointer type for the OpenSSH
format (golang/go#51974), so we need to handle both.

Fixes #429
FiloSottile added a commit to FiloSottile/age that referenced this issue Jul 3, 2022
OpenSSH never generated them (unencrypted, and golang.org/x/crypto/ssh
doesn't support encrypted PKCS#8 for now, so the encrypted_keys.go
change is technically superfluous) but there are other systems that
produce them (for example, 1Password). Unfortunately, ParseRawPrivateKey
returns a value type for PKCS#8 and a pointer type for the OpenSSH
format (golang/go#51974), so we need to handle both.

Fixes #429
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Unfortunate
Projects
None yet
Development

No branches or pull requests

5 participants