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

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

Open
levitte opened this issue Aug 27, 2020 · 3 comments
Open

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

levitte opened this issue Aug 27, 2020 · 3 comments
Labels
triaged: bug The issue/pr is/fixes a bug

Comments

@levitte
Copy link
Member

levitte commented Aug 27, 2020

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 Fix PEM_write_bio_PrivateKey_traditional() to not output PKCS#8 #12728 and [1.1.1] Fix PEM_write_bio_PrivateKey_traditional() to not output PKCS#8 #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 Fix PEM_write_bio_PrivateKey_traditional() to not output PKCS#8 #12728 and [1.1.1] Fix PEM_write_bio_PrivateKey_traditional() to not output PKCS#8 #12729.
@levitte levitte added the triaged: bug The issue/pr is/fixes a bug label Aug 27, 2020
@t8m
Copy link
Member

t8m commented Aug 27, 2020

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
Copy link
Member

kroeckx commented Aug 27, 2020 via email

@levitte
Copy link
Member Author

levitte commented Aug 27, 2020

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triaged: bug The issue/pr is/fixes a bug
Projects
None yet
Development

No branches or pull requests

3 participants