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 > >>>> > > > >
