Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default for TLS_PEER_VERIFY socket option are set to required, may lead to confusion when running samples against self-signed certs #14632

Closed
laperie opened this issue Mar 18, 2019 · 14 comments
Assignees
Labels
area: Networking area: Security Security Enhancement Changes/Updates/Additions to existing features priority: low Low impact/importance bug

Comments

@laperie
Copy link
Collaborator

laperie commented Mar 18, 2019

Describe the bug
samples/sockets/big_http_download

To Reproduce
Steps to reproduce the behavior:

  1. cd samples/sockets/big_http_download
  2. mkdir build && cd build
  3. cmake -DBOARD=qemu_x86 -DOVERLAY_CONFIG=overlay-tls.conf ..
  4. make run

Expected behavior
HTTPS request made to https://www.7-zip.org/a/7z1805.exe and file is fetched

Impact
We have no verified cases that TLS works

Screenshots or console output

SeaBIOS (version rel-1.12.0-0-ga698c8995f-prebuilt.qemu.org)
Booting from ROM..***** Booting Zephyr OS v1.14.0-rc1-1247-gb3eb510f5c3a *****
[00:00:00.010,000] net_ctx.net_context_bind: (0x004106c0): Context 0x0040f620 binding to UDP 0.0.0.0:59111 i0
[00:00:00.070,000] net_sock_tls: No entropy device on the system, TLS communication may be insecure!
[00:00:00.080,000] net_config: Initializing network
[00:00:00.080,000] net_config: IPv4 address: 192.0.2.1
[00:00:00.080,000] net_config: Running dhcpv4 client...
Preparing HTTP GET request for https://www.7-zip.org:443/a/7z1805.exe
addrinfo @0x424008: ai_family=1, ai_socktype=1, ai_protocol=6, sa_family=1, sin_port=bb01
sock = 0
[00:00:03.130,000] net_sock_addr.dns_resolve_cb: (0x0040fc98): dns status: -100
[00:00:03.130,000] net_sock_addr.dns_resolve_cb: (0x0040fc98): dns status: -103
[00:00:03.130,000] net_sock_tls.tls_alloc: (0x004106c0): Allocated TLS context, 0x00400580
[00:00:03.130,000] net_ctx.net_context_bind: (0x004106c0): Context 0x0040f6c0 binding to TCP 0.0.0.0:52357 i0

Environment (please complete the following information):
Linux
0.10.0 SDK

Additional info
The problem is consistent independently of the URL. Regular fetching of the URL works.
The problem does not seem to be related to the server certificate as the symptoms are identical for self-signed and proper certificates.

@laperie laperie added bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug area: Networking labels Mar 18, 2019
@jukkar
Copy link
Member

jukkar commented Mar 18, 2019

Assigning a bug to many persons does not usually work as expected as then it is not really known who will look at it. As this looks like a socket TLS issue and @rlubos is the main expert there, I am reassigning this to him atm.

@jukkar jukkar assigned rlubos and unassigned pfalcon, pfl and jukkar Mar 18, 2019
@jukkar
Copy link
Member

jukkar commented Mar 18, 2019

I actually tried this too in qemu_x86 with local https server (using overlay-tls.conf and overlay-e1000.conf), the log looks like this:

[00:00:00.010,000] <wrn> net_sock_tls: No entropy device on the system, TLS communication may be inse!
[00:00:00.010,000] <inf> net_config: Initializing network
[00:00:00.010,000] <inf> net_config: IPv4 address: 192.0.2.1
[00:00:00.010,000] <inf> net_config: Running dhcpv4 client...
Preparing HTTP GET request for https://192.0.2.2:4443/foobar
addrinfo @0x426008: ai_family=1, ai_socktype=1, ai_protocol=6, sa_family=1, sin_port=5b11
sock = 0
[00:00:03.030,000] <dbg> net_sock_addr.dns_resolve_cb: (0x00410780): dns status: -100
[00:00:03.030,000] <dbg> net_sock_addr.dns_resolve_cb: (0x00410780): dns status: -103
[00:00:03.040,000] <dbg> net_sock_tls.tls_alloc: (0x00410780): Allocated TLS context, 0x00400580
Error: connect(sock, ai->ai_addr, ai->ai_addrlen)
exit

