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

Makefile(ticdc): support build cdc in fips mode (#9961) #10131

Conversation

ti-chi-bot
Copy link
Member

This is an automated cherry-pick of #9961

What problem does this PR solve?

Issue Number: close #9962

What is changed and how it works?

  1. add support to build FIPS-ready cdc binary following the info shown in Makefile,cmd/tidb-server: add tidb-server FIPS build target tidb#47949
  2. add -fips in the release version of FIPS-ready binary
    image
  3. support sanitize version with -fips suffix when do version check

Check List

Tests

  • Unit test
    Make sure the version sanitize process works with -fips suffix;
  • Manual test (add detailed scripts or steps below)
ENABLE_FIPS=1 make cdc
go tool nm bin/cdc | grep boring

Check the boringcrypto linked via cgo, some function names like:

 6d3aa60 t local.crypto/internal/boring._Cfunc__goboringcrypto_AES_cbc_encrypt.abi0
 6d3abc0 t local.crypto/internal/boring._Cfunc__goboringcrypto_AES_ctr128_encrypt.abi0
 6d3ad20 t local.crypto/internal/boring._Cfunc__goboringcrypto_AES_decrypt.abi0
 6d3ade0 t local.crypto/internal/boring._Cfunc__goboringcrypto_AES_encrypt.abi0
 6d3aea0 t local.crypto/internal/boring._Cfunc__goboringcrypto_AES_set_decrypt_key.abi0
 6d3af80 t local.crypto/internal/boring._Cfunc__goboringcrypto_AES_set_encrypt_key.abi0
 6d3b060 t local.crypto/internal/boring._Cfunc__goboringcrypto_BN_bin2bn.abi0
 6d3b140 t local.crypto/internal/boring._Cfunc__goboringcrypto_BN_bn2bin_padded.abi0
 6d3b220 t local.crypto/internal/boring._Cfunc__goboringcrypto_BN_bn2le_padded.abi0
 6d4cec0 t local.crypto/internal/boring._Cfunc__goboringcrypto_BN_free
 6d3b300 t local.crypto/internal/boring._Cfunc__goboringcrypto_BN_free.abi0
 6d3b380 t local.crypto/internal/boring._Cfunc__goboringcrypto_BN_le2bn.abi0
 6d3b460 t local.crypto/internal/boring._Cfunc__goboringcrypto_BN_new.abi0
 6d3b4c0 t local.crypto/internal/boring._Cfunc__goboringcrypto_BN_num_bytes.abi0
 6d3b540 t local.crypto/internal/boring._Cfunc__goboringcrypto_BORINGSSL_bcm_power_on_self_test.abi0
 6d3b5a0 t local.crypto/internal/boring._Cfunc__goboringcrypto_ECDSA_sign.abi0
 6d3b700 t local.crypto/internal/boring._Cfunc__goboringcrypto_ECDSA_size.abi0
 6d3b780 t local.crypto/internal/boring._Cfunc__goboringcrypto_ECDSA_verify.abi0
 6d4cf00 t local.crypto/internal/boring._Cfunc__goboringcrypto_EC_GROUP_free
 6d3b8e0 t local.crypto/internal/boring._Cfunc__goboringcrypto_EC_GROUP_free.abi0
 6d3b960 t local.crypto/internal/boring._Cfunc__goboringcrypto_EC_GROUP_new_by_curve_name.abi0
 6d4cf40 t local.crypto/internal/boring._Cfunc__goboringcrypto_EC_KEY_free
 6d3ba00 t local.crypto/internal/boring._Cfunc__goboringcrypto_EC_KEY_free.abi0
 6d3ba80 t local.crypto/internal/boring._Cfunc__goboringcrypto_EC_KEY_generate_key_fips.abi0
 6d3bb00 t local.crypto/internal/boring._Cfunc__goboringcrypto_EC_KEY_get0_group.abi0
 6d3bb80 t local.crypto/internal/boring._Cfunc__goboringcrypto_EC_KEY_get0_private_key.abi0
 6d3bc00 t local.crypto/internal/boring._Cfunc__goboringcrypto_EC_KEY_get0_public_key.abi0
 6d3bc80 t local.crypto/internal/boring._Cfunc__goboringcrypto_EC_KEY_new_by_curve_name.abi0
 6d3bd20 t local.crypto/internal/boring._Cfunc__goboringcrypto_EC_KEY_set_private_key.abi0
 6d3bde0 t local.crypto/internal/boring._Cfunc__goboringcrypto_EC_KEY_set_public_key.abi0
 6d4cf80 t local.crypto/internal/boring._Cfunc__goboringcrypto_EC_POINT_free
 6d3bea0 t local.crypto/internal/boring._Cfunc__goboringcrypto_EC_POINT_free.abi0

Questions

Will it cause performance regression or break compatibility?

The original binary should not be affected. But the binary build with FIPS support is expected to be slower.

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

Need to add contents about the support for FIPS-ready binary and how to download it later;

Release note

Support build fips-ready cdc binary

@ti-chi-bot ti-chi-bot added first-time-contributor Indicates that the PR was contributed by an external member and is a first-time contributor. lgtm release-note Denotes a PR that will be considered when it comes time to generate release notes. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. type/cherry-pick-for-release-6.5 This PR is cherry-picked to release-6.5 from a source PR. labels Nov 22, 2023
@lidezhu
Copy link
Collaborator

lidezhu commented Nov 29, 2023

/cherry-pick-invite

1 similar comment
@wuhuizuo
Copy link
Contributor

/cherry-pick-invite

@ti-chi-bot
Copy link
Member Author

@wuhuizuo Please accept the invitation then you can push to the cherry-pick pull requests.
Comment with "/cherry-pick-invite" if the invitation is expired.
https://github.com/ti-chi-bot/tiflow/invitations

@purelind
Copy link
Collaborator

/cherry-pick-invite

@ti-chi-bot
Copy link
Member Author

@purelind Please accept the invitation then you can push to the cherry-pick pull requests.
Comment with "/cherry-pick-invite" if the invitation is expired.
https://github.com/ti-chi-bot/tiflow/invitations

@lidezhu
Copy link
Collaborator

lidezhu commented Nov 29, 2023

/cherry-pick-invite

@ti-chi-bot
Copy link
Member Author

@lidezhu Please accept the invitation then you can push to the cherry-pick pull requests.
Comment with "/cherry-pick-invite" if the invitation is expired.
https://github.com/ti-chi-bot/tiflow/invitations

@lidezhu
Copy link
Collaborator

lidezhu commented Nov 29, 2023

/retest

@overvenus
Copy link
Member

/hold

@ti-chi-bot ti-chi-bot bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 30, 2023
@overvenus overvenus changed the base branch from release-6.5 to feature/release-6.5-fips December 11, 2023 03:51
@overvenus
Copy link
Member

/unhold

@ti-chi-bot ti-chi-bot bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 11, 2023
Copy link
Contributor

ti-chi-bot bot commented Dec 11, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sdojjy

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

The pull request process is described here

Needs approval from an approver in each of these files:

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 ti-chi-bot bot added the approved label Dec 11, 2023
@lidezhu
Copy link
Collaborator

lidezhu commented Dec 11, 2023

/test verify

@lidezhu
Copy link
Collaborator

lidezhu commented Dec 11, 2023

/test verify

2 similar comments
@lidezhu
Copy link
Collaborator

lidezhu commented Dec 11, 2023

/test verify

@lidezhu
Copy link
Collaborator

lidezhu commented Dec 11, 2023

/test verify

Copy link
Contributor

ti-chi-bot bot commented Dec 11, 2023

@ti-chi-bot: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
jenkins-ticdc/verify 60f6602 link true /test verify

Full PR test history. Your PR dashboard.

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/test-infra repository. I understand the commands that are listed here.

@overvenus
Copy link
Member

/verify

@overvenus
Copy link
Member

/test verify

Copy link
Contributor

ti-chi-bot bot commented Dec 11, 2023

@overvenus: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

  • /test verify
  • /test wip-pull-build
  • /test wip-pull-check
  • /test wip-pull-unit-test-cdc
  • /test wip-pull-unit-test-dm
  • /test wip-pull-unit-test-engine

Use /test all to run the following jobs that were automatically triggered:

  • pingcap/tiflow/ghpr_verify

In response to this:

/test verify

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/test-infra repository.

@ti-chi-bot ti-chi-bot bot merged commit ec9ddbe into pingcap:feature/release-6.5-fips Dec 11, 2023
8 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved first-time-contributor Indicates that the PR was contributed by an external member and is a first-time contributor. lgtm release-note Denotes a PR that will be considered when it comes time to generate release notes. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. type/cherry-pick-for-release-6.5 This PR is cherry-picked to release-6.5 from a source PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants