luci-app-acme: support --cert-profile option

[ngehrsitz]

This makes the new --cert-profile option from openwrt/packages#28237 available using Luci.

• This PR is not from my main or master branch 💩, but a separate branch ✅
• Each commit has a valid ✒️ Signed-off-by: <my@email.address> row (via git commit --signoff)
• Each commit and PR title has a valid 📝 <package name>: title first line subject for packages
• Incremented 🆙 any PKG_VERSION in the Makefile
• Tested on: (architecture, openwrt version, browser) ✅
• ( Preferred ) Mention: @ the original code author for feedback
• ( Preferred ) Screenshot or mp4 of changes:
• ( Optional ) Closes: e.g. openwrt/luci#issue-number
• ( Optional ) Depends on: e.g. openwrt/packages#pr-number in sister repo
• Description: (describe the changes proposed in this PR)

[ngehrsitz]

@tohojo There was no version to increment in the Makefile, is that not necessary here?

[ngehrsitz]

Is there any way to control the order?

Ideally this would be below the URL, but it doesn´t appear to be the order these are defined in?

[systemcrash]

order? It doesn't appear.

[ngehrsitz]

This screenshot was captured without the change. My question was in general if you can control the order since the order here does not match the one in the code.

[systemcrash]

Order of what? The order the elements appear in? Code order = appearance order

[ngehrsitz]

In the code it is:

1. Keytype
   

   luci/applications/luci-app-acme/htdocs/luci-static/resources/view/acme/acme.js

   Lines 245 to 275 in 6efab68

   	o = s.taboption('advanced', form.ListValue, 'key_type', _('Key type'),
   	_('Key size (and type) for the generated certificate.')
   	);
   	o.value('rsa2048', _('RSA 2048 bits'));
   	o.value('rsa3072', _('RSA 3072 bits'));
   	o.value('rsa4096', _('RSA 4096 bits'));
   	o.value('ec256', _('ECC 256 bits'));
   	o.value('ec384', _('ECC 384 bits'));
   	o.rmempty = false;
   	o.optional = true;
   	o.modalonly = true;
   	o.cfgvalue = function(section_id) {
   	let keylength = uci.get('acme', section_id, 'keylength');
   	if (keylength) {
   	// migrate the old keylength to a new keytype
   	switch (keylength) {
   	case '2048': return 'rsa2048';
   	case '3072': return 'rsa3072';
   	case '4096': return 'rsa4096';
   	case 'ec-256': return 'ec256';
   	case 'ec-384': return 'ec384';
   	default: return ''; // bad value
   	}
   	}
   	return this.super('cfgvalue', arguments);
   	};
   	o.write = function(section_id, value) {
   	// remove old keylength
   	uci.unset('acme', section_id, 'keylength');
   	uci.set('acme', section_id, 'key_type', value);
   	};

2. URL
   

   luci/applications/luci-app-acme/htdocs/luci-static/resources/view/acme/acme.js

   Lines 277 to 284 in 6efab68

   	o = s.taboption('advanced', form.Value, "acme_server", _("ACME server URL"),
   	_('Use a custom CA instead of Let\'s Encrypt.') + ' ' + _('Custom ACME server directory URL.') + '<br />' +
   	'<a href="https://github.com/acmesh-official/acme.sh/wiki/Server" target="_blank">' + _('See more') + '</a>' + '<br />'
   	+ _('Default') + ' <code>letsencrypt</code>'
   	);
   	o.placeholder = "https://api.buypass.com/acme/directory";
   	o.optional = true;
   	o.modalonly = true;

3. Staging
   

   luci/applications/luci-app-acme/htdocs/luci-static/resources/view/acme/acme.js

   Lines 286 to 294 in 6efab68

   	o = s.taboption('advanced', form.Flag, 'staging', _('Use staging server'),
   	_(
   	'Get certificate from the Letsencrypt staging server ' +
   	'(use for testing; the certificate won\'t be valid).'
   	)
   	);
   	o.depends('acme_server', '');
   	o.optional = true;
   	o.modalonly = true;

4. Days
   

   luci/applications/luci-app-acme/htdocs/luci-static/resources/view/acme/acme.js

   Lines 296 to 300 in 6efab68

   	o = s.taboption('advanced', form.Value, 'days', _('Days until renewal'));
   	o.optional = true;
   	o.placeholder = 'acme.sh default (60 days)';
   	o.datatype = 'uinteger';
   	o.modalonly = true;

But in the Screenshot it´s

1. Staging
2. Keytype
3. URL
4. Days

Thus not the order it is defined in...

[github-actions]

Warning

Some formality checks failed.

Consider (re)reading submissions guidelines.

Failed checks

Issues marked with an ❌ are failing checks.

Commit c186253

• 🔶 Commit body line 1 is longer than 75 characters (is 111):
  This makes the new --cert-profile option from acme-acmesh: support --cert-profile option packages#28237 available in Luci.

For more details, see the full job log.

Something broken? Consider providing feedback.

[systemcrash]

@stokito any thoughts?

[stokito]

Thank you for the PR, it looks good. Maybe only it would be better to add a description with a known profile names and with a link to LE profiles page. Also we may call the acme.sh --list-profiles and show them in the drop down, but that would be over-complication.

I think we should merge it but...
For now, as far I understood the only reason to use them is to issue a cert for an IP address with the shortlived profile. And that's it: other profiles simply makes no sense at all. So they just created a new abstraction layer for nothing, if only they have a plan to use this latter to reduce compatibility, increase fragmentation, complicate understanding, make it prone for errors etc. I personally don't have any trust in the LE as an organization and I don't think that we, as users should follow them. Even in the docs they mentioned that the profiles was an idea of the "CA/Browser Forum" which is literally a syndicate of monopolists (DigiCert, Google) against whom initially the LE was formed. Instead we should move in a direction of decentralized web and stop to trust to the Certificate Authorities because they are not Authorities if they obey other Authorities IRL.

The IP address cert is always was a questionable thing. In fact, even if you don't have a domain... well okay, get one for free from DDNS. But this is another tool to use and if we can avoid it, then yes the IP cert would be better. And still, a user must see some domain for a safety reason. Otherwise it will be easier to make a fishing attack. Not sure if browsers do allow the IP certs and maybe they are only for tools like wget/curl but even in this case why would you use them with a router.

Ok, back to the PR. Can we get an IP cert without a profile (maybe just specify shorter days)? If yes, then lets just forget about profiles and make a single checkbox "IP addr cert". We also have a strict check for a domain name on the UI so a user anyway won't be able to issue such a cert.

[ngehrsitz]

My use case for shortlived certs is that I want to notice any issues with my certificate automation as fast as possible. With 90 day certificates it can take a long time until I actually notice that it´s broken. With shortlived certs I can just monitor if the Certificate Transparency notifications come in every week.
Linking to LE does not seem appropriate to me since we shouldn´t assume that LE is the users CA.
We can´t simplify this to a checkbox or dropdown approach unless we implement a dynamic resolution with acme.sh --list-profiles since other CAs will have different profile names.

[systemcrash]

What does acme.sh --list-profiles output?

[stokito]

https://github.com/acmesh-official/acme.sh/wiki/Profile-selection

@ngehrsitz please turn the PR from Draft so it can be merged. From code perspective all looks good, the feature looks like needed. You may ignore my ranting

[systemcrash]

Merged. Thanks @ngehrsitz