Re: [PATCH] wget: don't silently ignore certificate validation

[Denys Vlasenko]

On Sun, May 27, 2018 at 2:21 AM, Kang-Che Sung  wrote:
> On Sun, May 27, 2018 at 1:34 AM, Denys Vlasenko
>  wrote:
>> wget should work for common use cases.
>> Such as downloading sources of kernels, gcc and such.
>> From build scripts, not only by hand.
>> Without having to modify said scripts.
>> Your patch breaks that.
>> NAK.
>>
>> I don't care that security people are upset.
>> They are paranoid, it's part of their profession.
>> It does not mean everybody else have to be as paranoid.
>>
>> If you have a patch which adds actual cert checking
>> and thus does not introduce regressions, please post it.
>
> I think I need to point out that in usability perspective, BusyBox's current
> behaviour is not ideal. It should give a runtime warning that certificate
> checks are skipped, instead of pass it silently.

I'll accept such patch.

> Of course, it would be better
> if actual certificate check is implemented, but if builder disables it (for
> binary size or simplicity), there should be a runtime warning so that 
> usability
> for secure people won't be compromised.

Sure, it'll be wonderful if more people hack on TLS code, improving it.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] wget: don't silently ignore certificate validation

[Denys Vlasenko]

On Sun, May 27, 2018 at 8:55 PM, Michael Conrad  wrote:
> The story just broke earlier this year how a casino hotel "smart
> thermometer" in the fish tank was used as a backdoor to attack the rest of
> their network.
>
> If a smart device running busybox is programmed to automatically check for
> firmware updates, the designers might expect HTTPS to be a valid form of
> security to know that they were accessing their own servers.  If they don't
> write a test case to verify that certificates are checked, it's shoddy work,
> but there's a lot of shoddy work in smart devices.
>
> In such a scenario, anyone on the same wifi would be able to overwrite the
> firmware of the device.  This almost deserves a CVE number.

Anyone on the same wifi can wreak total havoc on the entire network
by messing with ARP, routing and DHCP. If you care that much,
you must not allow untrusted users on your local network.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] wget: don't silently ignore certificate validation

[Denys Vlasenko]

On Sun, May 27, 2018 at 8:19 PM, Ralf Friedl  wrote:
> Denys Vlasenko wrote:
>>
>> wget should work for common use cases.
>> Such as downloading sources of kernels, gcc and such.
>>  From build scripts, not only by hand.
>> Without having to modify said scripts.
>> Your patch breaks that.
>> NAK.
>>
>> I don't care that security people are upset.
>> They are paranoid, it's part of their profession.
>> It does not mean everybody else have to be as paranoid.
>
> I must admit I'm surprised by this statement.

I was surprised when one distro's security people decided to disable
ptrace for non-root users. Because "they don't need it,
and it's more secure that way". Unprivileged users suddenly
not being able to strace their own processes was seen
as unimportant.

Only a flood of thousands of irate emails made them understand
that computers have other purposes apart from being extremely secure.

> You add paranoid changes to programs like cp, unlinking the target in direct
> violation of POSIX, breaking some use cases. There was recent discussion
> about modifying the extraction of TAR and other archives, which introduced
> new problems and regressions.
> While there is nothing wrong with being careful, busybox is mainly used on
> single user systems, so it is unlikely that there is another user to create
> race conditions to exploit.

You misunderstood the nature of "tarball attacks". They are not local.

> On the other hand, not checking https means transfers could be attacked by
> someone anywhere on the network, not only a local user on the machine, so
> the number of potential attacked is much larger, and you don't even print a
> warning that the remote identity is not checked.

We used UNENCRYPTED !!! ftp and http for ~50 years, and somehow
civilization did not collapse. Somehow, when people needed security,
they found ways to ensure it.

There need to be a balance. Security considerations do not automatically
override everything.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] wget: don't silently ignore certificate validation

[Xabier Oneca -- xOneca]

Hello,

> The justification for including HTTPS in the first place:
> https://git.busybox.net/busybox/tree/networking/wget.c?id=8bc418f07eab79a9c8d26594629799f6157a9466#n74
>
> "my small automatic tooling to build cross-compilers from sources no
> longer works, I need to additionally keep a local copy of ~4 megabyte
> source tarball of a SSL library and ~2 megabyte source of wget, need to
> compile and built both before I can download anything. All this despite
> the fact that the build is done in a QEMU sandbox on a machine with
> absolutely nothing worth stealing, so I don't care if someone would go
> to a lot  of trouble to intercept my HTTPS download to send me an
> altered kernel tarball"
>
> This is incredibly terrible logic, your cross-compiler is now infected
> with malicious code. The purpose of compiling code is *usually* to use
> it, which means that wherever you use that code, you're no longer in a
> QEMU sandbox, and whichever real box you use it on, can now say hello to
> unlimited arbitrary code execution.

