Skip to content
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

Replace RSA related deprecated functions #189

Conversation

kleinmrk
Copy link
Contributor

This MR replaces RSA related functions that are deprecated in OpenSSL 3.0.0 by non-deprecated alternatives.

Heads-up: The changes include adding values to signature_verification_error and signature_generation_error enums. This is a breaking change for code that does not use exceptions but error codes.

@kleinmrk kleinmrk force-pushed the replace-rsa-related-deprecated-functions branch 2 times, most recently from 7864f65 to d7b50f7 Compare November 21, 2021 23:26
Copy link
Collaborator

@prince-chrismc prince-chrismc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor cosmetics

Otherwise LGTM

@@ -265,11 +269,13 @@ namespace jwt {
case signature_generation_error::digestfinal_failed:
return "failed to create signature: DigestFinal failed";
case signature_generation_error::rsa_padding_failed:
return "failed to create signature: RSA_padding_add_PKCS1_PSS failed";
return "failed to create signature: EVP_PKEY_CTX_set_rsa_padding failed";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch

include/jwt-cpp/jwt.h Outdated Show resolved Hide resolved
include/jwt-cpp/jwt.h Outdated Show resolved Hide resolved
include/jwt-cpp/jwt.h Outdated Show resolved Hide resolved
@prince-chrismc prince-chrismc changed the title Replace rsa related deprecated functions Replace RSA related deprecated functions Nov 22, 2021
@kleinmrk kleinmrk force-pushed the replace-rsa-related-deprecated-functions branch 2 times, most recently from 5a72537 to bba33ab Compare November 22, 2021 07:48
@kleinmrk
Copy link
Contributor Author

@prince-chrismc thanks for the prompt review, I already addressed the cosmetic issues 👍

Copy link
Owner

@Thalhammer Thalhammer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

include/jwt-cpp/jwt.h Outdated Show resolved Hide resolved
include/jwt-cpp/jwt.h Outdated Show resolved Hide resolved
@kleinmrk kleinmrk force-pushed the replace-rsa-related-deprecated-functions branch from bba33ab to 8ff6792 Compare November 22, 2021 15:14
@prince-chrismc
Copy link
Collaborator

Please do not force push 🙏 GitHub forces us to restart the review which is not fun!

Awesome, I'll merge this shortly 👍

@kleinmrk
Copy link
Contributor Author

@prince-chrismc sorry, I did not know that it means extra work for you, I just tried to keep the history clean and compact. Lesson learned 👍

@Thalhammer
Copy link
Owner

@prince-chrismc sorry, I did not know that it means extra work for you, I just tried to keep the history clean and compact. Lesson learned +1

Kepping the issue clean is not an issue, since we tend to squash merge anyway (Meaning all commits in the PR get crushed into a single merge commit in master). So they wont show up anywhere afterwards.

@prince-chrismc prince-chrismc merged commit 55aaf31 into Thalhammer:master Nov 23, 2021
@kleinmrk kleinmrk deleted the replace-rsa-related-deprecated-functions branch November 24, 2021 21:52
prince-chrismc added a commit to prince-chrismc/jwt-cpp that referenced this pull request Dec 2, 2021
* 🔧 Replace RSA related deprecated functions (Thalhammer#189)

* RSA_size() --> EVP_PKEY_size()

* getting rid of deprecated functions in rsa-pss signature creation

* getting rid of deprecated functions in rsa-pss signature verification

* generate_hash not used in pss

* Update lint.yml

* Update lint.yml

* Update lint.yml

Co-authored-by: Marek Klein <kleinmrk@gmail.com>
prince-chrismc added a commit that referenced this pull request Jan 17, 2022
* quick poc for #174

* Copy paste

* Rename include/jwt-cpp/traits/traits.h to include/jwt-cpp/traits/danielaparker-jsoncons/traits.h

* Create traits.h

* adding a namespace

* mustache + action to automate defaults generation

since they should always be the same

* fix order

* traits tests should use traits

* playing with header order

to make clang happy

* install all possible traits 

switch badge name to sha

* Update CMakeLists.txt

* Update traits.h

* try commenting corrected code

* rm typescript

* make sure to implace edit

* dont comment when no diff

* apply linting

* fixup boost-json traits

* move comment and render to composite action

* actions/github-script does not leave context in the usual spot

* enabled globstar

* make sure to forward inputs

* try using env normally

* fix typo

* pass in linter name correctly

* debug print

* fix linting issues

* more debug printing

* fix linting issues

* fix typpo + more rebug print

* apply linting

* more debug

* try npm install global to be less intrusive

* wip render traits tests

* Update lint.yml

* global install does not work

* Update action.yml

* Update defaults.h

* remove nodejs action poc

* include jsoncons in main test application (for coverage of new files)

* do not exclude json (since it's used by all the traits)

* exclude bundled json libraries

* apply linting

* add jsoncons for coverage testing

* jsoncons set default to a viable version

* exploring ci failure

* Revert "exploring ci failure" it's overlapping symbols

This reverts commit 3d5b7f9.

* add missing specialization

* be explicit with the tests

* Create CMakeLists.txt

* Create danielaparker-jsoncons.cpp

* Update lint.yml

* Update traits.yml

* lint

* better access too decoded claims

* more tests on verify

* tweak test

trying to find minimum

* lets enable linting tests

* using env impl for github script

* fix typo

* apply linting

* add boost to the tests (very hacky)

* Update lint.yml

* ceate light boost installer

there is no cmake install anymore so just a blanket copy should do

* remove path requirement

* outline for commit and push linting on comment

* try different webhook target

* more event types

* fix if for more event types

* just a test commit

* Update lint.yml

* Update lint.yml

* Update lint.yml

* Apply automatic changes

* add token

* screw up formatting

* Update lint.yml

* Update lint.yml

* Apply automatic changes

* fix merge conflicts

* missing errors

* playing with globbing

* lint issues

* matrix on comment should give many pushes

* Apply automatic changes

* Update lint.yml

* Apply automatic changes

* Apply automatic changes

* fix rebase

* apply render tests

* Apply automatic changes

* Apply automatic changes

* Delete package-lock.json

* fail linting

* Update .gitignore

* Update lint.yml

* Update lint.yml

* Update lint.yml

* Apply automatic changes

* Update lint.yml

* Update CMakeLists.txt

* refactor boost.json inclusion

- find paths
- compile library
- code coverage
- traits example

* Apply automatic changes

* debug log

* Update action.yml

* debug output

* Update action.yml

* Delete private-boost-json.cpp.in

* Update private-find-boost-json.cmake

* Update CMakeLists.txt

* Update CMakeLists.txt

* Create CMakeLists.txt

* Update boost-json.cpp

* Update boost-json.cpp

* Update boost-json.cpp

* Update boost-json.cpp

* remove old boost tests

* examples to include traits

* fix compilation

* test nlohmann json traits

* linting

* refactor traits to make badges

* try with less expression syntax

* trying single quotes

* advertise traits support

* Update lint.yml

* Update lint.yml

* Update lint.yml

* sync (prince-chrismc#16)

* 🔧 Replace RSA related deprecated functions (#189)

* RSA_size() --> EVP_PKEY_size()

* getting rid of deprecated functions in rsa-pss signature creation

* getting rid of deprecated functions in rsa-pss signature verification

* generate_hash not used in pss

* Update lint.yml

* Update lint.yml

* Update lint.yml

Co-authored-by: Marek Klein <kleinmrk@gmail.com>

* Update lint.yml

* make sure to use install `nlohmann-json`

* make action folder match trait names

* add example + badge for kazuho-picojson

* link to new examples

* revert debug code

* dont mix matrix with uses

* fix bad copy paste

* spec interface library so it's no compiled

* fix installer

* fix to interface

* fix example

* attempt 2

* make sure to avoid circular reference

* attempt 3

* white space

* white space

* Update and rename .github/actions/comment-linting/action.yml to .github/actions/process-linting-results/action.yml

* Update lint.yml

* Update action.yml

* Update action.yml

* Update README.md

Co-authored-by: prince-chrismc <prince-chrismc@users.noreply.github.com>
Co-authored-by: Marek Klein <kleinmrk@gmail.com>
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.

3 participants