all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Dominik Csapak <d.csapak@proxmox.com>
To: Lukas Wagner <l.wagner@proxmox.com>,
	Christoph Heiss <c.heiss@proxmox.com>,
	pdm-devel@lists.proxmox.com
Subject: Re: [PATCH proxmox/yew-pwt/datacenter-manager/installer v3 00/38] add auto-installer integration
Date: Mon, 20 Apr 2026 14:09:25 +0200	[thread overview]
Message-ID: <c3b2cdce-08c2-42c5-a6c3-61a40e4a03f7@proxmox.com> (raw)
In-Reply-To: <DHXXVZQKYYPV.2VP5WLJA05FOM@proxmox.com>



On 4/20/26 1:17 PM, Lukas Wagner wrote:
> On Mon Apr 20, 2026 at 12:08 PM CEST, Dominik Csapak wrote:
>>
>> On 4/20/26 11:52 AM, Lukas Wagner wrote:
>>> On Fri Apr 17, 2026 at 1:53 PM CEST, Dominik Csapak wrote:
>>>>>>> Check out my response to v2, which is (I think) the main reason why it
>>>>>>> is implemented like it is right now:
>>>>>>>
>>>>>>> https://lore.proxmox.com/all/DETMUXY1Q877.32G593TWC52WW@proxmox.com/T/#u
>>>>>>>
>>>>>>
>>>>>> ok i see, but i have some counter arguments to that, for brevity, here
>>>>>> is part of your response from that mail:
>>>>>>
>>>>>> ---
>>>>>> I think this is dangerous.
>>>>>>
>>>>>> While the answer-file does not leak any passwords (seems like the root
>>>>>> password is hashed), it still contains semi-sensitive data (email address,
>>>>>> SSH key, FQDN, etc.). Also, since each POST creates a new entry in the
>>>>>> installations JSON file, an unauthenticated user could abuse this to
>>>>>> make the PDM system run out of disk space (or simply disturb operations
>>>>>> by creating bogus entries).
>>>>>> ---
>>>>>>
>>>>>>
>>>>>> We already had the requirement for a http endpoint to be unauthenticated
>>>>>> so for a secured and protected network, this should be a non issue
>>>>>
>>>>> Yes, but I think it's a big difference if we integrate something like
>>>>> this into PDM natively. There will always be users who, against our
>>>>> recommendations, expose the PDM web interface directly to the internet
>>>>> or other untrusted network environments. I would very much argue
>>>>> against returning any kind of (semi)-sensitive information from the API
>>>>> without proper access controls, even if it is opt-in.
>>>>
>>>> why is it different? previously we *needed* the admin to expose
>>>> it unauthenticated if he wanted to use, it, i proposed
>>>> making authentication optional, it's still a win from a security pov?
>>>
>>> I'd argue it is different in the sense that PDM might be more widely
>>> exposed in the network than a homebrew answer-file responder.
>>>
>>> By including the feature in PDM, we advertise this method of automated
>>> installations, and due to the sheer size of our userbase, there will
>>> undoubtedly be leakage at some point if we include the unauth'd version.
>>> If we can make this impossible with a design decision now, even at a
>>> slight UX penalty, I'm all for it.
>>>
>>>>
>>>> IMO this boils down what we choose, neither options is more correct here.
>>>>
>>>> do we trust the admin to properly configure his environment?
>>>> if yes, then we can leave it up to their choice if they want to protect
>>>> these endpoints with a tokenA
>>>
>>> If this was any other feature, would we even discuss returning sensitive
>>> information from the API without authentication for an usability
>>> improvement, even as an opt-in? I highly doubt it.
>>>
>>> To me, the fact that answer-file servers were completely unauth'd before
>>> is not a very good reason why we should handle it the same way in PDM.
>>>
>>> I think I'd rather try to make it as simple as possible to use the
>>> token-based solution (as you mentioned, the UX could be improved),
>>> and then only if there are enough requests for the less secure variant
>>> in the forum or BZ, consider making the unauth'd variant an (advanced)
>>> option.
>>>
>>
>> After reading your response and thinking it over, i agree that it's
>> better to require full auth now, as long as the UX is better than it is
>> currently.
> 
> I'm glad we could find consensus on this. Thank you for the discussion!
> 
>>
>> Basically I'd try to reuse regular auth token, and maybe make
>> the cli command copyable (not sure how to handle the token
>> secrets here in a good way for the gui, though)
> 
> Apart from the slightly increased amount of code needed to implement the
> feature, do you see any big draw-back from using distinct tokens for
> this feature?
> 
> I think I wrote it in my original response for v2, the main reason why I
> suggested using distinct tokens is to also eliminate one source of user
> error. If we use regular tokens here, there will surely be a small share
> of users who don't want to deal with ACLs and just use a full
> admin-token for the ISO. Most users probably don't consider an ISO to be
> worth protecting, as embedding secrets is indeed a very rare thing to
> do. The ISO might end up on NAS shares, might be included in backups,
> etc., without people realizing that it contains a key to all of their
> infra.