@jukkar
Copy link
Member

jukkar commented Mar 18, 2019

@rlubos if you think this is not TLS related, please feel to reassing to me for example.
The connectivity works ok without https.

[00:00:00.010,000] <inf> net_config: Initializing network
[00:00:00.010,000] <inf> net_config: IPv4 address: 192.0.2.1
[00:00:00.010,000] <inf> net_config: Running dhcpv4 client...
Preparing HTTP GET request for http://192.0.2.2:8080/foobar
addrinfo @0x416008: ai_family=1, ai_socktype=1, ai_protocol=6, sa_family=1, sin_port=901f
sock = 0
[00:00:03.030,000] <dbg> net_sock_addr.dns_resolve_cb: (0x004019a0): dns status: -100
[00:00:03.030,000] <dbg> net_sock_addr.dns_resolve_cb: (0x004019a0): dns status: -103
[00:00:03.040,000] <dbg> net_sock.zsock_socket_internal: (0x004019a0): socket: ctx=0x0040099c, fd=0
[00:01:09.960,000] <dbg> net_sock.zsock_received_cb: (0x00400f78): ctx=0x0040099c, pkt=0x00405804, st0
[00:01:12.120,000] <dbg> net_sock.zsock_received_cb: (0x00400f78): ctx=0x0040099c, pkt=0x00405804, st0
[00:01:14.130,000] <dbg> net_sock.zsock_received_cb: (0x00400f78): ctx=0x0040099c, pkt=0x00405804, st0

@rlubos
Copy link
Contributor

rlubos commented Mar 19, 2019

Thanks, I'll try to reproduce/debug.

@rlubos
Copy link
Contributor

rlubos commented Mar 19, 2019

Hmm tried to reproduce, but the sample works out of the box for me :| (I've tested current master)

SeaBIOS (version rel-1.11.2-0-gf9626ccb91-prebuilt.qemu-project.org)
Booting from ROM..***** Booting Zephyr OS v1.14.0-rc1-1276-g932a33ad7502 *****
[00:00:00.060,000] <wrn> net_sock_tls: No entropy device on the system, TLS communication may be insecure!
[00:00:00.060,000] <inf> net_config: Initializing network
[00:00:00.060,000] <inf> net_config: IPv4 address: 192.0.2.1
[00:00:00.060,000] <inf> net_config: Running dhcpv4 client...
Preparing HTTP GET request for https://www.7-zip.org:443/a/7z1805.exe
addrinfo @0x424008: ai_family=1, ai_socktype=1, ai_protocol=6, sa_family=1, sin_port=bb01
sock = 0
1181017 bytes
Hash: 647a9a621162cd7a5008934a08e23ff7c1135d6f1261689fd954aa17d50f9729
Total downloaded so far: 1MB
sock = 0
294657 bytes

As you see the sample did connect and downloaded the file, next download is in progress... Perhaps some wireshark pcap or mbedTLS logs could be helpful to identify whether it's a problem with TLS handshake.

@pfalcon
Copy link
Contributor

pfalcon commented Mar 19, 2019

Confirming @rlubos's results with f4c8e3f and build completely OOB (the original report contains changes at least to logging config):

SeaBIOS (version rel-1.12.0-0-ga698c8995f-prebuilt.qemu.org)
Booting from ROM..***** Booting Zephyr OS v1.14.0-rc1-1277-gf4c8e3ff3197 *****
[00:00:00.170,000] <err> slip: [0x00400b00] cannot allocate pkt
--- 87 messages dropped ---
[00:00:00.170,000] <err> slip: [0x00400b00] cannot allocate pkt
[00:00:00.170,000] <err> slip: [0x00400b00] cannot allocate pkt
[00:00:00.170,000] <err> slip: [0x00400b00] cannot allocate pkt
[00:00:00.170,000] <err> slip: [0x00400b00] cannot allocate pkt
[00:00:00.170,000] <err> slip: [0x00400b00] cannot allocate pkt
[00:00:00.170,000] <err> slip: [0x00400b00] cannot allocate pkt
[00:00:00.170,000] <err> slip: [0x00400b00] cannot allocate pkt
[00:00:00.170,000] <err> slip: [0x00400b00] cannot allocate pkt
[00:00:00.170,000] <err> slip: [0x00400b00] cannot allocate pkt
[00:00:00.170,000] <err> slip: [0x00400b00] cannot allocate pkt
[00:00:00.180,000] <err> slip: [0x00400b00] cannot allocate pkt
[00:00:00.180,000] <err> slip: [0x00400b00] cannot allocate pkt
[00:00:00.180,000] <err> slip: [0x00400b00] cannot allocate pkt
[00:00:00.180,000] <err> slip: [0x00400b00] cannot allocate pkt
[00:00:00.180,000] <err> slip: [0x00400b00] cannot allocate pkt
[00:00:00.180,000] <err> slip: [0x00400b00] cannot allocate pkt
[00:00:00.180,000] <err> slip: [0x00400b00] cannot allocate pkt
[00:00:00.180,000] <err> slip: [0x00400b00] cannot allocate pkt
[00:00:00.180,000] <err> slip: [0x00400b00] cannot allocate pkt
[00:00:00.180,000] <err> slip: [0x00400b00] cannot allocate pkt
[00:00:00.180,000] <err> slip: [0x00400b00] cannot allocate pkt
[00:00:00.190,000] <err> slip: [0x00400b00] cannot allocate pkt
[00:00:00.190,000] <err> slip: [0x00400b00] cannot allocate pkt
[00:00:00.190,000] <err> slip: [0x00400b00] cannot allocate pkt
[00:00:00.190,000] <err> slip: [0x00400b00] cannot allocate pkt
[00:00:00.200,000] <err> slip: [0x00400b00] cannot allocate pkt
[00:00:00.200,000] <err> slip: [0x00400b00] cannot allocate pkt
[00:00:00.220,000] <wrn> net_sock_tls: No entropy device on the system, TLS communication may be insecure!
[00:00:00.220,000] <inf> net_config: Initializing network
[00:00:00.220,000] <inf> net_config: IPv4 address: 192.0.2.1
[00:00:00.220,000] <inf> net_config: Running dhcpv4 client...
Preparing HTTP GET request for https://www.7-zip.org:443/a/7z1805.exe
addrinfo @0x424008: ai_family=1, ai_socktype=1, ai_protocol=6, sa_family=1, sin_port=bb01
sock = 0
1181017 bytes
Hash: 647a9a621162cd7a5008934a08e23ff7c1135d6f1261689fd954aa17d50f9729
Total downloaded so far: 1MB
sock = 0
114433 bytes

("slip: [0x00400b00] cannot allocate pkt" eyecandy is from #13991).

@laperie laperie changed the title big_http_download sample broken for TLS Default for TLS_PEER_VERIFY socket option are set to required peer verification Mar 20, 2019
@laperie laperie added Enhancement Changes/Updates/Additions to existing features priority: low Low impact/importance bug and removed bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug labels Mar 20, 2019
@laperie
Copy link
Collaborator Author

laperie commented Mar 20, 2019

Okay. Turns out that the tests were failing in our environment because self-signed certs are not accepted by default. The following code snippet enabled the test (big_http_download.c:163):
#if defined(CONFIG_NET_TLS_PEER_NOVERIFY)
/* Allowing self-signed certificate, for testing with local httpd /
{
int peer_verify = 0 ;
CHECK(setsockopt(sock, SOL_TLS, TLS_PEER_VERIFY,
&peer_verify, sizeof(peer_verify)));
}
#endif /
CONFIG_NET_TLS_PEER_NOVERIFY */

@laperie
Copy link
Collaborator Author

laperie commented Mar 20, 2019

So, the best option going forward would be to enable the option to allow self-signed certificates (with appropriate disclaimers of course) for development time. But everything works now.
Hence, downgrading the priority and changing type from bug to enhancement.

@pfalcon
Copy link
Contributor

pfalcon commented Mar 20, 2019

So, the best option going forward would be to enable the option to allow self-signed certificates (with appropriate disclaimers of course) for development time.

I'd be +1 for enabled this option for this sample (big_http_download) with appropriate notice to README. Doubt we would enable it in Zephyr config by default, guys like @d3zd3z actually call for more stringent default setup of security-related things, not less than what we have now. But I found the "for development time" phrase resounding with what I have in mind - I wonder if we could/would/should have different Kconfig defaults for official releases vs just git master. That would be chory/cumbersome and likely confusing, so would need good discussion and consideration.

@pfalcon
Copy link
Contributor

pfalcon commented Mar 20, 2019

@rlubos, so you may want to check re: error reporting in such cases (I would say that's the only thing which can be distilled from this report in the meantime, unfortunately, it's not clear if that's the issue or not, as now the ticket title, description, and console dump don't correspond to each other).

@pfalcon pfalcon changed the title Default for TLS_PEER_VERIFY socket option are set to required peer verification Default for TLS_PEER_VERIFY socket option are set to required, may lead to confusion when running samples against self-signed certs Mar 20, 2019
@pfalcon pfalcon added the area: Security Security label Mar 20, 2019
@rlubos
Copy link
Contributor

rlubos commented Mar 20, 2019

TLS_PEER_VERIFY was added mostly to make developers life easier, not having to be concerned about certificate contents during development. I guess we could go a step further, and configure the default behavior thorough Kconfig, but we should at least pritnt out the warning in case the 'unsafe' default behavior is enabled (just as we do when no good entropy source is present in the system).

As for the error reporting - we alredy print the error code that mbedTLS produce in case of handshake failue (at socket log level) - what more could we make? Limited errno space is not helpinig here either.

@jukkar I'm trying to understand what exactly is wrong with the self-signed certificate that you use. We are actively using certificates available in net-tools repository, aren't they self-signed as well?
What comes to my mind is that it is very likely that we have a hostname mismatch here. The certificate should have assigned a hostname (called Common Name, CA), and we enforce it's verification in TLS sockets, see TLS_HOSTNAME option. If the name we provide does not match the name configured in the certificate, the handshake will fail. Not sure though how would mbedTLS behave in case there is no CA bound to the certificate.

The hostname check can be disabled by providing NULL for TLS_HOSTNAME option.

@pfalcon
Copy link
Contributor

pfalcon commented Mar 20, 2019

As for the error reporting - we alredy print the error code that mbedTLS produce in case of handshake failue (at socket log level)

Which level exactly?

  • what more could we make?

Well, as this report is against big_http_download, then to make sure it prints an error (using printf). @laperie logdump shows as if it hanged. (But then it also shows https://www.7-zip.org/a/7z1805.exe, while it turns out the report is actually about accessing something different with self-signed cert, as apparently https://www.7-zip.org doesn't have self-signed cert).

So, all in all, I just suggest that the behavior of big_http_download is checked when run against URL which doesn't have a proper cert, to make sure that this behavior isn't too confusing (i.e. it either works, or error is printed, extra points for clear enough error).

@jukkar
Copy link
Member

jukkar commented Jan 28, 2020

Going through old issues. This is a good candidate for closing. Can be re-opened if really needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Networking area: Security Security Enhancement Changes/Updates/Additions to existing features priority: low Low impact/importance bug
Projects
None yet
Development

No branches or pull requests

5 participants