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

DietPi-DDNS | Manage domains for your dynamic IP #4229

Merged
merged 30 commits into from
Apr 20, 2021
Merged

DietPi-DDNS | Manage domains for your dynamic IP #4229

merged 30 commits into from
Apr 20, 2021

Conversation

ravenclaw900
Copy link
Collaborator

@ravenclaw900 ravenclaw900 commented Mar 28, 2021

Status: Review

References:

ToDo:

  • Replace inline cURL with dedicated script to avoid having plain text passwords/tokens in journalctl logs
  • Replace password input box with password box (which replaces typed characters with *)? Enhances security but especially with for those long tokens it's a comfort downside. ... Ah, it's the very same with UNIX passwords, let's be consequent here. Via SSH copy&paste is always possible anyway.

+ DietPi-Software | Add DuckDNS
+ DietPi-Config | Add DuckDNS to avalible network options
@ravenclaw900 ravenclaw900 added this to the v7.1 milestone Mar 28, 2021
@ravenclaw900 ravenclaw900 self-assigned this Mar 28, 2021
@MichaIng
Copy link
Owner

Ay, before you invest too much time into the software install part of this, I'm working on a DietPi-DDNS script with generic DDNS provider support for those which can be updated via simple request, including support to enter a custom provider update URL. Similarly to DietPi-VPN, there is not much point in having this as a software install option as the script will be always present.

I'll incorporate/merge it with our DuckDNS specific work, especially the "OK" return code to check for success is a good idea.

@ravenclaw900 ravenclaw900 deleted the duckdns branch March 28, 2021 20:18
@MichaIng MichaIng restored the duckdns branch March 28, 2021 20:18
@MichaIng MichaIng reopened this Mar 28, 2021
@MichaIng
Copy link
Owner

I restored the branch, and re-opened PR to compare/incorporate the work.

+ DietPi-Software | Typo
@ravenclaw900 ravenclaw900 changed the title DietPi-Software | DuckDNS DietPi-DDNS Apr 11, 2021
+ Update DietPi-DDNS branch
+ DietPi-DDNS | Add DietPi-DDNS script
@ravenclaw900 ravenclaw900 marked this pull request as draft April 13, 2021 00:28
+ DietPi-Config | (Hopefully) CodeFactor Fixes
@ravenclaw900
Copy link
Collaborator Author

It's ready for you, @MichaIng.

dietpi/dietpi-config Outdated Show resolved Hide resolved
@MichaIng MichaIng linked an issue Apr 13, 2021 that may be closed by this pull request
@ravenclaw900 ravenclaw900 removed their assignment Apr 15, 2021
+ DietPi-DDNS | Create new tool to setup background jobs to regularly update dynamic IPs against DDNS providers
+ DietPi-Config | Network Options: Misc: Replace No-IP and DuckDNS menu entries with "Dynamic DNS", to start the new dietpi-ddns script
+ DietPi-Software | Remove DuckDNS and No-IP install options, now integrated into new dietpi-ddns script
+ DietPi-Launcher | Add new DietPi-DDNS
+ DietPi-Survey_report | Satisfy shellcheck
+ DietPi-DDNS | Remove obsolete No-IP client, when found, before setting up Cron job
@MichaIng MichaIng marked this pull request as ready for review April 20, 2021 12:30
@MichaIng MichaIng requested a review from Joulinar April 20, 2021 12:31
MichaIng and others added 4 commits April 20, 2021 14:51
+ DietPi-DDNS | Use separate update script which holds the cURL call with credentials and has strict read-only permissions. This assures that credentials are never shown as part of Cron journalctl logs.
+ DietPi-DDNS | Use password box for password/token input. While it makes the input harder, it enhances security by assuring that those are never visible on screen.
+ DietPi-DDNS | Use `noip.com` instead of `no-ip.com`
+ DietPi-Patches | Remove obsolete DietPi-NordVPN and No-IP install states
@Joulinar
Copy link
Collaborator

Question: what happen with user who have NoIP installed as of today. Are they able to continue to mange their NoIP account? Or do they need to re-setup thinks using dietpi-ddns or are we going to provide a migration path? I mean we are going to remove the install option, means people are not able to uninstall via dietpi-software anymore. Probably I missed something as I was checking the code just briefly

+ DietPi-Patches | Fix install state removal, especially on first boot where the states file does not exist yet
@MichaIng
Copy link
Owner

AFAIK /usr/local/etc/no-ip.conf contains the password in a hashed form, so there is no way for us to do an automated migration. The No-IP client stays installed and functional. Only when using dietpi-ddns to setup and finally apply a new DDNS Cron job, and the update test succeeds, the No-IP client + service is removed. The setup would need to be done manually indeed.

What we should add is an interactive prompt to inform and encourage users with No-IP installed to setup No-IP DDNS via our new script, which is lighter and uses up-to-date protocols etc and have the old client automatically removed as very last step of the setup. Most importantly it cannot happen that the client is removed but no Cron job installed.

+ DietPi-Patches | Add info about No-IP client migration
@MichaIng
Copy link
Owner

That should make it clear: 5c83d4e

@Joulinar
Copy link
Collaborator

Yeah that should be fine 😉

MichaIng
MichaIng previously approved these changes Apr 20, 2021
@MichaIng MichaIng requested review from fpetru and StephanStS April 20, 2021 17:19
+ DietPi-Patches | Syntax
MichaIng
MichaIng previously approved these changes Apr 20, 2021
+ DietPi-DDNS | Do not use sudo to run DDNS update test, to avoid plain text passwords/tokens in journal sudo log. The chance that the unprivileged user causes an issue is pretty low. That would require a strict custom setup/firewall to permit network access only for specific users, which would break various other common implementations as well. So it's okay to count on every user being able to perform network requests.
+ DietPi-DDNS | Current settings now need to be read from the new script as well
+ DietPi-DDNS | Fix obtaining time span
+ DietPi-DDNS | Workaround shellcheck false positive
dietpi/dietpi-ddns Outdated Show resolved Hide resolved
@MichaIng MichaIng merged commit 6ed100f into dev Apr 20, 2021
@MichaIng MichaIng deleted the duckdns branch April 20, 2021 20:20
if [[ $PROVIDER == 'DuckDNS' ]]
then
url="https://www.duckdns.org/update?domains=$DOMAINS&token=$PASSWORD"
unset -v USERNAME PASSWORD
Copy link
Collaborator Author

@ravenclaw900 ravenclaw900 Apr 20, 2021

Choose a reason for hiding this comment

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

unset -v USERNAME PASSWORD

Any reason for this line? Just asking because it makes the 'Token' field disappear.

Copy link
Owner

Choose a reason for hiding this comment

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

Ah, I didn't think about the values shown in the menu afterwards. Currently this is required to not have HTTP authentication used.

But then we need to a separate variable for this:

local http_auth=1
...
if [[ $PROVIDER == 'DuckDNS' ]]
then
	unset -v http_auth
...
...${http_auth:+ -u '$USERNAME:$PASSWORD'}...

Will implement this now.

Copy link
Owner

Choose a reason for hiding this comment

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

Done: 5ce84c7

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DietPi-Software | DuckDNS
3 participants