openssl / openssl

TLS/SSL and crypto library
https://www.openssl.org
Apache License 2.0
25.04k stars 9.97k forks source link

"traditional" PEM output isn't always so "traditional" #12730

Open levitte opened 3 years ago

levitte commented 3 years ago

While making PEM_write_bio_PrivateKey_traditional() more strict (see #12728 and #12729), I discovered that our own (legacy in 3.0) backends do not always support traditional output, but since PEM_write_bio_PrivateKey_traditional() didn't check that too closely, some "traditional" output may be PKCS#8 after all.

DH is one example, I haven't investigated others.

I just tested a little with the openssl installed a part of my system:

$ openssl version
OpenSSL 1.1.1g  21 Apr 2020
$ openssl genpkey -paramfile test/certs/dhp2048.pem > foo.pem
$ cat foo.pem
-----BEGIN PRIVATE KEY-----
MIICJgIBADCCARcGCSqGSIb3DQEDATCCAQgCggEBAKCNFeRygHLBuC5QJ1SYWJ55
dhKznHb8+1jeJ/i6EpGVZHz3DxMQBbl50+r9eq2H8QoGQzwf7UmGkYTsaSYGgBxg
PT638KazOMUJvRyyFrYnciBrZIMp0K67cWgnLpWzzjMXNaHe1dI8PP0o+KXSo4o5
nb2N1bL7kD7ui6O6lfeLt6WDflkj/REj3GNJbyFdLNHmMc1OSzlOxWUb+oKXuufM
sZ6WkH9dzN7BQanEBJ8PryIzcmgQSjZuI1RXigjd2iM/siS/MteBxJ1f7Z4bSK69
9zACb7BtahQWO4hb275cf4Cg1u6spFdP4jdrhJayWSBsQ9rjzGTVzvnXIvhK4jMC
AQIEggEEAoIBAEu33ABijTTUrr6+lmMRlfQdze+dncKSBwyHTHmCgISMdvpXWios
325Lb4LpmXSerfo/xU4Io3NvZ89dXsBLuYPCAgMtM5uQApI0t+IvWfOcVwdef0Ak
QdHZw2SQL6RXC9UACOEZIfjIUOfmW9WLKCtXdx7G4AI8Squu7cwNFbyjB3mrivym
2a6zpYNjML+o73Gs0epixV8eOIrzYl7dF8fB9nIykC5DLTyrAAzyyc1PKz+W/lI2
rb9OiHvQLdulDRXGuzilVAUeQta4CoUFitGBVfg1DFxMhZM68DJHeBPPRuTCzcqv
BWVYwob2cZFkw0YjKgPr8LhzG1gk6ywucvw=
-----END PRIVATE KEY-----
$ openssl pkcs8 -traditional -nocrypt -in foo.pem > bar.pem
$ cat bar.pem 
-----BEGIN DH PRIVATE KEY-----
MIICJgIBADCCARcGCSqGSIb3DQEDATCCAQgCggEBAKCNFeRygHLBuC5QJ1SYWJ55
dhKznHb8+1jeJ/i6EpGVZHz3DxMQBbl50+r9eq2H8QoGQzwf7UmGkYTsaSYGgBxg
PT638KazOMUJvRyyFrYnciBrZIMp0K67cWgnLpWzzjMXNaHe1dI8PP0o+KXSo4o5
nb2N1bL7kD7ui6O6lfeLt6WDflkj/REj3GNJbyFdLNHmMc1OSzlOxWUb+oKXuufM
sZ6WkH9dzN7BQanEBJ8PryIzcmgQSjZuI1RXigjd2iM/siS/MteBxJ1f7Z4bSK69
9zACb7BtahQWO4hb275cf4Cg1u6spFdP4jdrhJayWSBsQ9rjzGTVzvnXIvhK4jMC
AQIEggEEAoIBAEu33ABijTTUrr6+lmMRlfQdze+dncKSBwyHTHmCgISMdvpXWios
325Lb4LpmXSerfo/xU4Io3NvZ89dXsBLuYPCAgMtM5uQApI0t+IvWfOcVwdef0Ak
QdHZw2SQL6RXC9UACOEZIfjIUOfmW9WLKCtXdx7G4AI8Squu7cwNFbyjB3mrivym
2a6zpYNjML+o73Gs0epixV8eOIrzYl7dF8fB9nIykC5DLTyrAAzyyc1PKz+W/lI2
rb9OiHvQLdulDRXGuzilVAUeQta4CoUFitGBVfg1DFxMhZM68DJHeBPPRuTCzcqv
BWVYwob2cZFkw0YjKgPr8LhzG1gk6ywucvw=
-----END DH PRIVATE KEY-----
$ openssl asn1parse -i -in bar.pem
    0:d=0  hl=4 l= 550 cons: SEQUENCE          
    4:d=1  hl=2 l=   1 prim:  INTEGER           :00
    7:d=1  hl=4 l= 279 cons:  SEQUENCE          
   11:d=2  hl=2 l=   9 prim:   OBJECT            :dhKeyAgreement
   22:d=2  hl=4 l= 264 cons:   SEQUENCE          
   26:d=3  hl=4 l= 257 prim:    INTEGER           :A08D15E4728072C1B82E50275498589E797612B39C76FCFB58DE27F8BA129195647CF70F131005B979D3EAFD7AAD87F10A06433C1FED49869184EC692606801C603D3EB7F0A6B338C509BD1CB216B62772206B648329D0AEBB7168272E95B3CE331735A1DED5D23C3CFD28F8A5D2A38A399DBD8DD5B2FB903EEE8BA3BA95F78BB7A5837E5923FD1123DC63496F215D2CD1E631CD4E4B394EC5651BFA8297BAE7CCB19E96907F5DCCDEC141A9C4049F0FAF22337268104A366E2354578A08DDDA233FB224BF32D781C49D5FED9E1B48AEBDF730026FB06D6A14163B885BDBBE5C7F80A0D6EEACA4574FE2376B8496B259206C43DAE3CC64D5CEF9D722F84AE233
  287:d=3  hl=2 l=   1 prim:    INTEGER           :02
  290:d=1  hl=4 l= 260 prim:  OCTET STRING      [HEX DUMP]:028201004BB7DC00628D34D4AEBEBE96631195F41DCDEF9D9DC292070C874C798280848C76FA575A2A2CDF6E4B6F82E999749EADFA3FC54E08A3736F67CF5D5EC04BB983C202032D339B90029234B7E22F59F39C57075E7F402441D1D9C364902FA4570BD50008E11921F8C850E7E65BD58B282B57771EC6E0023C4AABAEEDCC0D15BCA30779AB8AFCA6D9AEB3A5836330BFA8EF71ACD1EA62C55F1E388AF3625EDD17C7C1F67232902E432D3CAB000CF2C9CD4F2B3F96FE5236ADBF4E887BD02DDBA50D15C6BB38A554051E42D6B80A85058AD18155F8350C5C4C85933AF032477813CF46E4C2CDCAAF056558C286F6719164C346232A03EBF0B8731B5824EB2C2E72FC

So basically, you end up getting a PEM file where the headers indicate "traditional", but the contents are PKCS#8 all the same.

Has anyone ever noticed? I suspect that most people have switched to PKCS#8 anyway, so these kinds of bugs go unnoticed for quite a while.

The question is what to do with this. Letting PEM_write_bio_PrivateKey_traditional() output things like the above don't seems right, and squarely contradict the documented intention (from man PEM_write_bio_PrivateKey_traditional):

PEM_write_bio_PrivateKey_traditional() writes out a private key in the "traditional" format with a simple private key marker and should only be used for compatibility with legacy programs.

I can see several possibilities:

  1. Live with it. That's not very appetizing, if not to say plain wrong.
  2. Simply apply #12728 and #12729, and live with the fact that some attempts to output "traditional" PEM will give out errors. In my opinion, this is better than to output PKCS#8 under false pretences. Our tests will have to be adjusted accoordingly
  3. Add missing support in our EVP_PKEY_ASN1_METHOD instances. That would be an amendment to #12728 and #12729.
t8m commented 3 years ago

IMO we should do 2. for now, because 1. is clearly a bug. And if there is a fix available to provide 3. during beta phase I think it should be acceptable as a bug fix.

kroeckx commented 3 years ago

Can we actually read such files that claim to be traditional files but aren't?

levitte commented 3 years ago

@kroeckx, I haven't really tested. Will do

nhorman commented 1 month ago

Marking as inactive, to be closed at the end of 3.4 dev barring further input