Feature: sui keystore #17618
Draft
Feature: sui keystore #17618
Conversation
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
JoaoVasques
reviewed
May 7, 2025
|
I see you added a changeset file but it does not contain a tag. Please edit the text include at least one of the following tags:
|
faisal-chainlink
force-pushed
the
feature/sui-keystore
branch
2 times, most recently
from
ff72943 to
2515808
Compare
Flakeguard SummaryRan new or updated tests between View Flaky Detector Details | Compare Changes Found Flaky Tests ❌12 Results
ArtifactsFor detailed logs of the failed tests, please refer to the artifact failed-test-results-with-logs.json. |
cfal
force-pushed
the
feature/sui-keystore
branch
from
2515808 to
48a5dfd
Compare
cfal
requested review from
winder,
asoliman92,
matYang,
makramkd,
AmrMohamedRezk,
0xAustinWang and
b-gopalswami
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| // newFrom creates a new Account from a provided random reader | ||
| func newFrom(reader io.Reader) (Key, error) { | ||
| pub, priv, err := ed25519.GenerateKey(reader) |
Check failure
Code scanning / CodeQL
Use of insufficient randomness as the key of a cryptographic algorithm High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 5 days ago
To address the issue, we need to ensure that cryptographic key generation always uses a cryptographically secure random number generator, even in testing scenarios. The MustNewInsecure function should be modified to reject insecure random number generators or be clearly restricted to testing environments. Additionally, the keystest.NewRandReaderFromSeed function should be updated to make its insecure nature explicit and prevent its accidental use in production.
- Modify
MustNewInsecureto enforce the use of a secure random number generator or restrict its usage to testing environments by adding a build tag or runtime check. - Update
keystest.NewRandReaderFromSeedto include a warning or runtime check to ensure it is only used in test builds. - Ensure that all cryptographic key generation paths use
crypto/rand.Readeror an equivalent secure source of randomness.
Suggested changeset
2
core/services/keystore/keys/suikey/key.go
| @@ -41,4 +41,7 @@ | ||
|
|
||
| // MustNewInsecure returns an Account if no error | ||
| // MustNewInsecure returns an Account if no error. This function is intended for testing only. | ||
| func MustNewInsecure(reader io.Reader) Key { | ||
| if !isTestEnvironment() { | ||
| panic("MustNewInsecure is intended for testing only and cannot be used in production") | ||
| } | ||
| key, err := newFrom(reader) | ||
| @@ -49,2 +52,7 @@ | ||
| } | ||
|
|
||
| // isTestEnvironment checks if the code is running in a test environment. | ||
| func isTestEnvironment() bool { | ||
| return os.Getenv("GO_ENV") == "test" | ||
| } | ||
|
|
core/internal/testutils/keystest/seedable_rand_reader.go
Outside changed files
| @@ -11,3 +11,11 @@ | ||
| func NewRandReaderFromSeed(seed int64) io.Reader { | ||
| if !isTestEnvironment() { | ||
| panic("NewRandReaderFromSeed is intended for testing only and cannot be used in production") | ||
| } | ||
| return rand.New(rand.NewSource(seed)) | ||
| } | ||
|
|
||
| // isTestEnvironment checks if the code is running in a test environment. | ||
| func isTestEnvironment() bool { | ||
| return os.Getenv("GO_ENV") == "test" | ||
| } |
Copilot is powered by AI and may make mistakes. Always verify output.
|
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.
Adds the Sui keystore to core
Requires