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/pkcs12: ToPEM should encode private keys as PKCS8 #28018

Open
paulmey opened this issue Oct 4, 2018 · 9 comments

Comments

@paulmey
Copy link

commented Oct 4, 2018

According to the table in Section 4 of RFC 7468, PEM blocks labeled PRIVATE KEY should be PKCS8:

4.  Guide

   For convenience, these figures summarize the structures, encodings,
   and references in the following sections:

Sec. Label                  ASN.1 Type              Reference Module
----+----------------------+-----------------------+---------+----------
  5  CERTIFICATE            Certificate             [RFC5280] id-pkix1-e
  6  X509 CRL               CertificateList         [RFC5280] id-pkix1-e
  7  CERTIFICATE REQUEST    CertificationRequest    [RFC2986] id-pkcs10
  8  PKCS7                  ContentInfo             [RFC2315] id-pkcs7*
  9  CMS                    ContentInfo             [RFC5652] id-cms2004
 10  PRIVATE KEY            PrivateKeyInfo ::=      [RFC5208] id-pkcs8
                            OneAsymmetricKey        [RFC5958] id-aKPV1
 11  ENCRYPTED PRIVATE KEY  EncryptedPrivateKeyInfo [RFC5958] id-aKPV1
 12  ATTRIBUTE CERTIFICATE  AttributeCertificate    [RFC5755] id-acv2
 13  PUBLIC KEY             SubjectPublicKeyInfo    [RFC5280] id-pkix1-e

                        Figure 4: Convenience Guide

However, pkcs12.ToPEM encodes the private key to a type-specific format.

		switch key := key.(type) {
		case *rsa.PrivateKey:
			block.Bytes = x509.MarshalPKCS1PrivateKey(key)
		case *ecdsa.PrivateKey:
			block.Bytes, err = x509.MarshalECPrivateKey(key)			
		...
		}

This code has been out for 3 years or so and I'm sure that everyone who uses it has compensated for this bug, so I'm not sure that we want to fix it?

@gopherbot gopherbot added this to the Unreleased milestone Oct 4, 2018

@agnivade

This comment has been minimized.

Copy link
Member

commented Oct 5, 2018

@bhavanasrini

This comment has been minimized.

Copy link

commented Mar 6, 2019

Is this resolved ???

@FiloSottile

This comment has been minimized.

Copy link
Member

commented Mar 6, 2019

Unfortunately, it's too late to fix this. It would almost certainly break all current users of the function.

We should visibly document the issue, though.

@bhavanasrini

This comment has been minimized.

Copy link

commented Mar 7, 2019

@FiloSottile @paulmey Is there any alternative for this?

@FiloSottile

This comment has been minimized.

Copy link
Member

commented Mar 7, 2019

@bhavanasrini If you truly need PKCS#8, you can decode and then re-encode the public key objects in your application.

@bhavanasrini

This comment has been minimized.

Copy link

commented Mar 7, 2019

You mean after applying ToPEM to pkcs ? or instead of using ToPEM I can use decode and re-encode ?

@FiloSottile

This comment has been minimized.

Copy link
Member

commented Mar 7, 2019

After using ToPEM, yeah. All Marshal functions in crypto/x509 have a Parse counterpart.

@bhavanasrini

This comment has been minimized.

Copy link

commented Mar 7, 2019

@FiloSottile Actually after converting to PEM I need my output in the form of bytes ... So I tried applying ParsePKCS1PrivateKey followed by MarshalPKCS1PrivateKey function. Output didn't change. I was thinking it is going to change my result.

@FiloSottile

This comment has been minimized.

Copy link
Member

commented Mar 7, 2019

@bhavanasrini If you use the pair of corresponding Parse and Marshal functions, the output is not supposed to change. Anyway, this issue is now about documenting the output of this function, for questions on how to use it see https://golang.org/wiki/Questions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.