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

Closed
paulmey opened this issue Oct 4, 2018 · 10 comments
Closed

x/crypto/pkcs12: ToPEM should encode private keys as PKCS8 #28018

paulmey opened this issue Oct 4, 2018 · 10 comments
Labels
Documentation FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Unfortunate
Milestone
Unreleased

Comments

@paulmey
Copy link

paulmey 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
Copy link
Contributor

agnivade commented Oct 5, 2018

@FiloSottile

@agnivade agnivade added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Oct 5, 2018
@bhavanasrini
Copy link

Is this resolved ???

@FiloSottile
Copy link
Contributor

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.

@FiloSottile FiloSottile added Unfortunate NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Mar 6, 2019
@bhavanasrini
Copy link

@FiloSottile @paulmey Is there any alternative for this?

@FiloSottile
Copy link
Contributor

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

@bhavanasrini
Copy link

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

@FiloSottile
Copy link
Contributor

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

@bhavanasrini
Copy link

bhavanasrini commented Mar 7, 2019 •
edited

@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
Copy link
Contributor

@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.

@AGWA AGWA mentioned this issue Mar 21, 2019
Closed
@gopherbot
Copy link

Change https://golang.org/cl/241337 mentions this issue: pkcs12: document that we use the wrong PEM type

@golang golang locked and limited conversation to collaborators Jul 7, 2021
c-expert-zigbee pushed a commit to c-expert-zigbee/crypto_go that referenced this issue Mar 28, 2022
@FiloSottile
pkcs12: document that we use the wrong PEM type
350b018 
Fixes golang/go#28018

Change-Id: I2daf99789328ef476de834c3cc703e01b468b3ee
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/241337
Reviewed-by: Katie Hockman <katie@golang.org>
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
c-expert-zigbee added a commit to c-expert-zigbee/crypto_go that referenced this issue Mar 29, 2022
@c-expert-zigbee
pkcs12: document that we use the wrong PEM type
889e997 
Fixes golang/go#28018

Change-Id: I2daf99789328ef476de834c3cc703e01b468b3ee
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/241337
Reviewed-by: Katie Hockman <katie@golang.org>
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
c-expert-zigbee added a commit to c-expert-zigbee/crypto_go that referenced this issue Mar 29, 2022
@c-expert-zigbee
pkcs12: document that we use the wrong PEM type
79769ec 
Fixes golang/go#28018

Change-Id: I2daf99789328ef476de834c3cc703e01b468b3ee
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/241337
Reviewed-by: Katie Hockman <katie@golang.org>
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
LewiGoddard pushed a commit to LewiGoddard/crypto that referenced this issue Feb 16, 2023
@FiloSottile
pkcs12: document that we use the wrong PEM type
fa7f2ee 
Fixes golang/go#28018

Change-Id: I2daf99789328ef476de834c3cc703e01b468b3ee
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/241337
Reviewed-by: Katie Hockman <katie@golang.org>
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
BiiChris pushed a commit to BiiChris/crypto that referenced this issue Sep 15, 2023
@FiloSottile
pkcs12: document that we use the wrong PEM type
76a08c0 
Fixes golang/go#28018

Change-Id: I2daf99789328ef476de834c3cc703e01b468b3ee
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/241337
Reviewed-by: Katie Hockman <katie@golang.org>
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
Documentation FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Unfortunate
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
5 participants
@FiloSottile @paulmey @agnivade @gopherbot @bhavanasrini