+1 for d) don't print any env vars in the web UI In the longer term, instead of a flag to hide/show certain variables, I would be more inclined to move towards role based access for the Flink dashboard.
Thanks, On Wed, Nov 16, 2022 at 10:22 PM Thomas Weise <t...@apache.org> wrote: > +1 for d) don't print any env vars in the web UI (at least by default) > > There could be an option to allow-list printing of env vars but it should > be off by default. > > Generally I think that those that should be able to see env vars probably > can get there by other means, like kubetcl exec > > Thanks, > Thomas > > > On Wed, Nov 16, 2022 at 4:43 PM Konstantin Knauf <kna...@apache.org> > wrote: > > > Hi everyone, > > > > thanks a lot for your feedback so far. Right now, we have pretty much a > > consensus to not show environment variables at all in the Web UI going > > forward. > > > > I think, we can address this in 1.16.1, as I consider this a > vulnerability > > that should be addressed in a patch release rather than waiting for the > > next minor release. > > > > Are there any other suggestions on how to proceed? > > > > Cheers, > > > > Konstantin > > > > > > > > Am Mi., 16. Nov. 2022 um 09:23 Uhr schrieb Gyula Fóra < > > gyula.f...@gmail.com > > >: > > > > > I am not opposed to removing this completely based on Chesnay's > > reasoning. > > > In general I agree that this feature probably does more harm than good. > > > > > > Gyula > > > > > > On Wed, Nov 16, 2022 at 9:13 AM Chesnay Schepler <ches...@apache.org> > > > wrote: > > > > > > > I'm inclined to go with d), removing it entirely. > > > > > > > > I must admit that I liked the idea behind the change; exposing more > > > > information about what might impact Flink's behavior is a good thing, > > > > although I'm irked that the statement in the FLIP about env variables > > > > already being exposed in the logs just isn't correct. > > > > > > > > I was quite disappointed when I saw that b) wasn't already > implemented. > > > > It is concerning that this actually made it's way through the review > > > > rounds as-is. > > > > > > > > That being said I don't think that b) would be sufficient in anyway; > > the > > > > desensitization logic for config options is quite limited (and > > > > inherently not perfect), but config options are too important to not > > log > > > > them. This isn't really the case for environment variables that have > > > > limited effects on Flink, and it's just too easy to leak secrets. > > > > Oh you abbreviated PASSWORD to PW? Well you just leaked it. > > > > > > > > This brings us to c), where my immediate gut instinct was that no > ones > > > > gonna bother to do that. > > > > > > > > as for e*) (opt-in flag that Gyula proposed); I think it's to easy to > > > > shoot yourself in the foot somewhere down the line. It may be fine at > > > > one point but setups evolve after all, and this seems like something > to > > > > easily slip through. > > > > > > > > On 15/11/2022 15:41, Konstantin Knauf wrote: > > > > > Hi everyone, > > > > > > > > > > important correction, this is since 1.16.0, not 1.17+. > > > > > > > > > > Best, > > > > > > > > > > Konstantin > > > > > > > > > > Am Di., 15. Nov. 2022 um 14:25 Uhr schrieb Gyula Fóra < > > > > gyula.f...@gmail.com > > > > >> : > > > > >> Thanks for bringing this important issue to discussion Konstantin! > > > > >> > > > > >> I am in favor of not showing them by default with an optional > > > > configuration > > > > >> to enable it. > > > > >> Otherwise this poses a big security risk of exposing previously > > hidden > > > > >> information after upgrade. > > > > >> > > > > >> Gyula > > > > >> > > > > >> On Tue, Nov 15, 2022 at 2:15 PM Maximilian Michels < > m...@apache.org> > > > > wrote: > > > > >> > > > > >>> Hey Konstantin, > > > > >>> > > > > >>> I'd be in favor of not printing them at all, i.e. option (d). We > > have > > > > the > > > > >>> configuration page which lists the effective config and already > > > removes > > > > >> any > > > > >>> known secrets. > > > > >>> > > > > >>> -Max > > > > >>> > > > > >>> On Tue, Nov 15, 2022 at 11:26 AM Konstantin Knauf < > > kna...@apache.org > > > > > > > > >>> wrote: > > > > >>> > > > > >>>> Hi all, > > > > >>>> > > > > >>>> since Flink 1.17 [1] the Flink Web UI prints *all* environment > > > > >> variables > > > > >>> of > > > > >>>> the Taskmanager or Jobmanagers hosts (Jobmanager -> > Configuration > > -> > > > > >>>> Environment). Given that environment variables are often used to > > > store > > > > >>>> sensitive information, I think, it is wrong and dangerous to > print > > > > >> those > > > > >>> in > > > > >>>> the Flink Web UI. Specifically, thinking about how Kubernetes > > > Secrets > > > > >> are > > > > >>>> usually injected into Pods. > > > > >>>> > > > > >>>> One could argue that anyone who can submit a Flink Job to a > > cluster > > > > has > > > > >>>> access to these environment variables anyway, but not everyone > who > > > has > > > > >>>> access to the Flink UI can submit a Flink Job. > > > > >>>> > > > > >>>> I see the the following options: > > > > >>>> a) leave as is > > > > >>>> b) apply same obfuscation as in flink-conf.yaml based on some > > > > heuristic > > > > >>> (no > > > > >>>> "secret", "password" in env var name) > > > > >>>> c) only print allow-listed values > > > > >>>> d) don't print any env vars in the web UI (at least by default) > > > > >>>> > > > > >>>> What do you think? > > > > >>>> > > > > >>>> Cheers, > > > > >>>> > > > > >>>> Konstantin > > > > >>>> > > > > >>>> [1] https://issues.apache.org/jira/browse/FLINK-28311 > > > > >>>> > > > > >>>> -- > > > > >>>> https://twitter.com/snntrable > > > > >>>> https://github.com/knaufk > > > > >>>> > > > > > > > > > > > > > > > > > > > > > > -- > > https://twitter.com/snntrable > > https://github.com/knaufk > > >
