Sitelet https://github.com/jasync-sql/jasync-sql/pull/373
Skip to content

Implement password encryption using an RSA public key - #373

Merged
oshai merged 4 commits into
jasync-sql:masterfrom
KarboniteKream:feat/rsa-public-key-encryption
Jan 26, 2023
Merged

oshai merged 4 commits into
jasync-sql:masterfrom
KarboniteKream:feat/rsa-public-key-encryption

Conversation

@KarboniteKream

@KarboniteKream KarboniteKream commented Jan 22, 2023 •

Copy link
Copy Markdown
Contributor

Description

This PR is a follow-up to #358, to implement password encryption over unsafe connection using an RSA public key. This is required for priority #2 as described on this page.

Detailed changes

  • Add rsaPublicKey property to Configuration
  • Add authentication state to MySQLConnection, and other configuration to HandshakeResponse, so they can be available during authentication phases
  • Implement password encryption using a public key in Sha256PasswordAuthentication
  • Provide custom public/private key pairs to test containers

@KarboniteKream
KarboniteKream marked this pull request as ready for review January 22, 2023 07:27
@KarboniteKream
KarboniteKream force-pushed the feat/rsa-public-key-encryption branch from 69c0a22 to e5be218 Compare January 22, 2023 07:32

@oshai oshai left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you very much for this effort! It's appreciated!
Other than the link to mysql I think it will be very hard to follow how auth works here in the driver.
And it looks like you pretty much revisited all auth methods here?
What do you think about adding an additional readme with impl details? So it will be more clear later, and easier to follow in the future.

Comment thread db-async-common/src/main/java/com/github/jasync/sql/db/Configuration.kt Outdated
Comment thread db-async-common/src/main/java/com/github/jasync/sql/db/Configuration.kt Outdated
Comment thread mysql-async/src/main/java/com/github/jasync/sql/db/mysql/MySQLConnection.kt Outdated
Comment thread db-async-common/src/main/java/com/github/jasync/sql/db/Configuration.kt Outdated
@KarboniteKream

KarboniteKream commented Jan 23, 2023 •

Copy link
Copy Markdown
Contributor Author

What do you think about adding an additional readme with impl details?

Will do! I agree it's difficult to follow, and I was thinking of extracting the authentication logic into a separate class to handle all of this in a later PR (and allow making some optimizations with reading the public key). For now I'll make README explaining how it all works, and later start work on a prototype to hopefully simplify the whole flow.

@KarboniteKream
KarboniteKream force-pushed the feat/rsa-public-key-encryption branch from f0f1ddf to a8c7d03 Compare January 24, 2023 02:00
@KarboniteKream

Copy link
Copy Markdown
Contributor Author

I've addressed the comments. Please take another look.

@@ -0,0 +1,60 @@
# Authentication methods

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Awesome!

@oshai

oshai commented Jan 25, 2023

Copy link
Copy Markdown
Contributor

Let me know if it's ready to merge or anything else you'd like to change before I merge.

@KarboniteKream

KarboniteKream commented Jan 25, 2023 •

Copy link
Copy Markdown
Contributor Author

It's ready to merge, thanks!

@oshai
oshai merged commit 0b9ea46 into jasync-sql:master Jan 26, 2023
@oshai

oshai commented Jan 26, 2023

Copy link
Copy Markdown
Contributor

Thanks you for the PR!

@KarboniteKream
KarboniteKream deleted the feat/rsa-public-key-encryption branch January 26, 2023 08:23
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.

2 participants