The one usage of this was translating something that was happening in-process to something over mojo. i.e. this was possible before network service, and was used just by cast. the status quo is the same.

I'm not sure how a separate API would help; i.e. once the mojo api is there any code in the browser could call it.

If the 1 usage is no longer live, we should just get rid of it. The existence and use of options like this have been the source of many bad bugs in many platforms and applications.

I think tsepez' point about a separate API is that this:

 interface->DisableCertificateVerificationWithAnObviouslyDangerousFunctionName();

is easier to audit for than

 < interface->SomeCall(false);
 > interface->SomeCall(true);  // Introduce Pri-0 security bug!

. But getting rid of the dangerous option entirely would be ideal.

The 1 usage is used; I just meant the mojo call was added to replace a C++ call.

Note it's a named boolean, so a simple CS search would find it, e.g.
https://cs.chromium.org/chromium/src/components/cast_channel/cast_socket.cc?type=cs&q=skip_cert_verification++-file:src/services/network+-file:out&sq=package:chromium&g=0&l=415

which would be equivalent to a search for a method call.

(An enum would also be better than true/false, FWIW.)

Given that it's currently searchable, I'm marking this as wontfix.

Yeah, its a human factors thing rather than a security boundary. But it is not a theoretical issue -- security sensitive APIs need to be designed to avoid dangerous accidental misuse.

Since the consequences of setting skip_cert_verification = true aren't immediately obvious and a developer may just toggle this option "to get it to work", then this API falls into the above category.

What I meant about a separate API was to remove the boolean entirely, then in

https://cs.chromium.org/chromium/src/services/network/public/mojom/tcp_socket.mojom?rcl=68a35fc7ca051d1b12187cb405d688c864d9d4b7&l=55

Change:

 UpgradeToTLS(HostPortPair host_port_pair,
               TLSClientSocketOptions? options,
               MutableNetworkTrafficAnnotationTag traffic_annotation,
               TLSClientSocket& request,
               SocketObserver? observer)
      => (int32 net_error,
          handle<data_pipe_consumer>? receive_stream,
          handle<data_pipe_producer>? send_stream,
          SSLInfo? ssl_info);

To:

 UpgradeToTLS(HostPortPair host_port_pair,
               TLSClientSocketOptions? options,
               MutableNetworkTrafficAnnotationTag traffic_annotation,
               TLSClientSocket& request,
               SocketObserver? observer)
      => (int32 net_error,
          handle<data_pipe_consumer>? receive_stream,
          handle<data_pipe_producer>? send_stream);

  UnsafelyUpgradeToTLSNoCertVerification(HostPortPair host_port_pair,
               TLSClientSocketOptions? options,
               MutableNetworkTrafficAnnotationTag traffic_annotation,
               TLSClientSocket& request,
               SocketObserver? observer)
      => (int32 net_error,
          handle<data_pipe_consumer>? receive_stream,
          handle<data_pipe_producer>? send_stream,
          SSLInfo ssl_info);

FYI it's possible to do this today, like the code before I changed it, without going to the network service by calling net/ directly. These methods don't have any warning in the method names.

Ok, fair enough, but can we change TLSClientSocketOptions.skip_cert_verification to TLSClientSocketOptions.unsafely_skip_cert_verification just so when this gets used the next time, the dev can't say "I didn't know the operation was unsafe?"

I would still prefer to see a separate method call on the interface, as it is way more obvious. I don't see what the downside of doing that in addition to changing the name itself in the struct.

FWIW, I have implemented #8 in https://chromium-review.googlesource.com/c/chromium/src/+/1222284, although I agree with #9.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5ec257f591db294d48119da2694aca5719c24679

commit 5ec257f591db294d48119da2694aca5719c24679
Author: Chris Palmer <palmer@chromium.org>
Date: Thu Sep 13 23:30:16 2018

Rename `skip_cert_verification` to `unsafely_skip_cert_verification`.

It's not safe, so call that out.

Bug: 880974
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: I9c2e4eb8116c12270aa776fd17db893b46b473d7
Reviewed-on: https://chromium-review.googlesource.com/1222284
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: mark a. foltz <mfoltz@chromium.org>
Commit-Queue: Chris Palmer <palmer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#591207}
[modify] https://crrev.com/5ec257f591db294d48119da2694aca5719c24679/components/cast_channel/cast_socket.cc
[modify] https://crrev.com/5ec257f591db294d48119da2694aca5719c24679/services/network/public/mojom/tcp_socket.mojom
[modify] https://crrev.com/5ec257f591db294d48119da2694aca5719c24679/services/network/public/mojom/tls_socket.mojom
[modify] https://crrev.com/5ec257f591db294d48119da2694aca5719c24679/services/network/tls_socket_factory.cc
[modify] https://crrev.com/5ec257f591db294d48119da2694aca5719c24679/services/network/tls_socket_factory.h

jam says (on referenced CL):

I don't think renaming this boolean makes things clearer or safer.

1) this is a method from a more trusted process to a less trusted process

2) the calling process can do this without mojo

3) in the one place that it's used, it's not unsafe to use this. i.e. the caller does their own handshake.

Now, someone reading the code now would look at the cast code and think it's erroneous or should be fixed somehow.

1. See e.g. https://www.sciencedirect.com/science/article/pii/S2210832716300722 for a nice summary of the papers written about how this and similar issues have played out on other platforms in the past, and why we're so uptight about APIs to TLS that skip checks.

2. Where's the call without mojom?  We'd like to fix it, too.

Re 1, can you point to which parts of this long doc you're referring to? The boolean is off by default, so we're just talking about its name.

Re 2: what I mean is that any code in the browser can create a socket today and skip cert verification, e.g. see the old code I removed in https://chromium-review.googlesource.com/c/chromium/src/+/1182949