[DOC] enhance RDoc for exporting pkeys #645

rhenium · 2023-06-28T03:37:17Z

Describe the behavior of OpenSSL::PKey::{DH,DSA,EC,RSA}#to_pem more clearly. Also, suggest the use of OpenSSL::PKey::PKey#private_to_pem and #public_to_pem when possible.

junaruga · 2023-06-28T15:45:29Z

ext/openssl/ossl_pkey_ec.c

- *     key.export([cipher, pass_phrase]) => String
- *     key.to_pem([cipher, pass_phrase]) => String
+ *     key.export([cipher, password]) => String
+ *     key.to_pem([cipher, password]) => String


If you replace the variable pass_phrase with the password here, do you like to replace the following parts too?

$ grep -r pass_phrase lib/ ext/ ext/openssl/ossl.c: * pass_phrase = 'my secure pass phrase goes here' ext/openssl/ossl.c: * key_secure = key.private_to_pem cipher, pass_phrase ext/openssl/ossl.c: * pass_phrase = 'my secure pass phrase goes here' ext/openssl/ossl.c: * key4 = OpenSSL::PKey.read key4_pem, pass_phrase ext/openssl/ossl.c: * pass_phrase = 'my secure pass phrase goes here' ext/openssl/ossl.c: * encryptor.pkcs5_keyivgen pass_phrase, salt ext/openssl/ossl.c: * decryptor.pkcs5_keyivgen pass_phrase, salt ext/openssl/ossl.c: * pass_phrase = 'my secure pass phrase goes here' ext/openssl/ossl.c: * io.write ca_key.private_to_pem(cipher, pass_phrase)

Good point. To my understanding, the words password and /pass.?phrase/ are completely interchangeable, but they haven't been used consistently.

In fact, RFCs and OpenSSL man pages are inconsistent in word choice.

RFC 8018: PKCS#5 (PBKDF2, OpenSSL::KDF.pbkdf2_hmac) which is used by PKCS#8's encryption uses the word "password".

RFC 7292: PKCS#12 (OpenSSL::PKCS12) also uses "password".

RFC 7914: scrypt (OpenSSL::KDF.scrypt) describes itself as a "password-based key derivation function", while it names the input parameter as the "passphrase".

OpenSSL's man pages appear to prefer "passphrase" in recent years, but OpenSSL has both functions named "password" and "passphrase". Notably, OpenSSL 3.0.0 added two functions named OSSL_ENCODER_CTX_set_pem_password_cb() and OSSL_ENCODER_CTX_set_passphrase_cb() which appear to serve the same purpose (!?).

I will push a new change to replace all /pass.?phrase/ with "password", except scrypt where "passphrase" is more appropriate.

Sure. Thanks for explaining it. OK. I see that you originally named the variables of the functions referring to the words used in the RFC documents.

RFC 8018: PKCS#5 - 2. Notation - "P: password, an octet string"

RFC 7914: scrypt - 6. The scrypt Algorithm - "P: Passphrase, an octet string."

I will push a new change to replace all /pass.?phrase/ with "password", except scrypt where "passphrase" is more appropriate.

It sounds good to me!

I replaced the occurrences of "passphrase" with "password" except those in the OpenSSL::Cipher#pkcs5_keyivgen example, which I started to think having the example itself does harm (#647):

ext/openssl/ossl.c: * encryptor.pkcs5_keyivgen pass_phrase, salt
ext/openssl/ossl.c: * decryptor.pkcs5_keyivgen pass_phrase, salt

junaruga · 2023-06-28T15:49:24Z

Thanks for the PR. I can see that enhancing Rdoc is a way to improve the user experience of exporting pkeys!

junaruga · 2023-06-28T15:54:43Z

ext/openssl/ossl.c

 *   pass_phrase = 'my secure pass phrase goes here'
 *
- *   key_secure = key.export cipher, pass_phrase
+ *   key_secure = key.private_to_pem cipher, pass_phrase


Oh the OpenSSL::PKey::RSA#private_to_pem is a recommended way than the OpenSSL::PKey::RSA#export?

On the following ruby script with debug gem, I see the key.private_to_pem cipher, pass_phrase works while key.export cipher, pass_phrase fails with an error.