I think the reverse is also true. Having two types of tokens is rather
confusing, and users may wonder why this token exists in this one
list but not in the other. Or worse, confusing them and their
secrets, e.g. clicking 'regenerate' token and overwriting the
wrong token in their application/client/etc.

We do already have the ACL system that users and admins should use.
Introducing a second 'mini ACL' system just so they can avoid the
main one sounds wrong?

As for the permissions. We could introduce a new privilege that
is not included in the Admin role, but needs a separate one.
This way we'd force users to choose an explicit privilege only
associated with the installer answers.
(also we could maybe warn in the UI if more privileges than needed are
given..)

> 
> Also, if we add support for generating ISOs in PDM, or display the
> command needed to generate the ISO, we would violate our 'rule' of never
> returning tokens from the API, after they have been initially created,
> that is.

that is indeed what i meant earlier with the token secrets.
And I don't think we could easily read out the token anyway
as it is hashed..

What we could try to do to improve UX is to create the token
on answer file generation (so each answer file has their own
token) and show the token + cli command at once.

If we'd want to create the iso, we'd have to create it in
the same step, as we don't have access to the token anymore later.

> 
>  From my point of view, both points can be solved by using distinct
> tokens. The first point is solved by making the new type of token
> unusable in any other context by design. For the second issue, I think
> due to the fact that the token is *only* usable in this context, we
> could afford to return it from the API or as a downloadable ISO,
> assuming the user has high-enough permissions.
> 
> What do you think?
> 
> Regarding the 'custom token' itself, one thing that we could do is to
> make the token a regular parameter on the POST call, instead of using
> the 'Authorization' header. This would avoid API users from being
> confused about why there is one API route that uses a *different* token
> for no clearly visible reason.
> 
> Ultimately, I don't think anybody would call this endpoint except our
> own installer, but it would still make things a tiny bit more clear, I
> think.

don't get me wrong, i'm not completely opposed to having a separate
token, but i probably would frame/name it so much different
that users can't confuse the two (e.g. maybe 'answer-file-secret' ?
or something like that)







  reply	other threads:[~2026-04-20 12:09 UTC|newest]

Thread overview: 61+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-03 16:53 Christoph Heiss
2026-04-03 16:53 ` [PATCH proxmox v3 01/38] api-macro: allow $ in identifier name Christoph Heiss
2026-04-03 16:53 ` [PATCH proxmox v3 02/38] schema: oneOf: allow single string variant Christoph Heiss
2026-04-03 16:53 ` [PATCH proxmox v3 03/38] schema: implement UpdaterType for HashMap and BTreeMap Christoph Heiss
2026-04-03 16:53 ` [PATCH proxmox v3 04/38] network-types: move `Fqdn` type from proxmox-installer-common Christoph Heiss
2026-04-03 16:53 ` [PATCH proxmox v3 05/38] network-types: implement api type for Fqdn Christoph Heiss
2026-04-03 16:53 ` [PATCH proxmox v3 06/38] network-types: add api wrapper type for std::net::IpAddr Christoph Heiss
2026-04-03 16:53 ` [PATCH proxmox v3 07/38] network-types: cidr: implement generic `IpAddr::new` constructor Christoph Heiss
2026-04-03 16:53 ` [PATCH proxmox v3 08/38] network-types: fqdn: implement standard library Error for Fqdn Christoph Heiss
2026-04-03 16:53 ` [PATCH proxmox v3 09/38] node-status: make KernelVersionInformation Clone + PartialEq Christoph Heiss
2026-04-03 16:53 ` [PATCH proxmox v3 10/38] installer-types: add common types used by the installer Christoph Heiss
2026-04-03 16:53 ` [PATCH proxmox v3 11/38] installer-types: add types used by the auto-installer Christoph Heiss
2026-04-03 16:53 ` [PATCH proxmox v3 12/38] installer-types: implement api type for all externally-used types Christoph Heiss
2026-04-03 16:53 ` [PATCH yew-widget-toolkit v3 13/38] widget: kvlist: add widget for user-modifiable data tables Christoph Heiss
2026-04-16 12:23   ` Dominik Csapak
2026-04-16 14:18     ` Christoph Heiss
2026-04-03 16:53 ` [PATCH datacenter-manager v3 14/38] api-types, cli: use ReturnType::new() instead of constructing it manually Christoph Heiss
2026-04-03 16:53 ` [PATCH datacenter-manager v3 15/38] api-types: add api types for auto-installer integration Christoph Heiss
2026-04-03 16:53 ` [PATCH datacenter-manager v3 16/38] config: add auto-installer configuration module Christoph Heiss
2026-04-03 16:53 ` [PATCH datacenter-manager v3 17/38] acl: wire up new /system/auto-installation acl path Christoph Heiss
2026-04-03 16:53 ` [PATCH datacenter-manager v3 18/38] server: api: add auto-installer integration module Christoph Heiss
2026-04-03 16:53 ` [PATCH datacenter-manager v3 19/38] server: api: auto-installer: add access token management endpoints Christoph Heiss
2026-04-03 16:53 ` [PATCH datacenter-manager v3 20/38] client: add bindings for auto-installer endpoints Christoph Heiss
2026-04-03 16:53 ` [PATCH datacenter-manager v3 21/38] ui: auto-installer: add installations overview panel Christoph Heiss
2026-04-03 16:53 ` [PATCH datacenter-manager v3 22/38] ui: auto-installer: add prepared answer configuration panel Christoph Heiss
2026-04-03 16:53 ` [PATCH datacenter-manager v3 23/38] ui: auto-installer: add access token " Christoph Heiss
2026-04-03 16:53 ` [PATCH datacenter-manager v3 24/38] docs: add documentation for auto-installer integration Christoph Heiss
2026-04-03 16:53 ` [PATCH installer v3 25/38] install: iso env: use JSON boolean literals for product config Christoph Heiss
2026-04-03 16:53 ` [PATCH installer v3 26/38] common: http: allow passing custom headers to post() Christoph Heiss
2026-04-14 12:13   ` Lukas Wagner
2026-04-15  8:53     ` Christoph Heiss
2026-04-03 16:53 ` [PATCH installer v3 27/38] common: options: move regex construction out of loop Christoph Heiss
2026-04-03 16:54 ` [PATCH installer v3 28/38] assistant: support adding an authorization token for HTTP-based answers Christoph Heiss
2026-04-14 12:13   ` Lukas Wagner
2026-04-03 16:54 ` [PATCH installer v3 29/38] tree-wide: used moved `Fqdn` type to proxmox-network-types Christoph Heiss
2026-04-03 16:54 ` [PATCH installer v3 30/38] tree-wide: use `Cidr` type from proxmox-network-types Christoph Heiss
2026-04-03 16:54 ` [PATCH installer v3 31/38] tree-wide: switch to filesystem types from proxmox-installer-types Christoph Heiss
2026-04-03 16:54 ` [PATCH installer v3 32/38] post-hook: switch to types in proxmox-installer-types Christoph Heiss
2026-04-03 16:54 ` [PATCH installer v3 33/38] auto: sysinfo: switch to types from proxmox-installer-types Christoph Heiss
2026-04-03 16:54 ` [PATCH installer v3 34/38] fetch-answer: " Christoph Heiss
2026-04-03 16:54 ` [PATCH installer v3 35/38] fetch-answer: http: prefer json over toml for answer format Christoph Heiss
2026-04-14 12:13   ` Lukas Wagner
2026-04-03 16:54 ` [PATCH installer v3 36/38] fetch-answer: send auto-installer HTTP authorization token if set Christoph Heiss
2026-04-14 12:13   ` Lukas Wagner
2026-04-14 12:14   ` Lukas Wagner
2026-04-03 16:54 ` [PATCH installer v3 37/38] tree-wide: switch out `Answer` -> `AutoInstallerConfig` types Christoph Heiss
2026-04-03 16:54 ` [PATCH installer v3 38/38] auto: drop now-dead answer file definitions Christoph Heiss
2026-04-14 12:16 ` [PATCH proxmox/yew-pwt/datacenter-manager/installer v3 00/38] add auto-installer integration Lukas Wagner
2026-04-14 13:58 ` Lukas Wagner
2026-04-17  8:42 ` Dominik Csapak
2026-04-17  9:10   ` Dominik Csapak
2026-04-17  9:25     ` Lukas Wagner
2026-04-17  9:48       ` Dominik Csapak
2026-04-17 11:28         ` Lukas Wagner
2026-04-17 11:53           ` Dominik Csapak
2026-04-20  9:54             ` Lukas Wagner
2026-04-20 10:08               ` Dominik Csapak
2026-04-20 11:18                 ` Lukas Wagner
2026-04-20 12:09                   ` Dominik Csapak [this message]
2026-04-20 12:26                     ` Christoph Heiss
2026-04-21  7:07                       ` Lukas Wagner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c3b2cdce-08c2-42c5-a6c3-61a40e4a03f7@proxmox.com \
    --to=d.csapak@proxmox.com \
    --cc=c.heiss@proxmox.com \
    --cc=l.wagner@proxmox.com \
    --cc=pdm-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal