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

[Rémi Denis-Courmont]

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.

(...)
>> 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.

-- 
Rémi Denis-Courmont
http://www.remlab.net/