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

Improvements to key loading helpers #186

Closed
prince-chrismc opened this issue Nov 19, 2021 · 4 comments · Fixed by #318
Closed

Improvements to key loading helpers #186

prince-chrismc opened this issue Nov 19, 2021 · 4 comments · Fixed by #318
Milestone

Comments

@prince-chrismc
Copy link
Collaborator

We should deprecate/remove the older helpers and rename them to have RSA in the name.

Originally posted by @prince-chrismc in #185 (comment)

@prince-chrismc
Copy link
Collaborator Author

I also switched from [pem_bio_read]. This change made it obvious that one could use the same code for reading RSA and EC keys. The functions only differ in type of errors they report, e.g., error::ecdsa_error::create_mem_bio_failed vs. error::rsa_error::create_mem_bio_failed.

Orignally posted in #185 (comment)

@prince-chrismc prince-chrismc added this to the 0.7.0 milestone Nov 19, 2021
@prince-chrismc prince-chrismc changed the title Improvement to key loading helpers Improvements to key loading helpers Nov 19, 2021
@prince-chrismc
Copy link
Collaborator Author

The newer EC family of helpers are also missing test code which was present with the old ones load_public_ec_key_from_string for example

@prince-chrismc
Copy link
Collaborator Author

prince-chrismc commented Dec 10, 2023

The implementations are nearly the same they just need different error codes but otherwise are interchangable

diff --git a/include/jwt-cpp/jwt.h b/include/jwt-cpp/jwt.h
index 000fb0b..1235585 100644
--- a/include/jwt-cpp/jwt.h
+++ b/include/jwt-cpp/jwt.h
@@ -778,7 +778,7 @@ namespace jwt {
                        ec.clear();
                        auto pubkey_bio = make_mem_buf_bio();
                        if (!pubkey_bio) {
-                               ec = error::ecdsa_error::create_mem_bio_failed;
+                               ec = error::rsa_error::create_mem_bio_failed;
                                return {};
                        }
                        if (key.substr(0, 27) == "-----BEGIN CERTIFICATE-----") {
@@ -786,13 +786,13 @@ namespace jwt {
                                if (ec) return {};
                                const int len = static_cast<int>(epkey.size());
                                if (BIO_write(pubkey_bio.get(), epkey.data(), len) != len) {
-                                       ec = error::ecdsa_error::load_key_bio_write;
+                                       ec = error::rsa_error::load_key_bio_write;
                                        return {};
                                }
                        } else {
                                const int len = static_cast<int>(key.size());
                                if (BIO_write(pubkey_bio.get(), key.data(), len) != len) {
-                                       ec = error::ecdsa_error::load_key_bio_write;
+                                       ec = error::rsa_error::load_key_bio_write;
                                        return {};
                                }
                        }
@@ -800,7 +800,7 @@ namespace jwt {
                        evp_pkey_handle pkey(PEM_read_bio_PUBKEY(
                                pubkey_bio.get(), nullptr, nullptr,
                                (void*)password.data())); // NOLINT(google-readability-casting) requires `const_cast`
-                       if (!pkey) ec = error::ecdsa_error::load_key_bio_read;
+                       if (!pkey) ec = error::rsa_error::load_key_bio_read;
                        return pkey;
                }
[ RUN      ] OpenSSLErrorTest.LoadECDSAPrivateKeyFromString
/home/runner/work/jwt-cpp/jwt-cpp/tests/OpenSSLErrorTest.cpp:478: Failure
Expected equality of these values:
  ec
    Which is: rsa_error:17
  e.expected_ec
    Which is: ecdsa_error:12
[  FAILED  ] OpenSSLErrorTest.LoadECDSAPrivateKeyFromString (0 ms)
[ RUN      ] OpenSSLErrorTest.LoadECDSAPublicKeyFromString
/home/runner/work/jwt-cpp/jwt-cpp/tests/OpenSSLErrorTest.cpp:478: Failure
Expected equality of these values:
  ec
    Which is: rsa_error:17
  e.expected_ec
    Which is: ecdsa_error:12
[  FAILED  ] OpenSSLErrorTest.LoadECDSAPublicKeyFromString (0 ms)
[ RUN      ] OpenSSLErrorTest.ECDSACertificate
/home/runner/work/jwt-cpp/jwt-cpp/tests/OpenSSLErrorTest.cpp:478: Failure
Expected equality of these values:
  ec
    Which is: rsa_error:17
  e.expected_ec
    Which is: ecdsa_error:12

@prince-chrismc
Copy link
Collaborator Author

Should be possible with some more templates compiler explorer

@prince-chrismc prince-chrismc modified the milestones: 0.7.0, 0.8.0 Dec 10, 2023
@prince-chrismc prince-chrismc linked a pull request Dec 11, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant