[vlc-devel] [PATCH 1/4] add vlc_keystore API

[Thomas Guillem]

On Thu, Nov 26, 2015, at 13:31, Rémi Denis-Courmont wrote:
> Le 2015-11-26 13:39, Thomas Guillem a écrit :
> > OK, we may then remove user as input for http, but there are other
> > protocol that can use an user as input like smb (user can be 
> > specified
> > via command line arguments).
> 
> I must disagree. SMB and FTP are no different than HTTP.
> The credentials are the pair of the username and password.
> 
> Besides username in URL is deprecated and discouraged for all 
> protocols.
> 
> >> > +#define KEY_PROTOCOL "protocol"
> >> > +#define KEY_PORT "port"
> >> > +#define KEY_SERVER "server" /* use it for KEY_HOST */
> >> > +#define KEY_DOMAIN "domain"
> >> > +#define KEY_REALM "realm"
> >> > +#define KEY_PATH "path"
> >>
> >> This seems more convoluted than necessary and less generic than 
> >> (URL,
> >> realm)
> >> and the authentication type is missing.
> >
> > Maybe I'll add some http/smb/ftp helpers that will use the
> > vlc_keystore_dict_store and these keys.
> 
> That's logically impossible. HTTP(S) needs an auth type, a realm, an 
> URL, and hypothetically some auth type specific scoping parameters. Type 
> and type-specific parameters can conceivably be coded in the output 
> rather than the input. But there is no URL in the key set above.
> 
> I think it is _not_ reasonable to require the back-end have such level 
> of abstraction, unless we actually need that abstraction. I still fail 
> to see any justification for the generic dictionary thing. I don't want 
> to have reverse abstraction and subtle differences and bugs in the 
> back-ends.
> 
> I note that kwallet indexes by URL, not by dictionary. It's probably 
> not alone.

Yes, just saw it, I did a quick look before doing my API and saw some
read/write Map methods. But that's not the same use case than my API:
for one key (that is an URI), you either store a map, a password or
binary blob.

> 
> (...)
> >> While I would agree that a key store API should support deleting
> >> credentials
> >> manually, I doubt that it will be:
> >> 1/ accessible/useful in VLC,
> >> and
> >> 2/ universally supported by all back-ends.
> >>
> >> That would rather seem to belong in the password manager 
> >> application.
> >
> > Yes, I hesitated too for this one, but I thought maybe we'll add a
> > libvlc_keystore api to use this function.
> 
> We should leave it up to the back-end whether the secrets are shared 
> with LibVLC apps, and with other apps, IMHO, deletion belongs in the 
> back-end-specific manager, _not_ belong in the media player, especially 
> if shared.
> 
> >> Same here.
> >
> > That's the function that retrieve the secret from a found entry.
> > With libsecret (And I hope with others modules too), you can search 
> > for
> > items without unlocking the keyring, and without fetching the secret.
> > This function will unlock the keyring (if not unlocked), and fetch 
> > the
> > password from the service. Because you don't want to unlock a keyring
> > (and ask the user for the master password) for nothing if no items
> > match.
> 
> You can't know what's inside the key ring if you don't unlock it.
> 
> I would argue that your back-end sucks very much if it even allows 
> listing entries while locked.

I don't think it sucks, the user can avoid to type a password for
searching entries that are not secret.
But unfortunately, Kwallet and Apple KeyChain don't allow that. Maybe
I'll simplify the api to return the secret directly as a char *.

Thanks for the review, I'll rework my API.

> 
> -- 
> Rémi Denis-Courmont
> http://www.remlab.net/
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel