-
Notifications
You must be signed in to change notification settings - Fork 167
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
Add OpenSSL::Digest.digests to get a list of available digests #726
Conversation
Test failure looks unrelated. |
Yes. #728 should fix it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this! This is a good addition.
Changes look good to me, with one style nit:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your work!
In my understanding the test still works with my proposed changes,right? If it still works, I hope you would accept my proposal. My intention is to change only essential parts, and align with the existing encryption name style with upper case in this testing file.
No, not without additional changes. As mentioned earlier
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your explanation. Sorry, I misunderstand the implementation in this PR. OK I checked the PR on my local, and confirmed the behavior was similar between OpenSSL::Digest
and OpenSSL::Cipher
.
OpenSSL::Digest
$ ruby -I lib -r openssl -e 'p OpenSSL::Digest.new("SHA512-224")'
#<OpenSSL::Digest: 6ed0dd02806fa89e25de060c19d3ac86cabb87d6a0ddd05c333b84f4>
$ ruby -I lib -r openssl -e 'p OpenSSL::Digest.new("sha512-224")'
#<OpenSSL::Digest: 6ed0dd02806fa89e25de060c19d3ac86cabb87d6a0ddd05c333b84f4>
$ ruby -I lib -r openssl -e 'p OpenSSL::Digest.digests.include?("SHA512-224")'
false
$ ruby -I lib -r openssl -e 'p OpenSSL::Digest.digests.include?("sha512-224")'
true
OpenSSL::Cipher
$ ruby -I lib -r openssl -e 'p OpenSSL::Cipher.new("AES-256-CBC-HMAC-SHA256")'
#<OpenSSL::Cipher:0x00007f883e05ce68>
$ ruby -I lib -r openssl -e 'p OpenSSL::Cipher.new("aes-256-cbc-hmac-sha256")'
#<OpenSSL::Cipher:0x00007f9d589fce58>
$ ruby -I lib -r openssl -e 'p OpenSSL::Cipher.ciphers.include?("AES-256-CBC-HMAC-SHA256")'
false
$ ruby -I lib -r openssl -e 'p OpenSSL::Cipher.ciphers.include?("aes-256-cbc-hmac-sha256")'
true
Please check my new comment for the digest_available?
. Thank you.
Is there an official document about the "shot name" and "long name" as a return value in OpenSSL project? I am confused because for example, both "SHA512-224" and "sha512-224" is the same string length. It's neither short nor long. |
By the way, I am not the main maintainer of this repository. I just commented. |
OBJ_nid2obj(3) explains it a bit, but not the reason why each object has two names in the first place. I don't really understand it, either. The list obtained from OBJ_NAME_do_all_sorted() appears to be excluding case-insensitive duplicates and looks weird (which is not necessarily wrong because names are case-insensitive, but still feels strange):
(All of the above are alias of SHA-1) Would it make sense to make OpenSSL::Digest.digests and OpenSSL::Cipher.ciphers include all possible names? It would be out of scope of this PR, though. |
This returns the long names of digests. Similar to
OpenSSL::Cipher.ciphers
(I took most of the implementation from it)Maybe the only confusing thing is that
Digest#name
returns the short name,Cipher#name
does have a little disclaimer in the docs it might not be the same as given to the constructor.