[SSL Observatory] Diginotar broken arrow as a tour-de-force of PKI fail

[ArkanoiD]

On Wed, Sep 07, 2011 at 02:38:57PM +0200, Jacob Appelbaum wrote:
> That is an interesting project!
> 
> I see that you are really trying and I respect that a great deal. I
> simply don't think that this is a safe path to take. The project you
> linked reminds me of a free software version of say, a bluecoat
> proxy/filter system. I don't think that we should design end user
> software to fail nicely for proxy/filters as the user will never be able
> to tell the difference between you and another local attacker.

Unlike for a local attacker, end user is expected to import my root to use
the system.

Here is old whitepaper about problems I address with this software:

http://sourceforge.net/projects/openfwtk/files/benevolent-ssl-mitm.pdf/download

> 
> I looked at the SSL stuff just to see what you're doing and I'm not sure
> that it disproves my main concern.
> 
> Here's an example - please correct me if I'm mistaken. I've never seen
> your project before and so I'm just reading through it to try to
> understand it.
> 
> What happens when a remote peer certificate is valid and also gets
> passed to pathfinder_verify(cert, buf) from
> openfwtk-2.1pre10/libssl/sslfunc.c at the momenet?
> 
> It looks like this happens:
> 
>  117 ??·size = i2d_X509(cert, NULL);
>  118 ??·iend = keybuf = malloc(size);
>  119 ??·certdata_str = malloc(size * 2 + 1);
>  120 ??·i2d_X509(cert, &iend);
>  121 ??·cp = keybuf;
> 
> I'm not convinced that this is safe at all.

Cannot believe my eyes, I just copypasted this function from pathfinder
examples. You are obviously right, I should xmalloc instead of malloc
and check the return values. It slipped somehow from my attention, will fix.

> 
> In fact, I'd bet that if i2d_X509 returns negative as it does in the
> event it has an error, you'll have *that* value cast into size. It will
> be cast from signed to unsigned as size is a size_t. Malloc may not take
> kindly to the result of such a cast. It may return NULL or a very large
> chuck of memory. In Linux this won't matter until you use it. As a
> result, in the first case it may not be a big deal. However, in the
> second malloc it is possible that (size * 2 + 1) will wrap and become a
> much smaller buffer than is required for later use in the function.
> 
> That makes this look a lot less safe:
> 
>  122 ??·certdata_str_i = certdata_str;
>  123 ??·while (cp < iend) {
>  124 ??·??·unsigned char ch = *cp++;
>  125 ??·??·*certdata_str_i++ = hex[( ch >> 4 ) & 0xf];
>  126 ??·??·*certdata_str_i++ = hex[ch & 0xf];
>  127 ??·}
>  128 ??·*certdata_str_i = 0;
> 
> If size is NULL to start, I think setting *certdata_str_i = 0 is
> probably not safe. It should seg fault on the spot. So perhaps that is
> safe? I don't think so but it's not arbitrary code execution. If it
> doesn't, I wonder what pathfinder_dbus does if it gets a NULL pointer?

Reports an error, there is a check in place.

> If the result of the malloc for certdata_str after the (size * 2 + 1) is
> not actually twice the size plus one of keybuf, I'd guess that all bets
> are off. The program seems like it will write hex values into
> *certdata_str_i++ all the way past the real length for certdata_str_i.
> 
> It may also be the case that a really large value will be written into
> certdata_str_i - What happens if a giant string is passed to
> pathfinder_dbus?

It gets dumped to dbus. First impression is that nothing nasty happens there, but I will definitely add more sanity checks.

> 
> It seems most likely that if the cert will has an error parsing the data
> will simply be incorrect. Perhaps no one will ever reach this code path?
> 
> ...
> 
> I guess that a lot of MITM code will have similar issues. I'm sure that
> it *can* be done correctly but I'm not convinced it *should* be done at all.
> 
> I certainly don't think browsers should weaken their security promises
> to enable this kind of tampering as it also directly creates a few
> different kinds of risk.
> 
> There's risk from the MITM observer and there is risk *for* the MITM
> observer from vulnerabilities such as the above bugs. The incentive for
> the attacker will shift to compromising the proxy systems as they will
> become more valuable than all the CAs combined for a specific target.
> 
> I certainly want my browser to fail closed if it's passing through such
> a system.

Proxy systems are generally simple enough to resolve problems like that.
Shame on me I missed such an obvious thing, but it is still unclear if it really imposes new security risk -- this framework is designed to "deny on fail".

> 
> Why do we want to create this new risk as part of the security solution?
> I don't think we do. We already have enough risk as it is.
> 
> > 
> >>
> >>>> I have requested cert pinning in Chrome because if the wrong certs are
> >>>> presented, I want it to fail closed. I want users to avoid being MITM'ed
> >>>> by attackers regardless of their intentions or corporate environmental
> >>>> needs.
> >>>
> >>> Ah, then losing all network traffic control (it means just that: poke one hole and one is more than enough) is good for security. "Great".
> >>>
> >>
> >> I think you're already doomed. Do you suppose that you are able to
> >> detect all the covert channels in existence? Lots of covert channels
> >> work through such a proxy system. That's a laugh and a half to say the
> >> least.
> > 
> > Ah, then let's give up and open everything, set firewalls to pass all and so on. Great.
> > 
> 
> That's not what I'm saying. As I said above - I think what you're doing
> is interesting and you've clearly put a lot of effort into trying to
> secure the local network in the current situation.
> 
> Still, I'm saying something different: I'm saying lets be honest about
> where we stand and try to change the game entirely. The game we're
> currently playing won't be won in a way that leads to security for the
> end user as a general reality.
> 
> If you want to MITM everyone on your corporate network, I say go for it.
> I don't think it's reasonable to ask browsers to cripple their software
> to fall prey to such SSL/TLS MITM attacks. CAA/DANE/DNSSEC and other
> systems will make this more and more the normal behaviour and it *must*
> fail closed in the general case. If you want to block all of that
> security technology, I'd say that you're basically not providing
> internet access any more and you'll need to deploy systems that meet
> your local filternet rules.
> 
> The game needs decentralisation in the trust root badly and my
> decentralised trust root is almost certainly not my local network.
> 
> All the best,
> Jacob
> 
> email protected and scanned by AdvascanTM - keeping email useful - www.advascan.com 
> 
>