Re: Problems with Plasma NetworkManagement

[Dan Williams <dcbw redhat com>]

On Wed, 2011-07-20 at 15:18 -0300, Lamarque Vieira Souza wrote:
> Em Wednesday 20 July 2011, Dan Williams escreveu:
> 
> > On Tue, 2011-07-19 at 21:53 -0300, Lamarque Vieira Souza wrote:
> 
> > > Em Saturday 09 July 2011, Lamarque Vieira Souza escreveu:
> 
> > > > Em Sunday 03 July 2011, Lamarque Vieira Souza escreveu:
> 
> > > > > Em Saturday 02 July 2011, Dan Williams escreveu:
> 
> > > > > > Yeah, the applet and editor are two separate processes. We
> can
> 
> > > 
> 
> > > send an
> 
> > > 
> 
> > > > > > Update() along anyway if you wish though, I simply had to
> make a
> 
> > > > > > 
> 
> > > > > > behavior call at the time it was implemented, and we can
> change
> 
> > > 
> 
> > > that
> 
> > > 
> 
> > > > > > behavior. Obviously agents should be smart enough to know
> when
> 
> > > 
> 
> > > the
> 
> > > 
> 
> > > > > > incoming secrets are different than what they already have
> and
> 
> > > 
> 
> > > possibly
> 
> > > 
> 
> > > > > > ignore the Update() if nothing has actually changed.
> 
> > > > > > 
> 
> > > > > > 
> 
> > > > > > 
> 
> > > > > > If NM is passing the old secrets, that's clearly not cool,
> and
> 
> > > 
> 
> > > we need
> 
> > > 
> 
> > > > > > to fix that.
> 
> > > > > 
> 
> > > > > Well, a new Update() call is not necessary, I just need NM to
> call
> 
> > > > > 
> 
> > > > > 
> 
> > > > > 
> 
> > > > > Update() with the new secrets instead of the old ones. The
> 
> > > 
> 
> > > attached patch
> 
> > > 
> 
> > > > > does exactly that and fix the problem for me. But NM's API
> does
> 
> > > 
> 
> > > not allow
> 
> > > 
> 
> > > > > me to list all settings from a connection, I have to name them
> one
> 
> > > 
> 
> > > by
> 
> > > 
> 
> > > > > one, so I created the patch only for
> NMSettingWirelessSecurity. I
> 
> > > 
> 
> > > think
> 
> > > 
> 
> > > > > you can improve it to work with all other secret settings,
> right?
> 
> > > > 
> 
> > > > Dan Willians, can you comment the patch I sent you last week?
> This
> 
> > > > 
> 
> > > > problem is a show-stop for Plasma NM and I would like to have it
> 
> > > 
> 
> > > solve as
> 
> > > 
> 
> > > > soon as possible.
> 
> > > 
> 
> > > Hi again, any news about this bug?
> 
> > 
> 
> > Yeah, I did look into it earlier this week. The patch you posted
> hacks
> 
> > around it but is not the correct fix obviously. What might be
> happening
> 
> > here is the patch to keep agent-owned secrets when re-reading the
> config
> 
> > files after committing changes to disk. The original problem was
> that
> 
> > when secrets come back from the secret agent, those may include
> 
> > system-owned secrets which need to get written out to disk. That
> 
> > triggers an inotify event which re-reads the connection; the newly
> read
> 
> > connection is compared to the in-memory one, but they differ because
> the
> 
> > in-memory one can contain agent-owned or always-ask secrets that
> don't
> 
> > get written to disk. We need those in-memory secrets of course since
> we
> 
> > just asked for them. That's why the code keeps some of the secrets
> 
> > around, and that might be why you're seeing "old" secrets.
> 
> 
> This sounds like a cache effect. My WPA wifi connection is not
> system-owned. Anyway, the in-memory secrets should be considered the
> updated ones, aren't they? Moreover, if NM is in the middle of an
> Update() process, which is the case, it should update the *in-memory
> secrets* first before doing anything which needs the secrets, like
> calling SaveSecrets(), right?
> 
> > So what's the flow here? Is it during connection, or just editing
> some
> 
> > random connection? ie if you edit a connection that you're not
> 
> > currently using, somehow the secrets are the old ones the secret
> agent
> 
> > used to have?
> 
> 
> It happens when editing a connection, any connection I tested so far.
> The connection editor calls NetworkManager's Update(), NetworkManager
> calls secret agent's SaveSecrets() passing the old secrets and our
> secret agent saves those secrets to kwallet (KDE's equivalent to
> keyring). The end effect is that the secrets are not updated.

So the real problem here turned out to be preservation of transient
secrets in nm_settings_connection_replace_settings() like I'd suggested
earlier.  We do want that behavior (since we need to make sure that
always-ask secrets get preserved during activation even if the
connection gets updated on-disk) but it was really happening at the
wrong time.  So what was going on here was that the Update() call sent
new settings (including new secrets) which were applied to the
connection in nm_settings_connection_replace_settings(), but then
overwritten by the transient secrets copy thing.  Then in
update_auth_cb() the old secrets (copied back by the transient thing)
were sent back to the agents.

I've fixed this in e2d88f59e6b95edfbd0b5ae7e7e396e8ab462664 though
that's not the best fix; the better fix is in the 'agent-secrets' branch
and I'll merge that right after 0.9 is released.  So please test latest
git and let me know if that works as you expect.

Dan