golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
121.58k stars 17.41k forks source link

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

Closed paulmey closed 4 years ago

paulmey commented 5 years ago

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?

agnivade commented 5 years ago

@FiloSottile

bhavanasrini commented 5 years ago

Is this resolved ???

FiloSottile commented 5 years ago

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 commented 5 years ago

@FiloSottile @paulmey Is there any alternative for this?

FiloSottile commented 5 years ago

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

bhavanasrini commented 5 years ago

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

FiloSottile commented 5 years ago

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

bhavanasrini commented 5 years ago

@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 commented 5 years ago

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

gopherbot commented 4 years ago

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