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

fix warning and improve encrypt #60

Closed
wants to merge 7 commits into from
Closed

fix warning and improve encrypt #60

wants to merge 7 commits into from

Conversation

ghost
Copy link

@ghost ghost commented May 13, 2019

fix #56
fix #59

  1. fix waring like above
  2. import default password 30 to 32,as 2 power 5
  3. -md5 change to sha1 to improve encrypt(sha1 is more stronger than md5)

maybe it is not incompatible with last versions

I have simply test it, it works well

Copy link
Owner

@elasticdog elasticdog left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned in #55 (comment), I have a bit of investigation to do on how long the -iter option has been available before I'd be comfortable in making this change. It would definitely be a backward incompatible thing, so I also need to understand the migration workflow for existing users. I'll circle back to this change though, and appreciate you taking the time to open up this PR.

@@ -0,0 +1,3 @@
.idea/**

transcrypt.iml
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These types of ignore patterns that aren't really project-specific should more naturally fall under your global user ~/.gitignore file:

git config --global core.excludesfile '~/.gitignore'

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for taking your time,this project was really simple and great,I will remember to exclude the project-specific file

@@ -292,8 +292,8 @@ save_helper_scripts() {
else
cipher=$(git config --get --local transcrypt.cipher)
password=$(git config --get --local transcrypt.password)
salt=$(openssl dgst -hmac "${filename}:${password}" -sha256 "$filename" | tail -c 16)
ENC_PASS=$password openssl enc -$cipher -md MD5 -pass env:ENC_PASS -e -a -S "$salt" -in "$tempfile"
salt=$(openssl dgst -hmac "${filename}:${password}" -sha256 "$filename" | tail -c 17)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went ahead and merged in #57 to fix the issue in #56 in a cross-platform manner, so this change can be reverted if you merge in the latest from the master branch.

@ghost
Copy link
Author

ghost commented May 30, 2019

backward incompatible is very import ,but '-iter' option openssl is really recommended,so I suggest that in new project , write transcrypt version to git,then if the transcrypt version is exist and correct ,then use new function,else use the old function.
security is very import ,so why users use this project, but also hack technology is developing,so I think it is necessarily to improve the encrypt strength to project their projects as they are care about that and why use this project to encrypt.

@elasticdog
Copy link
Owner

Definitely agree that we should modernize the algorithm to increase security and figure out the proper migration path, whether that means adding version checking into the script or something else...that said, it's not a decision I want to rush.

@ghost
Copy link
Author

ghost commented May 31, 2019

Definitely agree that we should modernize the algorithm to increase security and figure out the proper migration path, whether that means adding version checking into the script or something else...that said, it's not a decision I want to rush.

yeah,couldn't agree more,no need to rush

@ghost ghost closed this May 31, 2019
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants