[pve-devel] [PATCH manager] ui: storage: esxi: check 'skip certificate verification' by default

[pve-devel] [PATCH manager] ui: storage: esxi: check 'skip certificate verification' by default

[Dominik Csapak]

needing one less step when adding the storage, assuming most esxi
certificates are self-signed.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/manager6/storage/ESXIEdit.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/www/manager6/storage/ESXIEdit.js b/www/manager6/storage/ESXIEdit.js
index b105b1e3..d11232a0 100644
--- a/www/manager6/storage/ESXIEdit.js
+++ b/www/manager6/storage/ESXIEdit.js
@@ -48,7 +48,7 @@ Ext.define('PVE.storage.ESXIInputPanel', {
 		xtype: 'proxmoxcheckbox',
 		name: 'skip-cert-verification',
 		fieldLabel: gettext('Skip Certificate Verification'),
-		value: false,
+		value: true,
 		uncheckedValue: 0,
 		defaultValue: 0,
 		deleteDefaultValue: !me.isCreate,
-- 
2.39.2

Re: [pve-devel] [PATCH manager] ui: storage: esxi: check 'skip certificate verification' by default

[Thomas Lamprecht]

On 20/03/2024 16:39, Dominik Csapak wrote:
> needing one less step when adding the storage, assuming most esxi
> certificates are self-signed.

Well this makes it insecure by default though? Which is not something
I'd just not mention in such a commit message...

As that was the original reason I ticked it in the first place
when pondering between security and convenience...

If we do this I'd rather rename it to "Check Certificate" and have
that unticked.

Even better would be to be able to pass a finger-print, which was our
first idea, but Wolfgang found that the esxi python wrapper is to
enterprisy to hook into basic TLS validation, and he also rejected
proxying..

Re: [pve-devel] [PATCH manager] ui: storage: esxi: check 'skip certificate verification' by default

[Dominik Csapak]

On 3/21/24 18:07, Thomas Lamprecht wrote:
> On 20/03/2024 16:39, Dominik Csapak wrote:
>> needing one less step when adding the storage, assuming most esxi
>> certificates are self-signed.
> 
> Well this makes it insecure by default though? Which is not something
> I'd just not mention in such a commit message...

imho it is very obvious what it does from the commit subject?

'skipping the certificate verification'

?
but ok, i can add a sentence more in the description..

> 
> As that was the original reason I ticked it in the first place
> when pondering between security and convenience...
>

the thought here was that users that make the effort of giving
their esxi instances valid certificates, can simply uncheck the checkbox?

and i guess many of the users won't bother doing that for the
esxi instances? (e.g. vcenter does not make that distinction, all
it does is ask for hostname/ip + password, and cert management seems
to be non-trivial)


> If we do this I'd rather rename it to "Check Certificate" and have
> that unticked.

ok makes sense, i'd name it 'verify certificate' though to be in line
with our realm/metric server wording

also should this be only in the frontend, or do we want to reverse
the api/config option as well?

> 
> Even better would be to be able to pass a finger-print, which was our
> first idea, but Wolfgang found that the esxi python wrapper is to
> enterprisy to hook into basic TLS validation, and he also rejected
> proxying..

Re: [pve-devel] [PATCH manager] ui: storage: esxi: check 'skip certificate verification' by default

[Thomas Lamprecht]

On 22/03/2024 08:29, Dominik Csapak wrote:
> On 3/21/24 18:07, Thomas Lamprecht wrote:
>> On 20/03/2024 16:39, Dominik Csapak wrote:
>>> needing one less step when adding the storage, assuming most esxi
>>> certificates are self-signed.
>>
>> Well this makes it insecure by default though? Which is not something
>> I'd just not mention in such a commit message...
> 
> imho it is very obvious what it does from the commit subject?
> 
> 'skipping the certificate verification'
> 
> ?
> but ok, i can add a sentence more in the description..

as always, the reasoning and why's count a bit more in such
cases – making the default insecure is something where a giving
a reason for why this is OK is rather a must...

>> As that was the original reason I ticked it in the first place
>> when pondering between security and convenience...
>>
> 
> the thought here was that users that make the effort of giving
> their esxi instances valid certificates, can simply uncheck the checkbox?

So can the others?! And that would be pretty obvious if the error
message gets passed through like I requested already off list over
a week ago..

As again... this is making the connection completely insecure, and
users with valid certs won't be notified of that fact "so simply check"
it is really not something a user can be aware of if it "works" without
enabling basic security..
 > and i guess many of the users won't bother doing that for the
> esxi instances? (e.g. vcenter does not make that distinction, all
> it does is ask for hostname/ip + password, and cert management seems
> to be non-trivial)

again, not an argument for why we should make it less secure.

>> If we do this I'd rather rename it to "Check Certificate" and have
>> that unticked.
> 
> ok makes sense, i'd name it 'verify certificate' though to be in line
> with our realm/metric server wording
> 
> also should this be only in the frontend, or do we want to reverse
> the api/config option as well?

No, I'd keep it off there, so having the user pass the --skip- option
makes it more clear.

But tbh. I probably won't apply this in any way, as mentioned there
are other ways to actually improve on this, the error message would
be a relatively easy first one.

Re: [pve-devel] [PATCH manager] ui: storage: esxi: check 'skip certificate verification' by default

[Dominik Csapak]

On 3/22/24 09:46, Thomas Lamprecht wrote:
> On 22/03/2024 08:29, Dominik Csapak wrote:
>> On 3/21/24 18:07, Thomas Lamprecht wrote:
>>> On 20/03/2024 16:39, Dominik Csapak wrote:
>>>> needing one less step when adding the storage, assuming most esxi
>>>> certificates are self-signed.
>>>
>>> Well this makes it insecure by default though? Which is not something
>>> I'd just not mention in such a commit message...
>>
>> imho it is very obvious what it does from the commit subject?
>>
>> 'skipping the certificate verification'
>>
>> ?
>> but ok, i can add a sentence more in the description..
> 
> as always, the reasoning and why's count a bit more in such
> cases – making the default insecure is something where a giving
> a reason for why this is OK is rather a must...
> 

ok sorry i think i misunderstood you, yes, the reason should be clear
but i thought i did that by saying 'most esxi certs are self-signed'


>>> As that was the original reason I ticked it in the first place
>>> when pondering between security and convenience...
>>>
>>
>> the thought here was that users that make the effort of giving
>> their esxi instances valid certificates, can simply uncheck the checkbox?
> 
> So can the others?! And that would be pretty obvious if the error
> message gets passed through like I requested already off list over
> a week ago..
> 
> As again... this is making the connection completely insecure, and
> users with valid certs won't be notified of that fact "so simply check"
> it is really not something a user can be aware of if it "works" without
> enabling basic security..
>   > and i guess many of the users won't bother doing that for the
>> esxi instances? (e.g. vcenter does not make that distinction, all
>> it does is ask for hostname/ip + password, and cert management seems
>> to be non-trivial)
> 
> again, not an argument for why we should make it less secure.
> 
>>> If we do this I'd rather rename it to "Check Certificate" and have
>>> that unticked.
>>
>> ok makes sense, i'd name it 'verify certificate' though to be in line
>> with our realm/metric server wording
>>
>> also should this be only in the frontend, or do we want to reverse
>> the api/config option as well?
> 
> No, I'd keep it off there, so having the user pass the --skip- option
> makes it more clear.
> 
> But tbh. I probably won't apply this in any way, as mentioned there
> are other ways to actually improve on this, the error message would
> be a relatively easy first one.

ok fine with me