$ cat test_debug.rb require 'openssl' require 'debug' key = OpenSSL::PKey::RSA.new 2048 cipher = OpenSSL::Cipher.new 'aes-256-cbc' pass_phrase = 'my secure pass phrase goes here' binding.break key_secure = key.export cipher, pass_phrase

$ OPENSSL_CONF=${HOME}/.local/openssl-3.0.9-fips-debug/ssl/openssl_fips.cnf \ bundle exec ruby -I ./lib test_debug.rb [3, 10] in test_debug.rb 3| 4| key = OpenSSL::PKey::RSA.new 2048 5| cipher = OpenSSL::Cipher.new 'aes-256-cbc' 6| pass_phrase = 'my secure pass phrase goes here' 7| => 8| binding.break 9| 10| key_secure = key.export cipher, pass_phrase =>#0 <main> at test_debug.rb:8 (ruby) key.export cipher, pass_phrase eval error: PEM_write_bio_PrivateKey_traditional: initialization error (rdbg)/test_debug.rb:1:in `export' (rdbg)/test_debug.rb:1:in `<main>' nil (ruby) key.private_to_pem cipher, pass_phrase "-----BEGIN ENCRYPTED PRIVATE KEY-----\nMIIFLTBXBgkqhkiG9w0BBQ0wSjApBgkqhkiG9w0BBQwwHAQI1stA25hQ4rYCAggA\nMAwGCCqGSIb3DQIJBQAwHQYJYIZIAWUDBAEqBBAUpduGlHvQg39SEa67sP8IBIIE\n0JDNQlv17GWzFRJj0iOpjDHEs5bpxRn7TK0m0NG9+/IxrG/3ZAS21GWW7Q+jPNHI\n+fO0ne9kE4/g3ltOJp+JJN5xQ06Uvt7vchXmfa58dMP4aZjIZPVrh+A7RuC4HD9H\nlzGBfWz5YsdsQntjDnnXTj/lZ+Nbclq7nrnh4qJnQsEMAan/+t7QEu4wkIjeCDHG\nT53F/GVHuBcwXpQrV6BLnzqgnhbkFX/RuUfjTXwYJfO8ZL5aWOrochaAaL0bpM9b\nKp9nHQx1fJYzjp8TElljWjkFBrYuq8Fx1v4RgvHNMIFoV5WhLPmosBkyK32VWixN\nCKKiZeCmJcXNLEpf9t0wVfPwxg67Ups76ZAUiJfX/+yx6fPHORn5lBrsZbMVrvqP\n1Zy49YAL14MeWzY9ffYepyOb57n4+19aEEQ8xe+AR+iIzVETl9f2SsGPCuiZARjG\nhpF29TN+5EMDtoNEydFv1F9AF6Ia5T+m7f2zLk5dKv7AFFb2PQs6sH7zOe83+dZT\nU09vzWYfP1w7OaImdiaVpOx8pUecUGcymCfYOOCy9qseUuYb5znu3/pWnYkKyZTU\nKfyRKCTuNYFxtOhqMZtj0zrZxQVlmHvBWBBMzyG7oo/meHe4sJyY8exvJoeqnyd7\nF90B1olBrOTNwN4GYCfSqf3zq0RgTuXPGF0uhubSXLoN99XAd5SXaQPPnJZB+wEp\nXGzJl8KiMChT78bEWdE8hsW583nf2RKAK8Xnl1ie+j4gxcDaLky8JxlZ6UyFiwE8\n+0WRviy5Q7KzHg3xkOYyiLe2zqVY+GQJFYDpSuzfOIusf3kVlcVHeeAk2Swc/8hY\nF17NTC7MZxnUJPX20lS6dUozSw8RbfxQ2AuM2TUkIHG0cpl2rsI6N4ntIkeiaEH7\nLd6EFujmv+qOOARZ/jwsG5PJy4ZrxCSDf93OHAJPReyOr2B5VA1V6Up68WRXkWvk\nDfd36SihorFtJMyTBf7nzsDstPEaQP9l22l9XhaaBxMKs/abx0yg98oM37R8CCin\nNXM/GKABqzaxRUKpLT7NK73QOxo2I7yKQEWAJLToKCHAgrH2QkA+03M2sSD7KcqG\nJymCs0ZZ5ds7xivm/CP5S1vQLq/uGAaJiCknCNMll0r2tA0KuthI9173oTjOdArK\neLiEFKNB/AFZpukSdo8oEKw7wHDAEy0p/OXFQhUGakjDpoaXRjzu2EDy14AE8Cap\nAzQpjBlxs9ngCKZvBLhb2KqYtEIKQHQAWbZAcqu4GxyZaq4FJZRHltacXtFAvxEe\no+NarBAqk84E2UV5LfreGX1m9t1a+5FwUabrWDq+RljPzgCDyd6l83FIdNpOA/V9\na8DFD+j8r/gYNxmhsPZ9WZQkzKvdOujwGDihoPVr5LBf9h/ZNs4J8kfM1wimoBU5\nujL3b59Goyn1VI7M1zdoj7LNpWC6eLqvc0R4dMahqhEtwxl4f1jZFQE+i2fYVV8b\n4zJthvveGgCYW5lEp1RSFwrVXK9FKKnJSTnY+ScC+lz5K+W1HxCFesRgo5RRPtOD\nXzOpo3nBw0Wrln+hicBDmDn4LuLZ5cSwijabdGftMG+XDWaG0lKEfV8j/Xty9pkz\n9YvaKAPAOBwgaOZSfHQEsdVMVYxDkxJqjZQSs9J8+CD2\n-----END ENCRYPTED PRIVATE KEY-----\n"

Oh the OpenSSL::PKey::RSA#private_to_pem is a recommended way than the OpenSSL::PKey::RSA#export?

Ah okay. I found that you explain the context in the commit message.

Yes. OpenSSL::PKey::PKey#private_to_pem uses the standardized PKCS#8 format, optionally encrypted with PBES2 with HMAC SHA-256 (by default; this could be made configurable, but not currently). This is also more widely supported by applications that are not based on OpenSSL.

I see. Thanks for explaining it!

junaruga · 2023-06-28T16:22:07Z

For the 2nd commit, I see you modified the rdoc comments in the ossl_*_export functions too. Perhaps, describing the OpenSSL::PKey::{DH,DSA,EC,RSA}#export methods (right?) are kept for compatibility, but not suggested in the 2nd commit message is helpful.

rhenium · 2023-07-01T12:52:22Z

For the 2nd commit, I see you modified the rdoc comments in the ossl_*_export functions too. Perhaps, describing the OpenSSL::PKey::{DH,DSA,EC,RSA}#export methods (right?) are kept for compatibility, but not suggested in the 2nd commit message is helpful.

Sorry, I'm not sure if I understand this point. I tried to clarify it a bit in the updated commits.

junaruga · 2023-07-01T15:51:02Z

Sorry, I'm not sure if I understand this point. I tried to clarify it a bit in the updated commits.

Thanks for that. My point is that one of the important things in this PR is OpenSSL::PKey::{DH,DSA,EC,RSA}#export is not recommended. The method exists for a backward compatibility. In my understanding, that's why the following changes happened in the 2nd commit. So, I want to see the explanation of why the changes happened.

- *   key_secure = key.export cipher, password
+ *   key_secure = key.private_to_pem cipher, password

  *   open 'ca_key.pem', 'w', 0400 do |io|
- *     io.write ca_key.export(cipher, password)
+ *     io.write ca_key.private_to_pem(cipher, password)
  *   end

OpenSSL::PKey::RSA#export (aliased as #to_s and #to_pem) has quirks.
The output format depends on whether the key is a public key or private
key. Additionally, it uses OpenSSL's own, non-standard format for
encrypting private keys. The man page now says it "should only be used
for compatibility with legacy programs".

It's nice to see the details of the OpenSSL::PKey::RSA#export is written in the commit message. Which man page? The reference (a kind of man N something) is helpful.

Let's consistently use the word "password". Although they are considered synonymous, the mixed usage in the rdoc can cause confusion. OpenSSL::KDF.scrypt is an exception. This is because RFC 7914 refers to the input parameter as "passphrase".

Suggest the use of OpenSSL::PKey::PKey#private_to_pem and #public_to_pem in the top-level documentation. For new programs, these are recommended over OpenSSL::PKey::RSA#export (also aliased as #to_s and #to_pem) unless there is a specific reason to use it, i.e., unless the PKCS#1 output format specifically is required. The output format of OpenSSL::PKey::RSA#export depends on whether the key is a public key or a private key, which is very counter-intuitive. Additionally, when called with arguments to encrypt a private key, as in this example, OpenSSL's own, non-standard format is used. The man page of PEM_write_bio_PrivateKey_traditional(3) in OpenSSL 1.1.1 or later states that it "should only be used for compatibility with legacy programs".

rhenium · 2023-07-01T17:23:56Z

Thanks for the explanation. Good points. I expanded the commit message so it looks like this:

[DOC] prefer PKey#private_to_pem and #public_to_pem in RDoc

Suggest the use of OpenSSL::PKey::PKey#private_to_pem and #public_to_pem in the top-level documentation. For new programs, these are recommended over OpenSSL::PKey::RSA#export (also aliased as #to_s and #to_pem) unless there is a specific reason to use it, i.e., unless the PKCS#1 output format specifically is required.

The output format of OpenSSL::PKey::RSA#export depends on whether the key is a public key or a private key, which is very counter-intuitive.

Additionally, when called with arguments to encrypt a private key, as in this example, OpenSSL's own, non-standard format is used. The man page of PEM_write_bio_PrivateKey_traditional(3) in OpenSSL 1.1.1 or later states that it "should only be used for compatibility with legacy programs".

Describe the behavior of OpenSSL::PKey::{DH,DSA,EC,RSA}#to_pem and #to_der more clearly. They return a different result depending on whether the pkey is a public or private key. This was not documented adequately. Also, suggest the use of OpenSSL::PKey::PKey#private_to_pem and #public_to_pem instead, if possible.

junaruga · 2023-07-14T13:35:16Z

Thank you for updating the document! It looks really nice!

rhenium mentioned this pull request Jun 28, 2023

OpenSSL 3 FIPS mode - creating encrypted RSA key pair fails with PEM_write_bio_PrivateKey_traditional: initialization error (OpenSSL::PKey::PKeyError) #643

Closed

junaruga reviewed Jun 28, 2023

View reviewed changes

rhenium force-pushed the ky/pkey-document-traditional-pem branch from dd13c68 to d959fbd Compare July 1, 2023 12:38

rhenium added 2 commits July 2, 2023 02:02

[DOC] prefer "password" to "passphrase"

06d6764

Let's consistently use the word "password". Although they are considered synonymous, the mixed usage in the rdoc can cause confusion. OpenSSL::KDF.scrypt is an exception. This is because RFC 7914 refers to the input parameter as "passphrase".

rhenium force-pushed the ky/pkey-document-traditional-pem branch from d959fbd to d22769a Compare July 1, 2023 17:24

rhenium merged commit 6588fad into ruby:master Aug 16, 2023
40 checks passed

rhenium mentioned this pull request Aug 31, 2023

test_pkey_ec.rb test failures in OpenSSL FIPS #671

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DOC] enhance RDoc for exporting pkeys #645

[DOC] enhance RDoc for exporting pkeys #645

rhenium commented Jun 28, 2023

junaruga Jun 28, 2023

rhenium Jun 29, 2023

junaruga Jun 29, 2023

rhenium Jul 1, 2023

junaruga commented Jun 28, 2023

junaruga Jun 28, 2023

junaruga Jun 28, 2023

rhenium Jun 29, 2023

junaruga Jun 29, 2023

junaruga commented Jun 28, 2023

rhenium commented Jul 1, 2023

junaruga commented Jul 1, 2023 •

edited

rhenium commented Jul 1, 2023

junaruga commented Jul 14, 2023

[DOC] enhance RDoc for exporting pkeys #645

[DOC] enhance RDoc for exporting pkeys #645

Conversation

rhenium commented Jun 28, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

junaruga commented Jun 28, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

junaruga commented Jun 28, 2023

rhenium commented Jul 1, 2023

junaruga commented Jul 1, 2023 • edited

rhenium commented Jul 1, 2023

junaruga commented Jul 14, 2023

junaruga commented Jul 1, 2023 •

edited