sync-diff: replace hardcoded use of MD5 with configurable option to support FIPS-compliant environments. by maxz-db · Pull Request #12622 · pingcap/tiflow

maxz-db · 2026-04-24T17:53:56Z

What problem does this PR solve?

Issue Number: close #12627

What is changed and how it works?

Added a new checksum-algorithm configuration flag to sync-diff-inspector:
Supported options: md5" and "sha256" hash functions for checksumming
Default: md5 for backwards compatibility.

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
Manual test on dev environment.
No code

Questions

Will it cause performance regression or break compatibility?

No. This only reuses the existing binlog value normalization path in validator.

Do you need to update user documentation, design documentation or monitoring documentation?

No.

Release note

sync-diff: replace hardcoded use of MD5 with configurable option to support FIPS-compliant environments.

ti-chi-bot · 2026-04-24T17:54:02Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign 5kbpers for approval. For more information see the Code Review Process.
Please ensure that each of them provides their approval before proceeding.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

ti-chi-bot · 2026-04-24T17:54:06Z

Hi @maxz-db. Thanks for your PR.

I'm waiting for a pingcap member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

ti-chi-bot · 2026-04-24T17:54:06Z

Welcome @maxz-db!

It looks like this is your first PR to pingcap/tiflow 🎉.

I'm the bot to help you request reviewers, add labels and more, See available commands.

We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to pingcap/tiflow. 😃

pingcap-cla-assistant · 2026-04-24T17:54:10Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

gemini-code-assist

Code Review

This pull request introduces support for SHA256 checksumming alongside the existing MD5 implementation to facilitate FIPS compliance. Key changes include the addition of a ChecksumAlgorithm configuration type, updates to the Config and DataSource structs, and the modification of the core checksum utility to dynamically generate SQL using either MD5 or SHA2. Review feedback recommends making the new --checksum-algorithm flag visible to users and reverting the args parameter type from []interface{} to []any to maintain consistency with modern Go standards.

… option to support FIPS-compliant environments. (pingcap#6) Context: TiDB FIPS build mode introduced in 7.6.0: pingcap/tidb#47949. TiDB binaries built with FIPS 140-3 compliance mode disable MD5 hashing in OpenSSL library used by TiKV. Problem: sync-diff-inspector relies on hardcoded MD5() for chunk checksumming. For performance reasons, TiDB may push expression evaluation down to TiKV coprocessor (tidb_query_expr), which uses OpenSSL for cryptographic functions. In FIPS mode, TiKV's OpenSSL inner_evp_generic_fetch() tries to load MD5 algorithm and fails with error code 50856204 (EVP_R_UNSUPPORTED). As a result, sync-diff-inspector fails because TiDB rejected all MD5-based checksum queries due to OpenSSL security policy restrictions. Changes: - Added a new `checksum-algorithm` configuration flag: - Supported options: md5" and "sha256" hash functions for checksumming - Default: md5 for backwards compatibility.

joechenrh

And please update the PR description.

joechenrh · 2026-05-06T02:17:39Z

+	if len(ds) > 0 {
+		checksumAlgorithm = ds[0].ChecksumAlgorithm
+	}


len(ds) is validated to be positive before entering this function.

joechenrh · 2026-05-06T02:29:46Z

 	Conn          *sql.DB
 	SessionConfig SessionConfig `toml:"session" json:"session"`
+
+	ChecksumAlgorithm ChecksumAlgorithm `toml:"checksum-algorithm" json:"checksum-algorithm"`


How about passing Config to buildSourceFromCfg, so we don't need store ChecksumAlgorithm in the DataSource.

D3Hunter · 2026-05-06T09:55:39Z

 	db *sql.DB, schemaName, tableName string,
-	tbInfo *model.TableInfo, limitRange string, indexHint string, args []any,
+	tbInfo *model.TableInfo, limitRange string, indexHint string,
+	args []any, checksumAlgorithm string,


can we pass the param as ChecksumAlgorithm instead of string

D3Hunter · 2026-05-06T09:56:19Z

/ok-to-test

ti-chi-bot · 2026-05-06T10:34:07Z

@maxz-db: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-check	`98099ee`	link	true	`/test pull-check`
pull-dm-integration-test-next-gen	`98099ee`	link	false	`/test pull-dm-integration-test-next-gen`
pull-dm-integration-test	`98099ee`	link	true	`/test pull-dm-integration-test`

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

ti-chi-bot Bot added do-not-merge/needs-linked-issue do-not-merge/needs-triage-completed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Apr 24, 2026

ti-chi-bot Bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Apr 24, 2026

gemini-code-assist Bot reviewed Apr 24, 2026

View reviewed changes

Comment thread sync_diff_inspector/config/config.go Outdated

Comment thread sync_diff_inspector/utils/utils.go Outdated

Comment thread sync_diff_inspector/utils/utils_test.go Outdated

Comment thread sync_diff_inspector/utils/utils_test.go Outdated

maxz-db force-pushed the sync-diff-inspector-fips branch from 3a17fe4 to 98099ee Compare May 5, 2026 01:05

joechenrh reviewed May 6, 2026

View reviewed changes

D3Hunter changed the title ~~Replace hardcoded use of MD5 in sync-diff-inspector with configurable option to support FIPS-compliant environments.~~ sync-diff: replace hardcoded use of MD5 with configurable option to support FIPS-compliant environments. May 6, 2026

D3Hunter reviewed May 6, 2026

View reviewed changes

ti-chi-bot Bot added ok-to-test Indicates a PR is ready to be tested. and removed needs-ok-to-test Indicates a PR created by contributors and need ORG member send '/ok-to-test' to start testing. labels May 6, 2026

Conversation

maxz-db commented Apr 24, 2026 • edited by D3Hunter Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What problem does this PR solve?

What is changed and how it works?

Check List

Tests

Questions

Will it cause performance regression or break compatibility?

Do you need to update user documentation, design documentation or monitoring documentation?

Release note

Uh oh!

ti-chi-bot Bot commented Apr 24, 2026

Uh oh!

ti-chi-bot Bot commented Apr 24, 2026

Uh oh!

ti-chi-bot Bot commented Apr 24, 2026

Uh oh!

pingcap-cla-assistant Bot commented Apr 24, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

joechenrh left a comment

Choose a reason for hiding this comment

Uh oh!

joechenrh May 6, 2026

Choose a reason for hiding this comment

Uh oh!

joechenrh May 6, 2026

Choose a reason for hiding this comment

Uh oh!

D3Hunter May 6, 2026

Choose a reason for hiding this comment

Uh oh!

D3Hunter commented May 6, 2026

Uh oh!

ti-chi-bot Bot commented May 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

maxz-db commented Apr 24, 2026 •

edited by D3Hunter

Loading