Add client_ed25519 authentication

[Gusted]

Description

Implements the necessary client code for ed25519 authentication. A continuation of #1220, but doesn't use CGO. This patch uses https://filippo.io/edwards25519 to implement the crypto bits. The standard library crypto/ed25519 cannot be used as MariaDB chose a scheme that is simply not compatible with what the standard library provides.

Checklist

• Code compiles correctly
• Created tests which fail without the change (if possible)
• All tests passing
• Extended the README / documentation, if necessary
• Added myself / the copyright holder to the AUTHORS file

Summary by CodeRabbit

• New Features

  • Implemented support for "client_ed25519" authentication plugin to enhance security.

• Bug Fixes

  • Improved error handling in test functions for more accurate error reporting.

• Tests

  • Added new test cases for Ed25519 authentication process.

• Documentation

  • Updated the AUTHORS file to acknowledge a new contributor.

• Refactor

  • Replaced doEd25519Auth function with a more robust authEd25519 function.
  • Streamlined cleanup process in tests with deferred table drop execution.

[coderabbitai]

Walkthrough

The changes involve adding support for Ed25519 authentication in an authentication module, including the necessary cryptographic imports and a new authentication function. A test for this authentication method has also been added. Additionally, there's a new contributor added to the AUTHORS file. In a separate test file, there's improved cleanup with a DROP TABLE command and refined error handling to ensure errors are reported correctly in the test context.

Changes

File(s)	Summary
auth.go, auth_test.go	Introduced crypto/sha512 and filippo.io/edwards25519 imports, added authEd25519 function for Ed25519 authentication, and included a test for the authentication process.
AUTHORS	Added a new contributor "Gusted" with contact information.
driver_test.go	Added cleanup commands and updated error handling within the TestRawBytesAreNotModified function.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on X ?

---

Tips

Chat with CodeRabbit Bot (@coderabbitai)

• You can reply to a review comment from CodeRabbit.
• You can also chat with CodeRabbit bot directly around the specific lines of code or files in the PR by tagging @coderabbitai in a new comment.
• You can tag @coderabbitai in a PR comment and ask questions about the PR and the codebase.

