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

Should Drogon have built-in cryptographic functions? #671

Closed
marty1885 opened this issue Jan 2, 2021 · 20 comments
Closed

Should Drogon have built-in cryptographic functions? #671

marty1885 opened this issue Jan 2, 2021 · 20 comments
Assignees

Comments

@marty1885
Copy link
Member

marty1885 commented Jan 2, 2021

Happy new year!

#646 suggested to add security guidelines. I think that's a great idea. Especially if we could make Drogon secure by default. As of now. Drogon currently doesn't come with "security features" per se. Contrast to PHP have a built-in password_hash.

I've password hashing/verification and secure number/string generation upstream-able in my webapp. But I'm not sure if it should be upstreamed. They pull in extra dependencies that the core framework don't need and is trivial to write. And languages like Go and Rust asks user to pull in their own library.

I guess the question is:

  • Should Drogon provide cryptographic functions? Or is it the user's job?
  • How much security could we provide without extra dependencies?
  • If yes, To what extent should Drogon assist users?
    • I don't expect drogon to support full-on SRP and GPG. That's bloat.
    • In the mean while, I want to avoid users storing password in the database.

What are your thoughts?
Thanks.

@an-tao
Copy link
Member

an-tao commented Jan 2, 2021

Happy New Year!
@marty1885 thanks for the suggestion. This is a good idea. I think any common features of web applications should be considered for adding to Drogon. Is OpenSSL an enough dependency for this?
Plain text passwords shouldn't be stored in the database, instead, we should store something like hash(hash(password)+salt) in the database.
@rbugajewski what do you think about this topic? thanks.

@marty1885
Copy link
Member Author

marty1885 commented Jan 2, 2021

No, OpenSSL is not enough. It does not support proper password hashes like argon2 and pbkdf2. At best I can make something "good enough" like sha256(sha256(password)+salt) but that's far form secure. My current code uses Botan2. Which will be an extra dependency and not available on older distros (ex: Ubuntu 16.04).

I can try Crypto++. But it's hard to get right. And that feels wrong for security functions.

@an-tao
Copy link
Member

an-tao commented Jan 2, 2021

Botan2 looks great, It looks like it can replace the openssl. I plan to create Drogon2 project that uses c++20 and boost.asio, The burden of compatibility with the old system can be thrown away in this version. We could discuss the Drogon2 together in another issue.

@marty1885
Copy link
Member Author

Can't wait for Drogon2! Would that be a new library or based on drogon? I too have a lot want to improve but can't because of backwards compatibility. <insert a paragraph on how long I've tried to add http2>

Anyway, I rather push nothing than a half-backed solution. Maybe postpone this until drogon2?

@an-tao
Copy link
Member

an-tao commented Jan 2, 2021

As a workaround, maybe creating a plugin project like this can help u. what do you think?

@seiichi-yoshimune
Copy link

seiichi-yoshimune commented Jan 2, 2021

Botan2 looks great, It looks like it can replace the openssl. I plan to create Drogon2 project that uses c++20 and boost.asio, The burden of compatibility with the old system can be thrown away in this version. We could discuss the Drogon2 together in another issue.

Great, any rough timeline on Drogon2 project?

@an-tao
Copy link
Member

an-tao commented Jan 2, 2021

Botan2 looks great, It looks like it can replace the openssl. I plan to create Drogon2 project that uses c++20 and boost.asio, The burden of compatibility with the old system can be thrown away in this version. We could discuss the Drogon2 together in another issue.

Great, any rough timeline on Drogon2 project?

Not yet, I'm very busy recently. actually I've create the Drogon2 project and implemented most functions of Trantor by boost.asio.

@roq3
Copy link
Contributor

roq3 commented Mar 6, 2021

What about JWT ? I can make it as contributor for You guys and I need that.

@an-tao
Copy link
Member

an-tao commented Mar 6, 2021

What about JWT ? I can make it as contributor for You guys and I need that.

@roq3 Great and thanks so much! I look forward to your contribution.
Please tell us more details about your plan. This is a big feature, and we can have some discussion before you proceed.

@roq3
Copy link
Contributor

roq3 commented Mar 6, 2021

Big or not, but i started using drogon a few days ago and i missing JWT for my project (API REST), so u know...

There is a lot of good libs on github, like: https://github.com/adhocore/php-jwt JWT for PHP.

JWT for Drogon could be pretty much like this, same methods, same structure, etc, what u think?

@an-tao
Copy link
Member

an-tao commented Mar 7, 2021

I think we should make a JWTPlugin and a JWTFilter for this.

@roq3
Copy link
Contributor

roq3 commented Mar 7, 2021

I think we should make a JWTPlugin and a JWTFilter for this.

should be good

@roq3
Copy link
Contributor

roq3 commented Mar 14, 2021

im working on some private project based on drogon. I need hash functions for security: eg. store passwords in database, etc.

I think we need add this feature, cuz for now i need add some cpp/hpp files my own to drogon app to use sha256.

Easy way: http://www.zedwood.com/article/cpp-sha256-function
But is it good for us? or just add new dependency to crypto++

if we think about plugin for hashing with about 200 lines maybe this should be implemented in drogon framework. If there could be more lines and features maybe plugin will be better?

@roq3 roq3 mentioned this issue Mar 14, 2021
@marty1885
Copy link
Member Author

marty1885 commented Mar 15, 2021

@roq3 I've discussed with @an-tao and concluded that we shouldn't add additional dependencies to Drogon's core library. As the embedded users may not have the resource. And writing an supplementary then include/link it in your project will be the proper approach.

This is the password hashing and verification code I use in my private projects. Which additionally depends on libbsd and Botan2 (and needs C++17). Hope it helps.
https://gist.github.com/marty1885/6615102db63ba879ee51b89537e332f0
Consider it licensed under WTFPL. Feel free to do anything to it.

Hashing passwords with SHA256 is a bad idea. Use something like Argon2 or PBKDF2.

@rbugajewski
Copy link
Collaborator

I also think security related functionality should be kept outside of upstream. We already have enough open ends, features to implement, bugs to fix, and there are not enough volunteers in my opinion to deliver everything in a timely manner. By adding hashing functionality, even if this is “just” 200 LOCs, we will add maintenance burden to the currently active developers, and in the worst case open a can full of security holes.

@roq3
Copy link
Contributor

roq3 commented Mar 15, 2021

https://gist.github.com/marty1885/6615102db63ba879ee51b89537e332f0

thx, i will try this.

SHA256 with salt for password hashing should be ok, but i will change to what U suggest.

@marty1885
Copy link
Member Author

Closing the issue as we decided to not add dependencies. Fell free to reopen.

@KaungZawHtet
Copy link

Don't add dependencies into the core if u don't wish. But in my opinion, it's good to force the drogon users to use dependency manager (Conan & Vcpkg) for their colourful web application development. This is 2021 and every language is going well with dependency manager. C++ is only one left a little behind. We need to move our dev culture forward.

@rajhlinux
Copy link

Any recommendations on how I can issue tokens that are safe?
Everyone says to use JWT tokens, is UUIDv4 safe enough which in included into the drogon web framework?

Thanks.

@marty1885
Copy link
Member Author

marty1885 commented Mar 5, 2023

@rajhlinux JWT is more versatile, yes. I've been considering adding support it in the drogon-assist library. But I'm working on other major improvements right now. UUIDv4 is good enough for most applications. Personally I use secure random + base64.

std::vector<uint8_t> vec(16);
utils::secureRandomBytes(vec.data(), vec.size());
auto token = utils::base64Encode(vec.data(), vec.size());

Or use the secure random string in assist.

auto token = drassist::secureRandomString(32);

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

No branches or pull requests

7 participants