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

Jump to bottom

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

Open
dacapoday opened this issue Mar 27, 2022 · 5 comments
Open

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

dacapoday opened this issue Mar 27, 2022 · 5 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Unfortunate
Milestone
Unreleased

Comments

@dacapoday
Copy link

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
@1999chato

This comment was marked as off-topic.

Sign in to view
@1999chato

This comment was marked as off-topic.

Sign in to view
@seankhliao
Copy link
Member

cc @golang/security

@seankhliao seankhliao added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 28, 2022
@FiloSottile
Copy link
Contributor

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

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 FiloSottile mentioned this issue Jun 29, 2022
Closed
FiloSottile added a commit to FiloSottile/age that referenced this issue Jul 3, 2022
@FiloSottile
agessh: support PKCS#8-encoded Ed25519 private keys
f31a8ad 
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
@FiloSottile
agessh: support PKCS#8-encoded Ed25519 private keys
de7c1fb 
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
Assignees
No one assigned
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Unfortunate
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
5 participants
@FiloSottile @gopherbot @dacapoday @seankhliao @1999chato