CodeRabbit Commands (invoked as PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• The JSON schema for the configuration file is available here.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

[coderabbitai]

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between 98d7289 and 7f66ff9.

Files ignored due to filter (2)

• go.mod
• go.sum

Files selected for processing (2)

• auth.go (3 hunks)
• auth_test.go (1 hunks)

Additional comments: 4

auth.go (3)

• 13-22: The addition of crypto/sha512 and filippo.io/edwards25519 imports aligns with the PR objective to support ed25519 authentication.

• 231-266: The doEd25519Auth function correctly implements the ed25519 authentication logic using SHA-512 hashing and the edwards25519 package, as required by the PR objective.

• 334-339: The modification to the auth method to handle the "client_ed25519" plugin is correct and aligns with the PR objective.

auth_test.go (1)

• 1332-1381: The TestEd25519Auth function appears to be correctly implemented based on the PR objective and the provided summary. It tests the ed25519 authentication process as expected.

[coderabbitai]

Review Status

Actionable comments generated: 2

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between 98d7289 and e4ea511.

Files ignored due to filter (2)

• go.mod
• go.sum

Files selected for processing (3)

• AUTHORS (1 hunks)
• auth.go (3 hunks)
• auth_test.go (1 hunks)

Files skipped from review due to trivial changes (1)

• AUTHORS

Files skipped from review as they are similar to previous changes (1)

• auth.go

Additional comments: 2

auth_test.go (2)

• 1332-1381: The addition of TestEd25519Auth function is consistent with the PR objective to add support for Ed25519 authentication and the summary provided. The test appears to be well-structured, sending a client authentication packet, checking the written authentication response, and handling the server's OK response.

• 1332-1381: The test function TestEd25519Auth is correctly implemented to test the Ed25519 authentication process. It sends a client authentication packet, checks the written authentication response, and handles the server's OK response.

[Gusted]

Hi @methane,

Is there anything required on my part to get this merged?

[coderabbitai]

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between 98d7289 and 68789d1.

Files ignored due to filter (2)

• go.mod
• go.sum

Files selected for processing (3)

• AUTHORS (1 hunks)
• auth.go (3 hunks)
• auth_test.go (1 hunks)

Files skipped from review due to trivial changes (1)

• AUTHORS

Additional comments: 6

auth.go (5)

• 13-22: The addition of crypto/sha512 and filippo.io/edwards25519 imports aligns with the PR's objective to implement ed25519 authentication.

• 231-267: The implementation of doEd25519Auth should be carefully reviewed to ensure it correctly follows the ed25519 algorithm and adheres to security best practices. The use of SetBytesWithClamping and SetUniformBytes should be verified against the MariaDB documentation or the ed25519 specification to ensure the authentication process is secure and correct.

• 236-237: Error handling within doEd25519Auth is done correctly by returning errors to the caller, allowing for proper error management upstream.

Also applies to: 246-247, 260-261

• 334-339: The integration of doEd25519Auth into the auth function with a case for "client_ed25519" is done correctly. It is important to ensure that this new case does not interfere with the existing authentication methods and that the length check for authData is appropriate for the "client_ed25519" plugin.

• 231-267: The doEd25519Auth function is correctly not exported, maintaining consistency with the encapsulation of other authentication functions in auth.go.

auth_test.go (1)

• 1332-1381: The implementation of TestEd25519Auth appears to be consistent with the existing test patterns in the file, as it checks the length and byte equality of the authentication response. This approach aligns with the previous discussion and the decision to maintain consistency with the established testing approach.

[methane]

No.
I need to learn about ed25519, because this code uses low level APIs instead of high level APIs in stdlib.
(I have used ed25519 as a black box so I can review only code using high level APIs.)

[coveralls]

coverage: 82.681% (-0.04%) from 82.719%
when pulling ae976ad on Gusted:ed25519-auth
into d9f4383 on go-sql-driver:master.

[coderabbitai]

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between 68789d1 and ee271ad.

Files selected for processing (1)

• auth.go (3 hunks)

Additional comments: 5

auth.go (5)

• 231-267: The implementation of authEd25519 seems to follow the ed25519 authentication process as specified by MariaDB's documentation. Ensure that the cryptographic operations and the control flow are correctly implemented and that there are no security issues with the way the password and the scramble are being used.

• 334-338: The handling of the "client_ed25519" case in the auth function checks for the correct length of authData. This is a good practice to ensure that the data being processed is as expected. However, ensure that the length check aligns with the protocol's specifications and that the error handling is consistent with the rest of the codebase.

• 334-339: The addition of the "client_ed25519" case in the auth function's switch statement is correctly implemented and follows the pattern of the other cases. Ensure that the integration with the existing authentication flow is tested and that there are no edge cases that could cause unexpected behavior.

• 232-266: The usage of the filippo.io/edwards25519 library in the authEd25519 function appears to be correct. However, it is crucial to ensure that the library's functions are used securely and that the cryptographic operations are performed as intended by the MariaDB's ed25519 authentication specification.

• 231-267: The authEd25519 function is a critical part of the new authentication feature. Ensure that there are comprehensive tests covering various scenarios, including edge cases and error handling, to validate the correctness and security of the implementation.

[methane]

I tested this code works with MariaDB.

1. Create user with ed25519

$ docker run -e MYSQL_ALLOW_EMPTY_PASSWORD=yes -p 3306:3306 --rm --name mysql mariadb:latest

$ mysql -uroot -h127.0.0.1

MariaDB [(none)]> INSTALL SONAME 'auth_ed25519';
Query OK, 0 rows affected (0.019 sec)

MariaDB [(none)]> CREATE USER username@'%' IDENTIFIED VIA ed25519 USING PASSWORD('secret');
Query OK, 0 rows affected (0.012 sec)

$ mariadb -h127.0.0.1 -uusername -psecret
Welcome to the MariaDB monitor.  Commands end with ; or \g.
Your MariaDB connection id is 5
Server version: 11.2.2-MariaDB-1:11.2.2+maria~ubu2204 mariadb.org binary distribution

2. Connect to the MariaDB server from go

package main

import (
	"database/sql"
	"fmt"

	_ "github.com/go-sql-driver/mysql"
)

func must(err error) {
	if err != nil {
		panic(err)
	}
}

func main() {
	db, err := sql.Open("mysql", "username:secret@tcp(127.0.0.1:3306)/")
	must(err)
	defer db.Close()

	var i int
	must(db.QueryRow(`SELECT 12`).Scan(&i))
	fmt.Println(i)
}

[coderabbitai]

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between ee271ad and a560843.

Files selected for processing (1)

• driver_test.go (1 hunks)

Additional comments: 1

driver_test.go (1)

• 165-178: > Note: This review was outside the patches, and no patch overlapping with it was found. Original lines [15-18]

The test suite appears to be well-structured and covers a broad spectrum of the MySQL driver's functionality, ensuring robustness and reliability of the driver under various conditions.

[methane]

=== NAME  TestRawBytesAreNotModified/default
    testing.go:1490: test executed panic(nil) or runtime.Goexit: subtest may have called FailNow on a parent test
--- FAIL: TestRawBytesAreNotModified (0.06s)
    --- FAIL: TestRawBytesAreNotModified/default (0.06s)

[coderabbitai]

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between a560843 and ae976ad.

Files selected for processing (1)

• driver_test.go (3 hunks)

Additional comments: 1

driver_test.go (1)

• 3181-3194: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [3181-3204]

The added error handling for QueryContext and Scan calls within the loop is a good practice to ensure that any issues during query execution or data retrieval are properly reported and can be debugged effectively.