LetsEncrypt: Second Domain: Invaid character "_"

[langfingaz]

Thanks for the recent support to setup a second domain through ncp-config (#1025). By default if one runs the letsencrypt ncp-script, a _ is preselected for both domains.

I just changed the first _ to my desired domain and left the second domain as _. I ran into the following error:

Error message

Cannot issue for "_": Domain name contains an invalid character

Running letsencrypt
Saving debug log to /var/log/letsencrypt/letsencrypt.log
Plugins selected: Authenticator webroot, Installer None
Obtaining a new certificate
An unexpected error occurred:
Error creating new order :: Cannot issue for "_": Domain name contains an invalid character
Please see the logfiles in /var/log/letsencrypt for more details.
Done. Press any key...

System information

NextCloudPi diagnostics

NextCloudPi version  v1.19.1
NextCloudPi image    NextCloudPi_docker_07-20-19
distribution         Debian GNU/Linux 10 \n \l
automount            no
USB devices          none
datadir              /data/nextcloud/data
data in SD           yes
data filesystem      btrfs
data disk usage      108G/1.9T
rootfs usage         11G/117G
swapfile             none
dbdir                /data/database
Nextcloud check      ok
Nextcloud version    17.0.1.1
HTTPD service        up
PHP service          up
MariaDB service      up
Redis service        up
Postfix service      up
internet check       ok
port check 80        open
port check 443       open
IP                   ***REMOVED SENSITIVE VALUE***
gateway              ***REMOVED SENSITIVE VALUE***
interface            eth0
certificates         ***REMOVED SENSITIVE VALUE***
NAT loopback         no
uptime               7days

[langfingaz]

I guess the strings "" and "_" as domain should be ignored while iterating over the given domains in https://github.com/nextcloud/nextcloudpi/edit/master/bin/ncp/NETWORKING/letsencrypt.sh

[theCalcaholic]

I'd say "_" is just a bad default character. ncp supports suggestions for text fields, which, imo, should be used instead.

EDIT: In fact, there is no default value of "_" in the config for letsencrypt:
https://github.com/nextcloud/nextcloudpi/blob/master/etc/ncp-config.d/letsencrypt.cfg
Are you sure this was the case on a fresh installation?

[langfingaz]

@theCalcaholic I just pulled the latest x86 nextcloudpi docker container and tried to reproduce the behavior with the prefilled "_" on a fresh instance:

/usr/local/etc/ncp-config.d/nc-scan.cfg has "" as value for path one to three. But at first run of ncp-config there are three "_" prefilled as values. So if one just changes the first path to (for example) ncp and then starts the scan, three scans will be started of which two will fail for the unknown user "1_".

To only execute one scan (for ncp), the user has to remove the two "_" for path two and three - which I think is not really intuitive.

In the case of a files scan, this is not problematic, as all scans for valid users (ncp) will succeed.

After running ncp-update-ncp and selecting NETWORKING -> letsencrypt, the prefilled values are as follows:

DOMAIN	mycloud.ownyourbits.com
OTHER_DOMAIN	_
EMAIL	mycloud@ownyourbits.com
NOTIFYUSER	ncp

And this causes problems as described above. So I think a good solution would be to ignore "" and "_" as domains. So the user can just change the first domain (mycloud.ownyourbits.com) and does not need to worry about the second.

Greetings,
Daniel

[theCalcaholic]

Ah, thanks for your research.

I still think we should prevent that '_' to show up in the configuration in the first place (and not ignore it when applying). That's the cleaner solution, imo.

It looks like the terminal version of the letsencrypt app fills in the _ character in the form when being loaded. The web version does no such thing.
So I'd suggest to either change that behaviour (I assume it affects all text fields in all apps) or prevent that value to be added to the configuration (there might be cases where _ is a valid input).

What do you think?

EDIT: Here's where that character comes from: https://github.com/nextcloud/nextcloudpi/blob/master/etc/library.sh#L49
I think the reason, why a "_" character is being used here, is, that the dialog utility receives the configuration variables as a list of parameters (space separated string). If an empty parameter would be added, it would not be recognized.

EDIT2: This might be a possible solution: https://unix.stackexchange.com/questions/496843/passing-generated-empty-strings-as-command-line-arguments
But maybe you're right and it's too much effort (or possible fallacies) to solve this at the source...

@nachoparker what do you think about the issue?

[nachoparker]

Yes, this was a quick fix for the issue that we cannot use empty parameters with dialog.

I forget what the exact issue was, but there is definitely room for improvement here. I remember I was trying to release fast and this was a quick solution.

[nachoparker]

this has been fixed already