-
Notifications
You must be signed in to change notification settings - Fork 177
This adds CMS support to the Ruby layer. #917
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
base: master
Are you sure you want to change the base?
Conversation
718a84a
to
a3012ee
Compare
The CMS_ContentInfo object can be used. Access to the CMS_SignedInfo structure exists, but it can not be created or freed, as it is just a pointer into CMS_ContentInfo stack. feat: added NOINTERN and NO_SIGNER_CERT_VERIFY flags feat: look for CMS_sign, and set HAVE_CMS_SIGN if present, enabling CMS support
a3012ee
to
51b28bc
Compare
Not sure how to get the aws-lc-latest test case to run. Does it even work? |
@@ -143,6 +143,9 @@ def find_openssl_library | |||
# added in 1.1.0, currently not in LibreSSL | |||
have_func("EVP_PBE_scrypt(\"\", 0, (unsigned char *)\"\", 0, 0, 0, 0, 0, NULL, 0)", evp_h) | |||
|
|||
# look for CMS code, normally included, but some variations compile it out | |||
have_func("CMS_sign", ssl_h) |
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.
ruby/openssl currently aims to support OpenSSL 1.1.1 or later, LibreSSL versions that are supported by upstream, and the latest version of AWS-LC.
I think we can use !defined(OPENSSL_NO_CMS)
without this mkmf check.
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.
Okay, I put this in during 1.1.1, because I found that some targets (maybe it was openwrt) did not include CMS for space reasons.
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.
AFAIK OPENSSL_NO_XXX
macros are provided for this purpose. Their headers should have the macro if it is compiled with ./Configure no-cms
.
ext/openssl/ossl_cms.c
Outdated
static VALUE | ||
ossl_cmssi_new(CMS_SignerInfo *cmssi) | ||
{ | ||
VALUE obj; | ||
|
||
obj = NewCMSsi(cCMSSignerInfo); | ||
SetCMSsi(obj, cmssi); | ||
|
||
return obj; | ||
} |
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.
The CMS_SignerInfo
is only valid as long as the CMS_ContentInfo
is alive, so we must prevent the CMS_ContentInfo
from being free'd earlier than it.
I think you can add a reference to it in a hidden instance variable, rb_ivar_set(obj, rb_intern("cms"), cms_content_info_wrapper)
.
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.
so, I think it should be:
rb_ivar_set(obj, rb_intern("cms"), cmssi);
I don't undnerstrand the cms_content_info_wrapper here.
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.
I meant the Ruby object wrapping the CMS_ContentInfo
.
* | ||
*/ | ||
static VALUE | ||
ossl_cmsci_initialize(int argc, VALUE *argv, VALUE self) |
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.
As I commented on ossl_cmssi_new()
, we must avoid freeing CMS_ContentInfo
or destructively changing it as long as an object wrapping CMS_SignerInfo
borrowed from it exists. We'd need to prevent #initialize
from being called twice.
From a quick look, the current implementation of My understanding is that CMS is intended to be the successor of PKCS#7 and backwards-compatible with it. I wonder if we could avoid maintaining two separate modules for what is basically the same structure. I haven't looked into the OpenSSL API and I'm not sure if it's possible to do fully copy the existing behaviors, but would it be feasible to update I'm also not sure why OpenSSL decided to use different structs for PKCS#7 and CMS in the first place. |
AWS-LC doesn't appear to have openssl/cms.h. AWS-LC defines |
CMS > PKCS7, but PKCS7 things can not in general parse CMS. If we were to do anything, it would be to rip PKCS7 code out. |
…t it will not get freed before thing contained in
…CMS_get0_SignerInfos function was successful
I ran the latest master branch of the ruby/openssl in my forked repository, and the awc-ls-latest case passed. So, this PR's change might break the awc-ls-latest case. https://github.com/junaruga/ruby-openssl/actions/runs/16567236800 |
It seems that aws-lc does not include CMS support.
The error that I saw, however, was about impossible version numbers, which
I couldn't understand at all. With OPENSSL_NO_CMS as the key to compiling
the CMS code, then aws-lc-latest "works", because it doesn't test anything.
|
Yes, AWS-LC doesn't have openssl/cms.h either. openssl/cms.h from Ubuntu's base system appears to be included incorrectly as a result. |
This updates the patch to support openssl 3.x.
I have maintained a patch for some years for openssl1.1.1 that adds CMS support.
(This is needed for RFC8995 code)