[vlc-devel] [PATCH 1/2] gnutls: show a dialog allowing the user to bypass certificate issues

[Ludovic Fauvet]

On Thu, Jun 28, 2012 at 3:07 PM, Rémi Denis-Courmont <remi at remlab.net> wrote:
> On Thu, 21 Jun 2012 10:34:48 +0200, Ludovic Fauvet <etix at videolan.org>
> wrote:
>> On Thu, Jun 21, 2012 at 5:13 AM, Rémi Denis-Courmont <remi at remlab.net>
>> wrote:
>>> Le jeudi 21 juin 2012 02:35:58 Ludovic Fauvet, vous avez écrit :
>>>> ---
>>>>  include/vlc_tls.h     |    2 +-
>>>>  modules/access/http.c |    4 +++-
>>>>  modules/misc/gnutls.c |   37 ++++++++++++++++++++++++++++++++++---
>>>>  src/network/tls.c     |    9 +++++----
>>>>  4 files changed, 43 insertions(+), 9 deletions(-)
>>>
>>> I don't understand why you export the trust bit. If the certificate is
>>> not
>>> trustworthy, the connection should fail anyway.
>>
>> The reason is that the GNU TLS module is loaded 5 times for a single
>> connection,
>
> That would be a bug, and it should be fixed rather than worked around.

I was only assuming the HTTP module was behaving correctly. My
intention was not to work around it.

> You cannot negotiate TLS more than once per TCP connection, at least not
> for any of the few protocols that VLC actually implements.

Thank you but I never said the opposite.

>> thus asking the user's authorization 5 times in a row
>
> ...would be The Right Thing.

For the same resource, on the same domain and less than 1sec after the
previous confirmation?
Firefox don't ask you for every single resource contained in a
webpage. Once you accept it (still without remembering it) is, at
least, for the complete page.

> Just because the user agreed to use one particular untrusted certificate
> does not imply that (s)he would agree to use any other untrusted
> certificate. Definitely not so.

This is not the purpose of this patch.

>> and AFAIK exporting the trust bit to the http module is the most obvious
>> way to keep the state for the whole http session.
>
> Doing it like every body else is obvious to me. And that is not what
> everybody else does. This proposal strikes me as obviously stupid rather
> than obvious.
>
> For the reference, "everybody else" remembers which certificates have been
> permitted under which circumstances, e.g. certificate X is permitted for
> host "streaming.example.com." even though its CN is "www.example.com.".

Again, this is out of the scope of this patch. Reconnecting to the
same server after stopping the playback requires a new confirmation
from the user. I don't remember anything except for the current http
session.

> There is no deying that this is not a simple development task. If it were
> as simple as you make it, it would have been implemented a long time ago.
> But TLS, just like HTTP cookies, is a security-sensitive feature. You
> cannot take many shortcuts when it comes to security.

Absolutely, but you didn't raise any valid argument until now except
that there's maybe a bug inside the http module...

>>> I think the messages are way too simplistic for normal people to
>>> understand.
>>>
>>> Conversely, for expired certificates, the dates are needed. For
>>> mismatched
>>> namse, the names are needed. And for untrusted roots, the certificates
>>> need to
>>> be shown. Otherwise there is no way to determine whether the situation
>>> is safe
>>> or not.
>>
>> A question dialog won't suffice then.
>
> A VLC question dialog in the current form might be too primitive to
> present the needed data. Nevertheless, it is very much a Yes/No question.

Agreed.

>>> I am also not sure this works very well with streaming output cases.
>>
>> This code is for the TLS client only. Shouldn't be an issue for
>> streaming output cases.
>
> You can use a secure input when streaming, can't you?

So what? The behavior is still the same:
- If you don't have an interface attached, it will reject the
untrusted cert and break.
- If you got one the code will block until the user confirmation.
This is not what I expect from the code, this is what the code do. I tested it.

Just one single question, did you even read the patch?

>
> --
> Rémi Denis-Courmont
> Sent from my collocated server

Regards,
-- Ludovic Fauvet