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

Feat/connect: Entropy check workflow in ResetDevice #15887

Draft
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

szymonlesisz
Copy link
Contributor

@szymonlesisz szymonlesisz commented Dec 10, 2024

Description

implementation of entropy check workflow

TODO:

  • random tries
  • Device.capability ? (check is based on version)
  • e2e tests for node and web

Copy link

🚀 Expo preview is ready!

  • Project → trezor-suite-preview
  • Platforms → android, ios
  • Scheme → trezorsuitelite
  • Runtime Version → 20
  • More info

Learn more about 𝝠 Expo Github Action

Buffer.from(options.trezorEntropy, 'hex'),
Buffer.from(options.hostEntropy, 'hex'),
]);
const entropy = crypto.createHash('sha256').update(data).digest();

Check failure

Code scanning / CodeQL

Use of password hash with insufficient computational effort

Password from [a call to _commonCall](1) is hashed insecurely. Password from [a call to _commonCall](2) is hashed insecurely.

Copilot Autofix AI about 16 hours ago

To fix the problem, we should replace the use of crypto.createHash('sha256') with a more secure password hashing scheme such as bcrypt. This will ensure that the hashed entropy data is resistant to brute-force attacks. The best way to fix the problem without changing existing functionality is to use the bcrypt library to hash the entropy data.

  1. Install the bcrypt library if it is not already installed.
  2. Import the bcrypt library in the relevant file.
  3. Replace the crypto.createHash('sha256') function with bcrypt.hashSync to hash the entropy data.
Suggested changeset 2
packages/connect/src/api/firmware/verifyEntropy.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/packages/connect/src/api/firmware/verifyEntropy.ts b/packages/connect/src/api/firmware/verifyEntropy.ts
--- a/packages/connect/src/api/firmware/verifyEntropy.ts
+++ b/packages/connect/src/api/firmware/verifyEntropy.ts
@@ -2,2 +2,3 @@
 import crypto from 'crypto';
+import bcrypt from 'bcrypt';
 
@@ -92,3 +93,4 @@
     ]);
-    const entropy = crypto.createHash('sha256').update(data).digest();
+    const saltRounds = 10;
+    const entropy = Buffer.from(bcrypt.hashSync(data.toString('hex'), saltRounds), 'hex');
     const strength = Math.floor(options.strength / 8);
EOF
@@ -2,2 +2,3 @@
import crypto from 'crypto';
import bcrypt from 'bcrypt';

@@ -92,3 +93,4 @@
]);
const entropy = crypto.createHash('sha256').update(data).digest();
const saltRounds = 10;
const entropy = Buffer.from(bcrypt.hashSync(data.toString('hex'), saltRounds), 'hex');
const strength = Math.floor(options.strength / 8);
packages/connect/package.json
Outside changed files

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/packages/connect/package.json b/packages/connect/package.json
--- a/packages/connect/package.json
+++ b/packages/connect/package.json
@@ -88,3 +88,4 @@
         "bs58check": "^4.0.0",
-        "cross-fetch": "^4.0.0"
+        "cross-fetch": "^4.0.0",
+        "bcrypt": "^5.1.1"
     },
EOF
@@ -88,3 +88,4 @@
"bs58check": "^4.0.0",
"cross-fetch": "^4.0.0"
"cross-fetch": "^4.0.0",
"bcrypt": "^5.1.1"
},
This fix introduces these dependencies
Package Version Security advisories
bcrypt (npm) 5.1.1 None
Copilot is powered by AI and may make mistakes. Always verify output.
Unable to commit as this autofix suggestion is now outdated
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
@szymonlesisz szymonlesisz force-pushed the feat/connect-display_random branch from 1f5d46a to 7030c83 Compare December 11, 2024 11:53
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.

1 participant