Well, I see it as "some servers no longer allow to download through
HTTP because they redirect to HTTPS first, so I need a tool which
speaks SSL". In this case, I see the reasoning behind that comment is
acceptable.

Cheers,

Xabier Oneca_,,_
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] wget: don't silently ignore certificate validation

[Michael Conrad]

The story just broke earlier this year how a casino hotel "smart 
thermometer" in the fish tank was used as a backdoor to attack the rest 
of their network.


If a smart device running busybox is programmed to automatically check 
for firmware updates, the designers might expect HTTPS to be a valid 
form of security to know that they were accessing their own servers.  If 
they don't write a test case to verify that certificates are checked, 
it's shoddy work, but there's a lot of shoddy work in smart devices.


In such a scenario, anyone on the same wifi would be able to overwrite 
the firmware of the device.  This almost deserves a CVE number.


-Mike

On 5/26/2018 1:34 PM, Denys Vlasenko wrote:

wget should work for common use cases.
Such as downloading sources of kernels, gcc and such.
 From build scripts, not only by hand.
Without having to modify said scripts.
Your patch breaks that.
NAK.

I don't care that security people are upset.
They are paranoid, it's part of their profession.
It does not mean everybody else have to be as paranoid.

If you have a patch which adds actual cert checking
and thus does not introduce regressions, please post it.


On Sat, May 26, 2018 at 6:38 PM,   wrote:

//config:   If you still think this is unacceptable, send patches.


That’s exactly what I did.
http://lists.busybox.net/pipermail/busybox/2018-May/086444.html

Jakub


On 2018-05-26 17:54, Denys Vlasenko wrote:

On Sat, May 26, 2018 at 5:39 PM,   wrote:

That's a crime against security!


Say what?


That’s a hyperbole. The thing is that when you don’t verify the peer’s
certificate, then you’re vulnerable to MitM attack with fake certificate
injection. The whole SSL/TLS is totally useless in that moment. It’s more
or
less like putting the door’s key under the carpet right in front of the
door.

Allowing to bypass/ignore certificate verification is ok-ish in some
situations, but only when the user do it consciously, using explicit
option
such as --no-check-certificate, not silently as the default option.


wget.c:

//config:   If you still think this is unacceptable, send patches.
//config:
//config:   If you still think this is unacceptable, do not want to
send
//config:   patches, but do want to waste bandwidth explaining how
wrong
//config:   it is, you will be ignored.

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox



___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] wget: don't silently ignore certificate validation

[Ralf Friedl]

Denys Vlasenko wrote:

wget should work for common use cases.
Such as downloading sources of kernels, gcc and such.
 From build scripts, not only by hand.
Without having to modify said scripts.
Your patch breaks that.
NAK.

I don't care that security people are upset.
They are paranoid, it's part of their profession.
It does not mean everybody else have to be as paranoid.

I must admit I'm surprised by this statement.
You add paranoid changes to programs like cp, unlinking the target in 
direct violation of POSIX, breaking some use cases. There was recent 
discussion about modifying the extraction of TAR and other archives, 
which introduced new problems and regressions.
While there is nothing wrong with being careful, busybox is mainly used 
on single user systems, so it is unlikely that there is another user to 
create race conditions to exploit.
On the other hand, not checking https means transfers could be attacked 
by someone anywhere on the network, not only a local user on the 
machine, so the number of potential attacked is much larger, and you 
don't even print a warning that the remote identity is not checked. You 
don't expect everybody to read the complete source code before using 
busybox, do you?

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] wget: don't silently ignore certificate validation

[Natanael Copa]

Denys,

Most common use case for https is to give some sort of guarantee that
you actually get what you think you get or that you get from who you
think you get it from. That is what most people expect when downloading
from https. If you don't care about verifying that, then the common use
case is to use http (without the 's').

Now, I think it is perfectly fine that some people does not care about
checking certificates, but in that case I think its reasonable to
explicitly tell that to wget. This is exactly what the GNU wget does
and this is what Jirutka's patch does. I am confident that this is what
the big majority would want from the tool.

Apparently there are strong opinions in both directions here what the
desired behavior should be, so I think it makes sense to have a config
option for this?

-nc


On Sat, 26 May 2018 19:34:05 +0200
Denys Vlasenko  wrote:

> wget should work for common use cases.
> Such as downloading sources of kernels, gcc and such.
> From build scripts, not only by hand.
> Without having to modify said scripts.
> Your patch breaks that.
> NAK.
> 
> I don't care that security people are upset.
> They are paranoid, it's part of their profession.
> It does not mean everybody else have to be as paranoid.
> 
> If you have a patch which adds actual cert checking
> and thus does not introduce regressions, please post it.
> 
> 
> On Sat, May 26, 2018 at 6:38 PM,   wrote:
> >> //config:   If you still think this is unacceptable, send patches.  
> >
> >
> > That*s exactly what I did.
> > http://lists.busybox.net/pipermail/busybox/2018-May/086444.html
> >
> > Jakub
> >
> >
> > On 2018-05-26 17:54, Denys Vlasenko wrote:  
> >>
> >> On Sat, May 26, 2018 at 5:39 PM,   wrote:  
> >
> > That's a crime against security!  
> 
> 
>  Say what?  
> >>>
> >>>
> >>> That*s a hyperbole. The thing is that when you don*t verify the peer*s
> >>> certificate, then you*re vulnerable to MitM attack with fake certificate
> >>> injection. The whole SSL/TLS is totally useless in that moment. It*s more
> >>> or
> >>> less like putting the door*s key under the carpet right in front of the
> >>> door.
> >>>
> >>> Allowing to bypass/ignore certificate verification is ok-ish in some
> >>> situations, but only when the user do it consciously, using explicit
> >>> option
> >>> such as --no-check-certificate, not silently as the default option.  
> >>
> >>
> >> wget.c:
> >>
> >> //config:   If you still think this is unacceptable, send patches.
> >> //config:
> >> //config:   If you still think this is unacceptable, do not want to
> >> send
> >> //config:   patches, but do want to waste bandwidth explaining how
> >> wrong
> >> //config:   it is, you will be ignored.  

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] wget: don't silently ignore certificate validation

[Eli Schwartz]

On 05/27/2018 11:58 AM, Eli Schwartz wrote:
> It's unacceptable that for something which you see as primarily useful
> in downloading very important source code, you simply don't care that
> the source code may be compromised by a MITMed attack.
> This is incredibly terrible logic, your cross-compiler is now infected
> with malicious code. The purpose of compiling code is *usually* to use
> it, which means that wherever you use that code, you're no longer in a
> QEMU sandbox, and whichever real box you use it on, can now say hello to
> unlimited arbitrary code execution.

By the way, *this would be perfectly fine* if it logged warnings when
you did it. Users would then know that they need to take extra
precautions, like validating PGP signatures.

Rather than assuming that the S in HTTPS actually means something to
busybox.

...

Speaking with my Arch Linux maintainer hat on, I'm disabling WGET_HTTPS
until a patch is released which adds said warnings.

I won't demand that busybox wget learn to *check* certificates, though
obviously I would appreciate such functionality.

-- 
Eli Schwartz
Bug Wrangler and Trusted User
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] wget: don't silently ignore certificate validation

[Eli Schwartz]

On 05/26/2018 01:34 PM, Denys Vlasenko wrote:
> wget should work for common use cases.
> Such as downloading sources of kernels, gcc and such.
> From build scripts, not only by hand.
> Without having to modify said scripts.
> Your patch breaks that.
> NAK.
> 
> I don't care that security people are upset.
> They are paranoid, it's part of their profession.
> It does not mean everybody else have to be as paranoid.
> 
> If you have a patch which adds actual cert checking
> and thus does not introduce regressions, please post it.

It's unacceptable that for something which you see as primarily useful
in downloading very important source code, you simply don't care that
the source code may be compromised by a MITMed attack.

> On Sat, May 26, 2018 at 6:38 PM,   wrote:
>>> //config:   If you still think this is unacceptable, send patches.
>>
>>
>> That’s exactly what I did.
>> http://lists.busybox.net/pipermail/busybox/2018-May/086444.html
>>
>> Jakub
>>
>>
>> On 2018-05-26 17:54, Denys Vlasenko wrote:
>>> On Sat, May 26, 2018 at 5:39 PM,   wrote:
>> That's a crime against security!
> Say what?


 That’s a hyperbole. The thing is that when you don’t verify the peer’s
 certificate, then you’re vulnerable to MitM attack with fake certificate
 injection. The whole SSL/TLS is totally useless in that moment. It’s more
 or
 less like putting the door’s key under the carpet right in front of the
 door.

 Allowing to bypass/ignore certificate verification is ok-ish in some
 situations, but only when the user do it consciously, using explicit
 option
 such as --no-check-certificate, not silently as the default option.

The justification for including HTTPS in the first place:
https://git.busybox.net/busybox/tree/networking/wget.c?id=8bc418f07eab79a9c8d26594629799f6157a9466#n74

"my small automatic tooling to build cross-compilers from sources no
longer works, I need to additionally keep a local copy of ~4 megabyte
source tarball of a SSL library and ~2 megabyte source of wget, need to
compile and built both before I can download anything. All this despite
the fact that the build is done in a QEMU sandbox on a machine with
absolutely nothing worth stealing, so I don't care if someone would go
to a lot  of trouble to intercept my HTTPS download to send me an
altered kernel tarball"

This is incredibly terrible logic, your cross-compiler is now infected
with malicious code. The purpose of compiling code is *usually* to use
it, which means that wherever you use that code, you're no longer in a
QEMU sandbox, and whichever real box you use it on, can now say hello to
unlimited arbitrary code execution.

-- 
Eli Schwartz
Bug Wrangler and Trusted User
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] wget: don't silently ignore certificate validation

[Kang-Che Sung]

On Fri, May 25, 2018 at 12:50 AM, Jakub Jirutka  wrote:
> Internal TLS code (FEATURE_WGET_HTTPS) does not implement validation
> of the server's certificate.  It is documented in the code, but not
> even mentioned in the --help message, so users typically don't know
> about this behaviour.  That's a crime against security!
>
> This patch changes this behaviour for the case when both
> FEATURE_WGET_LONG_OPTIONS and FEATURE_WGET_HTTPS are enabled - any
> attempt to open a TLS connection using internal TLS code (i.e. without
> certificate validation) ends with error, unless the user specified
> option "--no-check-certificate".
>

Jakub,

I wonder if you can revise your patch, so that it prints a warning that
certificate check is skipped instead of error-ing and quitting.
That way the patch might have a chance of getting accepted.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] wget: don't silently ignore certificate validation

[Kang-Che Sung]

On Sun, May 27, 2018 at 1:34 AM, Denys Vlasenko
 wrote:
> wget should work for common use cases.
> Such as downloading sources of kernels, gcc and such.
> From build scripts, not only by hand.
> Without having to modify said scripts.
> Your patch breaks that.
> NAK.
>
> I don't care that security people are upset.
> They are paranoid, it's part of their profession.
> It does not mean everybody else have to be as paranoid.
>
> If you have a patch which adds actual cert checking
> and thus does not introduce regressions, please post it.
>

I think I need to point out that in usability perspective, BusyBox's current
behaviour is not ideal. It should give a runtime warning that certificate
checks are skipped, instead of pass it silently. Of course, it would be better
if actual certificate check is implemented, but if builder disables it (for
binary size or simplicity), there should be a runtime warning so that usability
for secure people won't be compromised.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] wget: don't silently ignore certificate validation

[tiggersWelt.net (Support)]

Good evening Denys,

I agree with you that this patch is unacceptable, I also agree that
everyone who is complaining about the situation should send patches, but
I strongly disagree that it is valid to break security to keep "common
use cases" working. Using security-techniques like https should never
give a false impression of being secure while implementing it wrong.

I remember that I've missed HTTPS-Support in wget for years and was very
happy to see it being implemented back in 2014/2015. I've never had a
look at the code nor its documentation, because I had trust that busybox
is doing things right. I regret this was a personal mistake by myself.

Most encryption is worth nothing without authentication. If
authentication is skipped, encryption is broken by design and should not
be implemented at all. It's valid to add a parameter to switch off
authentication, but the default should verify that everything is
correct. If you think it's ok to skip authentication, we could also skip
checking bounds of a string because for common use cases there is
everything okay and no checks need to be applied.

To be constructive I've taken Jakub's Patch and rewrote it a bit to lead
to a small but fast improvement: Added verification-options to openssl
s_client helper and fail hard if the helper could not be spawned. This
may be switched off using "--no-check-certificate".

Internal wrapper and FTPS keep the old broken state and continue to work
without authentication while s_client-helper will fail whenever
verification fails. I remember that GNU wget also switched to this
behavior some years ago, so it should be suitable for any common case.
The broken state regarding the internal wrapper should be documented,
but I'm too tired to do this. The attached patch fixes this issue for me
and would restore some of my trust if being applied.


Kind regards,

Bernd


Am 26.05.2018 um 19:34 schrieb Denys Vlasenko:
> wget should work for common use cases.
> Such as downloading sources of kernels, gcc and such.
> From build scripts, not only by hand.
> Without having to modify said scripts.
> Your patch breaks that.
> NAK.
> 
> I don't care that security people are upset.
> They are paranoid, it's part of their profession.
> It does not mean everybody else have to be as paranoid.
> 
> If you have a patch which adds actual cert checking
> and thus does not introduce regressions, please post it.
> 
> 
> On Sat, May 26, 2018 at 6:38 PM,   wrote:
>>> //config:   If you still think this is unacceptable, send patches.
>>
>>
>> That’s exactly what I did.
>> http://lists.busybox.net/pipermail/busybox/2018-May/086444.html
>>
>> Jakub
>>
>>
>> On 2018-05-26 17:54, Denys Vlasenko wrote:
>>>
>>> On Sat, May 26, 2018 at 5:39 PM,   wrote:
>>
>> That's a crime against security!
>
>
> Say what?


 That’s a hyperbole. The thing is that when you don’t verify the peer’s
 certificate, then you’re vulnerable to MitM attack with fake certificate
 injection. The whole SSL/TLS is totally useless in that moment. It’s more
 or
 less like putting the door’s key under the carpet right in front of the
 door.

 Allowing to bypass/ignore certificate verification is ok-ish in some
 situations, but only when the user do it consciously, using explicit
 option
 such as --no-check-certificate, not silently as the default option.
>>>
>>>
>>> wget.c:
>>>
>>> //config:   If you still think this is unacceptable, send patches.
>>> //config:
>>> //config:   If you still think this is unacceptable, do not want to
>>> send
>>> //config:   patches, but do want to waste bandwidth explaining how
>>> wrong
>>> //config:   it is, you will be ignored.
> ___
> busybox mailing list
> busybox@busybox.net
> http://lists.busybox.net/mailman/listinfo/busybox
> 

-- 
\\\||///
  \\  - -  //
   (  @ @  )
-oOo--( )--oOo---
 tiggersWelt.net www.tiggersWelt.net
 Inhaber Bernd Holzmüller   i...@tiggerswelt.net
Büro: 07 11 / 550 425-90
 Marktstraße 57  Fax: 07 11 / 550 425-99
 70372 Stuttgart

 Impressum: https://tiggerswelt.net/impressum
 networking/wget.c | 54 +++---
 1 file changed, 43 insertions(+), 11 deletions(-)

diff --git a/networking/wget.c b/networking/wget.c
index 30c339244..a5306d3a5 100644
--- a/networking/wget.c
+++ b/networking/wget.c
@@ -136,18 +136,21 @@
 //usage:#define wget_full_usage "\n\n"
 //usage:   "Retrieve files via HTTP or FTP\n"
 //usage:	IF_FEATURE_WGET_LONG_OPTIONS(
-//usage: "\n	--spider	Only check URL existence: $? is 0 if exists"
+//usage: "\n	--spider		Only check URL existence: $? is 0 if exists"
+//usage:		IF_FEATURE_WGET_OPENSSL(
+//usage: "\n	

Re: [PATCH] wget: don't silently ignore certificate validation

[Denys Vlasenko]

wget should work for common use cases.
Such as downloading sources of kernels, gcc and such.
From build scripts, not only by hand.
Without having to modify said scripts.
Your patch breaks that.
NAK.

I don't care that security people are upset.
They are paranoid, it's part of their profession.
It does not mean everybody else have to be as paranoid.

If you have a patch which adds actual cert checking
and thus does not introduce regressions, please post it.


On Sat, May 26, 2018 at 6:38 PM,   wrote:
>> //config:   If you still think this is unacceptable, send patches.
>
>
> That’s exactly what I did.
> http://lists.busybox.net/pipermail/busybox/2018-May/086444.html
>
> Jakub
>
>
> On 2018-05-26 17:54, Denys Vlasenko wrote:
>>
>> On Sat, May 26, 2018 at 5:39 PM,   wrote:
>
> That's a crime against security!


 Say what?
>>>
>>>
>>> That’s a hyperbole. The thing is that when you don’t verify the peer’s
>>> certificate, then you’re vulnerable to MitM attack with fake certificate
>>> injection. The whole SSL/TLS is totally useless in that moment. It’s more
>>> or
>>> less like putting the door’s key under the carpet right in front of the
>>> door.
>>>
>>> Allowing to bypass/ignore certificate verification is ok-ish in some
>>> situations, but only when the user do it consciously, using explicit
>>> option
>>> such as --no-check-certificate, not silently as the default option.
>>
>>
>> wget.c:
>>
>> //config:   If you still think this is unacceptable, send patches.
>> //config:
>> //config:   If you still think this is unacceptable, do not want to
>> send
>> //config:   patches, but do want to waste bandwidth explaining how
>> wrong
>> //config:   it is, you will be ignored.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] wget: don't silently ignore certificate validation

[jakub]

//config:   If you still think this is unacceptable, send patches.


That’s exactly what I did. 
http://lists.busybox.net/pipermail/busybox/2018-May/086444.html


Jakub

On 2018-05-26 17:54, Denys Vlasenko wrote:

On Sat, May 26, 2018 at 5:39 PM,   wrote:

That's a crime against security!


Say what?


That’s a hyperbole. The thing is that when you don’t verify the peer’s
certificate, then you’re vulnerable to MitM attack with fake 
certificate
injection. The whole SSL/TLS is totally useless in that moment. It’s 
more or
less like putting the door’s key under the carpet right in front of 
the

door.

Allowing to bypass/ignore certificate verification is ok-ish in some
situations, but only when the user do it consciously, using explicit 
option

such as --no-check-certificate, not silently as the default option.


wget.c:

//config:   If you still think this is unacceptable, send patches.
//config:
//config:   If you still think this is unacceptable, do not want to 
send
//config:   patches, but do want to waste bandwidth explaining how 
wrong

//config:   it is, you will be ignored.

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] wget: don't silently ignore certificate validation

[Denys Vlasenko]

On Sat, May 26, 2018 at 5:39 PM,   wrote:
>>> That's a crime against security!
>>
>> Say what?
>
> That’s a hyperbole. The thing is that when you don’t verify the peer’s
> certificate, then you’re vulnerable to MitM attack with fake certificate
> injection. The whole SSL/TLS is totally useless in that moment. It’s more or
> less like putting the door’s key under the carpet right in front of the
> door.
>
> Allowing to bypass/ignore certificate verification is ok-ish in some
> situations, but only when the user do it consciously, using explicit option
> such as --no-check-certificate, not silently as the default option.

wget.c:

//config:   If you still think this is unacceptable, send patches.
//config:
//config:   If you still think this is unacceptable, do not want to send
//config:   patches, but do want to waste bandwidth explaining how wrong
//config:   it is, you will be ignored.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] wget: don't silently ignore certificate validation

[jakub]

That's a crime against security!


Say what?


That’s a hyperbole. The thing is that when you don’t verify the peer’s 
certificate, then you’re vulnerable to MitM attack with fake certificate 
injection. The whole SSL/TLS is totally useless in that moment. It’s 
more or less like putting the door’s key under the carpet right in front 
of the door.


Allowing to bypass/ignore certificate verification is ok-ish in some 
situations, but only when the user do it consciously, using explicit 
option such as --no-check-certificate, not silently as the default 
option.


Jakub

On 2018-05-25 14:19, Denys Vlasenko wrote:
On Thu, May 24, 2018 at 6:50 PM, Jakub Jirutka  
wrote:

Internal TLS code (FEATURE_WGET_HTTPS) does not implement validation
of the server's certificate.  It is documented in the code, but not
even mentioned in the --help message, so users typically don't know
about this behaviour.




That's a crime against security!


Say what?

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] wget: don't silently ignore certificate validation

[Denys Vlasenko]

On Thu, May 24, 2018 at 6:50 PM, Jakub Jirutka  wrote:
> Internal TLS code (FEATURE_WGET_HTTPS) does not implement validation
> of the server's certificate.  It is documented in the code, but not
> even mentioned in the --help message, so users typically don't know
> about this behaviour.


> That's a crime against security!

Say what?
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

[PATCH] wget: don't silently ignore certificate validation

[Jakub Jirutka]

Internal TLS code (FEATURE_WGET_HTTPS) does not implement validation
of the server's certificate.  It is documented in the code, but not
even mentioned in the --help message, so users typically don't know
about this behaviour.  That's a crime against security!

This patch changes this behaviour for the case when both
FEATURE_WGET_LONG_OPTIONS and FEATURE_WGET_HTTPS are enabled - any
attempt to open a TLS connection using internal TLS code (i.e. without
certificate validation) ends with error, unless the user specified
option "--no-check-certificate".
---
 networking/wget.c | 30 +++---
 1 file changed, 19 insertions(+), 11 deletions(-)

diff --git a/networking/wget.c b/networking/wget.c
index 30c339244..074de9184 100644
--- a/networking/wget.c
+++ b/networking/wget.c
@@ -136,18 +136,21 @@
 //usage:#define wget_full_usage "\n\n"
 //usage:   "Retrieve files via HTTP or FTP\n"
 //usage:   IF_FEATURE_WGET_LONG_OPTIONS(
-//usage: "\n   --spiderOnly check URL existence: $? is 0 if 
exists"
+//usage: "\n   --spiderOnly check URL existence: $? is 
0 if exists"
+//usage:   IF_FEATURE_WGET_HTTPS(
+//usage: "\n   --no-check-certificate  Don't validate the server's 
certificate"
+//usage:   )
 //usage:   )
-//usage: "\n   -c  Continue retrieval of aborted transfer"
-//usage: "\n   -q  Quiet"
-//usage: "\n   -P DIR  Save to DIR (default .)"
-//usage: "\n   -S  Show server response"
+//usage: "\n   -c  Continue retrieval of aborted 
transfer"
+//usage: "\n   -q  Quiet"
+//usage: "\n   -P DIR  Save to DIR (default .)"
+//usage: "\n   -S  Show server response"
 //usage:   IF_FEATURE_WGET_TIMEOUT(
-//usage: "\n   -T SEC  Network read timeout is SEC seconds"
+//usage: "\n   -T SEC  Network read timeout is SEC 
seconds"
 //usage:   )
-//usage: "\n   -O FILE Save to FILE ('-' for stdout)"
-//usage: "\n   -U STR  Use STR for User-Agent header"
-//usage: "\n   -Y on/off   Use proxy"
+//usage: "\n   -O FILE Save to FILE ('-' for stdout)"
+//usage: "\n   -U STR  Use STR for User-Agent header"
+//usage: "\n   -Y on/off   Use proxy"
 
 #include "libbb.h"
 
@@ -271,6 +274,7 @@ enum {
WGET_OPT_HEADER = (1 << 10) * ENABLE_FEATURE_WGET_LONG_OPTIONS,
WGET_OPT_POST_DATA  = (1 << 11) * ENABLE_FEATURE_WGET_LONG_OPTIONS,
WGET_OPT_SPIDER = (1 << 12) * ENABLE_FEATURE_WGET_LONG_OPTIONS,
+   WGET_OPT_NO_CHECK_CERT = (1 << 13) * ENABLE_FEATURE_WGET_LONG_OPTIONS,
 };
 
 enum {
@@ -714,6 +718,11 @@ static void spawn_ssl_client(const char *host, int 
network_fd, int flags)
int pid;
char *servername, *p;
 
+#if ENABLE_FEATURE_WGET_LONG_OPTIONS
+   if (!(option_mask32 & WGET_OPT_NO_CHECK_CERT))
+   bb_error_msg_and_die("unable to validate the server's 
certificate");
+#endif
+
servername = xstrdup(host);
p = strrchr(servername, ':');
if (p) *p = '\0';
@@ -1402,10 +1411,9 @@ IF_DESKTOP(  "tries\0"Required_argument 
"t")
"header\0"   Required_argument "\xff"
"post-data\0"Required_argument "\xfe"
"spider\0"   No_argument   "\xfd"
+   "no-check-certificate\0" No_argument   "\xfc"
/* Ignored (we always use PASV): */
 IF_DESKTOP("passive-ftp\0"  No_argument   "\xf0")
-   /* Ignored (we don't do ssl) */
-IF_DESKTOP("no-check-certificate\0" No_argument   "\xf0")
/* Ignored (we don't support caching) */
 IF_DESKTOP("no-cache\0" No_argument   "\xf0")
 IF_DESKTOP("no-verbose\0"   No_argument   "\xf0")
-- 
2.17.0

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox