Skip to content

Conversation

@rhenium
Copy link
Member

@rhenium rhenium commented Jul 3, 2024

The returned Hash from these methods contain 0 in place of a missing parameter in the key, for example:

pkey = OpenSSL::PKey.read(OpenSSL::PKey::RSA.new(2048).public_to_pem)
pp pkey.params
#=>
# {"n"=>#<OpenSSL::BN 286934673421[...snip]>,
#  "e"=>#<OpenSSL::BN 65537>,
#  "d"=>#<OpenSSL::BN 0>,
#  "p"=>#<OpenSSL::BN 0>,
#  "q"=>#<OpenSSL::BN 0>,
#  "dmp1"=>#<OpenSSL::BN 0>,
#  "dmq1"=>#<OpenSSL::BN 0>,
#  "iqmp"=>#<OpenSSL::BN 0>}

Let's use nil instead, which is more appropriate for indicating a missing value.


Also:

pkey: implement PKey::{RSA,DSA,DH}#params in Ruby

Move the definitions to lib/openssl/pkey.rb.

They don't have to be implemented in the native extension and can be simplified by using existing methods.

Add missing test cases to verify the current behavior. The next patch
will rewrite those methods.
Move the definitions to lib/openssl/pkey.rb. They need not to be in the
extension and can be implemented using existing methods.

This reduces direct usage of the now-deprecated OpenSSL APIs around the
low-level structs such as DH, DSA, or RSA.
The returned Hash from these methods contain 0 in place of a missing
parameter in the key, for example:

	pkey = OpenSSL::PKey.read(OpenSSL::PKey::RSA.new(2048).public_to_pem)
	pp pkey.params
	#=>
	# {"n"=>#<OpenSSL::BN 286934673421[...snip]>,
	#  "e"=>#<OpenSSL::BN 65537>,
	#  "d"=>#<OpenSSL::BN 0>,
	#  "p"=>#<OpenSSL::BN 0>,
	#  "q"=>#<OpenSSL::BN 0>,
	#  "dmp1"=>#<OpenSSL::BN 0>,
	#  "dmq1"=>#<OpenSSL::BN 0>,
	#  "iqmp"=>#<OpenSSL::BN 0>}

Let's use nil instead, which is more appropriate for indicating a
missing value.
@rhenium rhenium force-pushed the ky/pkey-get-params-nil branch from 1f5bdaf to f247ec3 Compare January 22, 2025 16:21
@rhenium rhenium merged commit 6a48f7c into ruby:master Jan 22, 2025
53 checks passed
ksss added a commit to ksss/rbs that referenced this pull request Jan 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant