Replace HMAC based flag verification with bcrypt hashing#45
Merged
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR replaces deterministic HMAC-based flag verification with bcrypt hashing, aligning flag handling with the project’s existing password hashing approach and removing the FLAG_HMAC_SECRET dependency across the codebase.
Changes:
- Introduces bcrypt-based flag hashing/verification (
HashFlag,CheckFlag) and updates flag submission logic to use bcrypt comparisons. - Removes
FLAG_HMAC_SECRETfrom configuration, logging/redaction, docs, and SQL generation scripts. - Updates tests and SQL generators to reflect salted (non-deterministic) bcrypt hashes and plaintext
submissions.provided.
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
internal/utils/flag.go |
Replaces HMAC helpers with bcrypt-based flag hashing and verification. |
internal/utils/flag_test.go |
Updates unit tests for salted bcrypt hashes and verification behavior. |
internal/service/ctf_service.go |
Hashes flags with bcrypt on create/update and verifies submissions via bcrypt compare. |
internal/config/config.go |
Removes HMAC secret config and renames bcrypt cost field to BcryptCost. |
internal/config/config_test.go |
Updates config tests to match removed HMAC secret and renamed bcrypt cost field. |
scripts/sql_common/crypto_utils.py |
Removes HMAC flag hashing and adds bcrypt-based hash_flag. |
scripts/generate_yaml_sql/* |
Removes HMAC secret usage and generates bcrypt flag_hash values. |
scripts/generate_dummy_sql/* |
Removes HMAC secret usage; stores plaintext submission provided values and bcrypt challenge flag_hash. |
README.md, .env.example, scripts/generate_dummy_sql/defaults/settings.yaml |
Removes FLAG_HMAC_SECRET references and updates documentation/config defaults. |
Various *_test.go files |
Updates service/repo/http tests to use bcrypt-based flag hashing/verification and renamed config field. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Member
Author
diff --git a/internal/utils/flag.go b/internal/utils/flag.go
index 7db6cf3..79bb661 100644
--- a/internal/utils/flag.go
+++ b/internal/utils/flag.go
@@ -11,6 +11,14 @@ func HashFlag(flag string, cost int) (string, error) {
return string(hash), nil
}
-func CheckFlag(hash, flag string) bool {
- return bcrypt.CompareHashAndPassword([]byte(hash), []byte(flag)) == nil
+func CheckFlag(hash, flag string) (bool, error) {
+ if err := bcrypt.CompareHashAndPassword([]byte(hash), []byte(flag)); err != nil {
+ if err == bcrypt.ErrMismatchedHashAndPassword {
+ return false, nil
+ }
+
+ return false, err
+ }
+
+ return true, nil
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Previously, flags were protected using an HMAC secret for encryption. While this approach was chosen for security reasons, we determined that using Bcrypt provides sufficiently strong protection without relying on HMAC. Unlike general-purpose hashing functions, Bcrypt is specifically designed to be computationally expensive and highly resistant to brute-force attacks.
Since Bcrypt is already used for password verification, adopting the same approach improves maintainability and consistency. It also helps mitigate potential risks such as HMAC key exposure or weak HMAC secret configurations.
There are no changes to the REST API specification as a result of this update. For more details, please refer to the source code.
The Bcrypt cost parameter uses the
BCRYPT_COSTenvironment variable as-is and is shared with the password configuration.