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

Replace deprecated clear text password with hash #389

Merged
merged 1 commit into from
Nov 1, 2024

Conversation

Hooverdan96
Copy link
Member

@Hooverdan96 Hooverdan96 commented Sep 23, 2024

Fixes #388 .

General information on project

  • The developer deprecated clear text password earlier this year with a hash password that can be generated based on a script they are providing.

Information on docker image

same author, however, the developer is recommending to use the image on Github instead of Docker hub:
ghcr.io/wg-easy/wg-easy

Changes made to update Rockon definition:

  • update image source
  • replace password parameter with password hash parameter
  • update description with instructions for hash.
  • fix indentation formatting (tab to space)
  • increased version number

Checklist

  • Passes JSONlint validation
  • Entry added to root.json in alphabetical order (for new rock-on only)
  • "description" object lists and links to the docker image used
  • "description" object provides information on the image's particularities (advantage over another existing rock-on for the same project, for instance)
  • "website" object links to project's main website

Testing

I have installed it using the same parameters as before, with the exception of the password now being entered as a hash value. Works well for me, connectivity from outside the network operates just as before it broke. The admin WebUI is accessible with the underlying password, so that transition works as well.

@Hooverdan96 Hooverdan96 added the needs review Test install, function, on / off behaviour, all links / info. label Sep 23, 2024
@Hooverdan96
Copy link
Member Author

Here are some testing screenshots for setup and config. One thing I noticed is that the share mapped representation is incorrect in the summary page during configuration, they're swapped, all other parameters are in the column where they're supposed to be. This might require another small addition to what @phillxnet has already done recently.

I am not showing how I got the password hash, it's pretty self-explanatory on the link provided in the Rockon description.

image

image

image
Here one can see the swapped parameter vs. mapped representation for the share:
image

image

image

image

If a previous instance was installed, the settings/keys will just show up and can continue to be used as before.

@phillxnet
Copy link
Member

@Hooverdan96 Just a quick note on:

One thing I noticed is that the share mapped representation is incorrect in the summary page during configuration, they're swapped, all other parameters are in the column where they're supposed to be. This might require another small addition to what @phillxnet has already done recently.

In a recent comment on related improvements in this area: rockstor/rockstor-core#2887 (comment)
I noticed a reversal of the Env entries, but Shares looked to be consistently mapped. Do you fancy creating an issue for this, given I failed to previously on noticing the Env reverse mapping. We likely have a few inadvertent reversals as I think a long time ago I only fixed one of a few of these types of reversals. So it would be good to established exactly where and what we are reversing. Above comment has an example of the Env switch that I failed to issue here on GitHub.

Copy link
Member

@phillxnet phillxnet left a comment

Choose a reason for hiding this comment

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

@Hooverdan96 Thanks again for your continued efforts in maintaining this Rock-on. As always much appreciated.

Re:

I am not showing how I got the password hash, it's pretty self-explanatory on the link provided in the Rockon description.

WireGuard-new-link-re-password-prep

I've only done a cursory pre-merge test here, to ensure the resulting Rock-on repo appears sane still. This looks to be the case. But I see a couple of minor link issues:

Am I doing it wrong :).

@Hooverdan96
Copy link
Member Author

Of course, you're not doing it wrong! I don't know what's happening. When I put my final html string into a validator, the links worked just fine. Of course I trusted that, and didn't retry this on a sample implementation. I'll address it.

Replace deprecated clear text password with hash
- adjust WG-Easy Image File
- replace password parameter with password hash parameter
- update description with instructions for hash. Adjust URL links.
- fix indentation formatting (tab to space)
@Hooverdan96
Copy link
Member Author

I also created new issue #2904 in the rockstor-core repo.

@phillxnet
Copy link
Member

@Hooverdan96 Appologies for the ages in getting back around to this PR:
Re:

Now works for me post your fix; and:

  • The GitHub repo link in the description has bugs:

I now get:

wireguard-pr389-follow-up

Where:

So following on from my last review and you having already proven the actual function re install etc, I'll merge and get this out to PRODUCTION. Again my apologies here on the delay revisiting this Rock-on maintenance PR. And thanks again for all your efforts here and essentially everywhere else in the project. As always much appreciated.

@phillxnet phillxnet merged commit efa60d0 into rockstor:master Nov 1, 2024
@phillxnet
Copy link
Member

PR product PRODUCTION published.

@Hooverdan96 Hooverdan96 deleted the 388_update_wgeasy branch November 3, 2024 03:56
@Hooverdan96 Hooverdan96 removed the needs review Test install, function, on / off behaviour, all links / info. label Nov 3, 2024
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.

Update Wireguard Rockon WGEasy with password hash option
2 participants