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: Fri, 17 Apr 2026 13:53:21 +0200 [thread overview]
Message-ID: <52dc9715-66dc-4dc2-a3dc-4216f62f33f2@proxmox.com> (raw)
In-Reply-To: <DHVE7INI0B6R.3W4L73YN7EJW0@proxmox.com>
On 4/17/26 1:26 PM, Lukas Wagner wrote:
> On Fri Apr 17, 2026 at 11:48 AM CEST, Dominik Csapak wrote:
>>
>>
>> On 4/17/26 11:23 AM, Lukas Wagner wrote:
>>> On Fri Apr 17, 2026 at 11:10 AM CEST, Dominik Csapak wrote:
>>>> also two things i forgot:
>>>>
>>>> i think we could make the endpoint available without authentication?
>>>> at least that's what i would have expected, since the current
>>>> http endpoints also had to be public?
>>>>
>>>> second things is about the tokens + acl, I'd probably
>>>> use the existing token acl mechanism, so instead of
>>>> having custom tokens + a token list per answer,
>>>>
>>>> use the standard pdm tokens + an acl path (e.g.
>>>> /system/auto-installation/answers/<id> ) maybe a separate privilege
>>>> could make sense here, so the token does not have access
>>>> to anything else?
>>>>
>>>
>>> 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?
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 token
(for example, we allow the migration traffic to be unencrypted too,
but only if the admin decides that)
we could print a big warning that this info is publicly exposed if
no token restriction is used, or something like this.
but as i said this is more of choice, not a purely technical argument
>
>>
>> also i'm not for making it unauthenticated for all responses, but
>> make it the admins choice.
>>
>> as for the DOS vector of pdm, we could check if the user was
>> authenticated, and only generate the data then.
>
> There is no user information available when the installer requests the
> answer file, how would we check it?
>
> Sorry, maybe I'm misunderstanding your proposal here :)
>
how it currently works is that the iso must be prepared with a token,
i'd propose leaving that in, but only as an option if the admin
wants to do the authentication, but not as a requirement
the api could check if there is a user in the rpcenv anyway and check
their permissions against the available prepared answers
>>
>> but how much data is that really? e.g. each api call (authenticated or
>> not) creates a new line in the access log. so with enough api calls,
>> i can fill up the disk too...
>
> You're right about the access log. That being said, an unauthenticated
> user would still be able to create bogus entries installation entries
> that would show up in the UI.
>
btw. most of the pain with the authentication imho here comes
from having separate api tokens just for the automated installs
and the lack of guidance what the user should do.
if we e.g. can autogenerate the 'proxmox-auto-install-assistant'
cli command with the gui and make it copyable, makes this probably
a lot less cumbersome
prev parent reply other threads:[~2026-04-17 11:54 UTC|newest]
Thread overview: 55+ 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 [this message]
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=52dc9715-66dc-4dc2-a3dc-4216f62f33f2@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.