x509-create-sign.cpp: Fix build failure with Openssl 4.0 - #794
x509-create-sign.cpp: Fix build failure with Openssl 4.0#794LocutusOfBorg wants to merge 1 commit into
Conversation
|
Thank you for your pull request and welcome to our community! To contribute, please sign the Oracle Contributor Agreement (OCA).
To sign the OCA, please create an Oracle account and sign the OCA in Oracle's Contributor Agreement Application. When signing the OCA, please provide your GitHub username. After signing the OCA and getting an OCA approval from Oracle, this PR will be automatically updated. If you are an Oracle employee, please make sure that you are a member of the main Oracle GitHub organization, and your membership in this organization is public. |
oracle-contributor-agreement
Bot
added
the
OCA Required
|
OCA signed. |
klaus-vb
left a comment
klaus-vb
left a comment
There was a problem hiding this comment.
Much appreciated. Looks good to me, and I'll see that it gets integrated.
The second change made me look at how I can test it easily, and I ended up spotting unfinished code and some pre-existing coding style violations... will take care of that in a follow-up change after this one is integrating (hope the OCA signing is ticked off soon).
|
Hello, for the OCA I don't really know, for the X509_NAME, feel free to avoid the cast, I really don't like it but duplicating the variables just to please gcc was even worse :) |
In future OpenSSL versions |
|
@loqs please have a new look, thanks! |
|
@LocutusOfBorg Thank you. |
…e with OpenSSL 4.0. Inspired by github:gh-794. svn:sync-xref-src-repo-rev: r174697
* r174697: Runtime/common/crypto: Code cleanup regarding const, fix build failure with OpenSSL 4.0. Inspired by github:gh-794. * r174698: Runtime/common/crypto/x509-create-sign.cpp: Cleanup, fix parameter to meet naming conventions. Runtime/tools/RTSignTool.cpp: Fix code for creating self-signed certificate. svn:sync-xref-src-repo-rev: r174699
|
I have applied a close derivative of the initial proposal (due to overload yesterday just visible now), and have thought about the issue @loqs raised, but decided to keep the code as is for now. The reason is the pitfall which @LocutusOfBorg stepped into: the original code is sneakily modifying the (assumed writable) subject name "by reference", and then set the issuer name to the same. In the second change this is now fully broken: it creates an empty name object and sets the subject name to it (by value, meaning it will be empty), then updates the name object to contain the actual subject name and then sets the issuer name to it. Not the same as before, and certainly not the right thing for creating a self-signed cert. No need to propose further fixes, I'll take care of it soon and will point to the adjustment to be reviewed by further people. Before closing this PR. |
…t name. No longer adjust it "by reference" and set the issuer to the same, instead use the clean approach of creating a name object first and setting both subject and issuer name. github:gh-794 svn:sync-xref-src-repo-rev: r174705
…Change handling of subject name. No longer adjust it "by reference" and set the issuer to the same, instead use the clean approach of creating a name object first and setting both subject and issuer name. github:gh-794 svn:sync-xref-src-repo-rev: r174706
|
The brokenness of the second change would've been visible with the now actually working Whatever, see becbd31 for the cleaned up code avoiding to write to the some day maybe actually read-only subject name object inside cert object. Feedback welcome. |
3 participants
No description provided.