So, if you serve a older VLC, with the correct signature, and you serve the old status file and the old status file signature, and that on the same year of the current version, then you could:
show a 'no upgrade' or
serve a "VLC downgrade".
But:
VLC does not auto-upgrade anyway, without user intervention, and
the downgrade will be warned during the installer...
Note that the old status files are not public, so you'd need to plan a long time before...
And it must be done the same way because of the validity of our privkey...
This is far from the "issue described above", or the usual HTTP shaming done on this tumblr...
Not to mention that seeing VLC uses GnuTLS, that is the most secure software ever, so using TLS would solve all the issues... (This part was irony, seeing that some people don't get it)
I think the Linux situation is quite different, as (at least in the case of Debian and other dpkg-based distros) the distribution's archive signing key is making a cryptographic assertion that "this is THE thing called 'vlc'" and they even have some protection against rollback attacks (in the form of Valid-Until lines in Release files).
If I understand Windows and Mac code signing correctly, the assertion being made for first-time installs there is merely "this is a binary signed by someone who paid us a little bit of money for a code-signing cert". It is not difficult (or unheard of) for attackers to buy certs from Microsoft and Apple.
Replying to [comment:7 jb]:
Btw https://get.videolan.org/ shows all the keys, the hash and the signatures under HTTPS, so you can check everything for your first time install...
I think the Linux situation is quite different, as (at least in the case of Debian and other dpkg-based distros) the distribution's archive signing key is making a cryptographic assertion that "this is THE thing called 'vlc'" and they even have some protection against rollback attacks (in the form of Valid-Until lines in Release files).
If I understand Windows and Mac code signing correctly, the assertion being made for first-time installs there is merely "this is a binary signed by someone who paid us a little bit of money for a code-signing cert". It is not difficult (or unheard of) for attackers to buy certs from Microsoft and Apple.
Replying to [comment:7 jb]:
Btw https://get.videolan.org/ shows all the keys, the hash and the signatures under HTTPS, so you can check everything for your first time install...
Sorry, I fail to see what your points have to do with this bugreport.
For first time install, we allow people to check if their downloads are genuine, with our own hashes+signature (served over HTTPS) + code-signing on OS X (soon Windows too).
But for real, making huge scary claims at least partially via Twitter without first endeavoring to understand the broader context/ecosystem in which HTTP-based updates may occur can end up taking up one of the most scarce resources we have as a community: sec-oriented dev time.
Also much love to the VLC team for being responsive/respectful, and to Morgan for fighting the good fight and raising what I agree is an important issue. I'd still much rather live in an HTTPS-only world.
I didn't mean to make huge and scary claims, but I'll have to fall on my sword and admit that tweets like - https://twitter.com/headhntr/status/500708383582732288 - (which I meant with humor, hence the caps and exclamations) can be unproductive and add to FUD. For that, I apologize.
I would like still TLS on the download though even if the package signing is perfect. Defense in depth and all that :)
If VLC doesn't solve all of the issues covered by TUF,
Sorry, but that statement is very vague.
I've made it clearer in my response below.
If you have a particular issue, please report it, but "SOLVE ALL THE ISSUES" (meme) is too vague to be useful
There are myraid of problems pointed out in the following papers:
A Look in the Mirror: Attacks on Package ManagersPackage Management SecuritySurvivable Key Compromise in Software Update Systems
It seems that the VLC security team should read each of the papers and to declare that the issues contained within are resolved by VLC. Anyone implementing an updater needs to understand all of the issues involved.
''
Rollback attacks. Attackers should not be able to trick clients into
installing software that is older than that which the client previously knew
to be available.
''
This is currently possible with VLC. This has been acknowledged in ticket:11987#comment:4, I think.
''Indefinite freeze attacks. Attackers should not be able respond to client
requests with the same, outdated metadata without the client being aware of
the problem.
''
This appears to be currently possible with VLC.
''Endless data attacks. Attackers should not be able to respond to client
requests with huge amounts of data (extremely large files) that interfere
with the client's system.
''
This appears to be currently possible with VLC.
''
Slow retrieval attacks. Attackers should not be able to prevent clients
from being aware of interference with receiving updates by responding to
client requests so slowly that automated updates never complete.''
This appears to be currently possible with VLC.
''Extraneous dependencies attacks. Attackers should not be able to cause
clients to download or install software dependencies that are not the
intended dependencies.
''
This appears to not apply during updates for VLC. That is good.
''Mix-and-match attacks. Attackers should not be able to trick clients into
using a combination of metadata that never existed together on the
repository at the same time.
''
This may be currently possible with VLC, I haven't looked in depth.
''Malicious repository mirrors should not be able to prevent updates from good
mirrors.''
This appears to be currently possible with VLC.
It is not possible to download VLC over TLS in the first place - why not have a single HTTPS mirror for those that care?
The certs for the HTTPS hosts with GnuPG signatures should be pinned in popular browsers (eg: Chrome, Firefox, etc). That is not a matter for updates but it is a related issue.
I'm sorry, but some described attacks (Slow retrieval attacks, Endless data attacks) are issues that are the case for all download system like most Linux Distributions, and that will not be fixed.
Mirrors are HTTP and will stay HTTP for a few obvious reasons. Moreover, they will install binaries, so there is no security issue. Moreover, downloads are never done automatically, without user intervention.
The 'Mix-and-match attack' is not possible because of status file signatures.
And the "Malicious repository" does not apply to VLC. Corrupted mirrors are removed from the pool.
So, only the "Rollback attack" and "Indefinite freeze attack" might be of concern here, and they could be fixed with using HTTPS on the update.videolan.org and carrying the hash of the file in the status file.
Finally, jumping and screaming on twitter, and spreading FUD, while most downloaders just use plain HTTP with no authentication, automatically, without doing the basic fact checking is extremely annoying.
Not to mention spending time on that, instead of taking down people like softonic, cnet, or all repackagers...
I'm sorry, but some described attacks (Slow retrieval attacks, Endless data attacks) are issues that are the case for all download system like most Linux Distributions, and that will not be fixed.
Do you see how TUF deals with this case?
A possible fix is to say that if VLC hasn't been able to contact the update server for a reasonable time frame, the user should be notified that they may need an update and that their network is preventing such an update. It is true that many distributions of software have this issue - that doesn't make it a non-issue, it makes it an epidemic with serious security concerns.
Mirrors are HTTP and will stay HTTP for a few obvious reasons.
Offering at least one HTTPS mirror is a reasonable compromise. If you have a bug in your HTTP client code, for example, every check to update is a chance to exploit. The same is true for the TLS stack. At the moment, VLC users get the attack surface of both.
I took a small peak at the HTTP code in VLC 2.0.3, as available in Debian GNU/Linux.
Do you feel that the HTTP code is bug free? The Gnutls stack is clearly not bug free but there is little to be done beyond sandboxing. The luahttp implementation seems promising but it is not used by everyone shipping VLC, is it?
Moreover, they will install binaries, so there is no security issue.
I don't follow - what do you mean?
Moreover, downloads are never done automatically, without user intervention.
That doesn't matter - if it is safe and secure, it can be automatic or not automatic. Consent of the user is important and Chrome shows us important numbers about security and safety in updating at risk software.
The 'Mix-and-match attack' is not possible because of status file signatures.
Could you explain that? Walk us through your analysis?
And the "Malicious repository" does not apply to VLC. Corrupted mirrors are removed from the pool.
An attacker can selectively serve hostile data to targets. Your detection of corrupted mirrors will only work when it isn't a competent attacker. Imagine the following: an attacker runs a mirror, fingerprints your corrupt mirror scanner, serves the correct content and serves bad (i.e.: old, exploitable) content to all others. This may also happen selectively with a MITM device for only a single target - this is a point in the favor of using SSL/TLS - make that attacker deploy a certificate that when it isn't in a correct chain, it is an error condition that may be reported.
So, only the "Rollback attack" and "Indefinite freeze attack" might be of concern here
Those are also concerns, agreed. The first, Rollback attacks, is perhaps the most worrying. Security issues in VLC are common and largely unavoidable. Bugs happen. This is why a secure installer and a secure updater is important.
The Indefinite freeze attack essentially allows for denial of updates which has a similar consequence.
and they could be fixed with using HTTPS on the update.videolan.org and carrying the hash of the file in the status file.
This is one part of one solution. I think this is the minimum required change but it also suggests deployment of HSTS and pinning in popular browsers. It is already the case that https://www.videolan.org/ is available - it would be useful if everyone went there by default and that browsers knew to request it rather than port 80.
Another is to have users securely install a stub from this location and then using HTTP with a TUF compliant updater isn't a problem. This is similar to what Mozilla is doing for their installer.
Another is to carry a full mirror on an HTTPS site for users who do not have GnuPG and to pin those certs in popular browsers. This may be done with Chrome easily. I believe that Mozilla will pick up those pinning settings as well.
We haven't discussed it but if the main signing key is compromised, it is also game over for VLC users. This is another aspect of TUF that is as of yet undiscussed.
Finally, jumping and screaming on twitter, and spreading FUD, while most downloaders just use plain HTTP with no authentication, automatically, without doing the basic fact checking is extremely annoying.
I'm not doing any such thing. If someone hadn't done it, I wouldn't have looked at VLC's update system and I wouldn't have found the valid issues. QED.
More specifically:
while most downloaders just use plain HTTP
What other options do they have? We cannot measure this fact and draw conclusions unless they have other options that are equally presented to them. It doesn't make sense - if VLC doesn't offer HTTPS downloads, users won't use it. The default matters most here and the default is insecurely offering HTTP mirrors for installation is it not?
Not to mention spending time on that, instead of taking down people like softonic, cnet, or all repackagers...
It isn't a waste of time to attract attention to these issues, nor would it be useful to overlook all of the issues raised by everyone - from Morgan to your replies. VLC is important and it needs our support.
Still, it is annoying and I hear you.
All of these things need to be done - thank you for writing Free Software and for improving it for everyone. The response here is largely positive and much better than most projects. There is no disrespect directed at yourself or at VLC from my side.
I decided to look at the actual update code from a order of operations perspective. Furthermore, I looked at the cryptographic protections of the updates. I found some issues but it isn't clear if they are serious. It depends on the threat model, I suppose. The update mechanism uses keys that are of an unsafe length. These are some rambling observations and I'm sure they're full of mistakes. Sorry in advance if this walk through the code is a waste of time for anyone....
In src/misc/update.c we see that stream_UrlNew() inside of GetUpdateFile() seems to parse version information before verifying signatures. The comment actually says: "Now that we know the status file is valid, we must download its signature to authenticate it." Huh? How can VLC know that it is valid, if it hasn't yet fetched the signature? That seems like a bug to me.
This is especially a problem because the HTTP GET request leaks the version number in the User-Agent. (e.g.: User-Agent: VLC/2.0.3 LibVLC/2.0.3) - so again, HTTPS would help with these kinds of issues. Another option is to remove the information leak in the User-Agent - both would be best of all.
Furthermore, looking through that code, I worry that GetUpdateFile() is unsafe. It fetches versions, then fetches signatures, then it compares the embedded public key with the key that signed the update. At this point, if the videolan_public_key_longid isn't the matched key, a new key, rather than the embedded key is fetched from the network ( in download_key() ) on the HTTP servers offered by VLC. These should be TLS enabled servers and they should have pinned certs. Defense in depth here would be a nice enhancement if anything in the following analysis is incorrect (eg: it isn't as safe as it looks).
Looking at download_key() I think that the code starting around line 215 is odd:
'''
uint8_t *p_hash = hash_from_public_key( p_new_pkey );
if( !p_hash )
{
msg_Err( p_update->p_libvlc, "Failed to hash signature" );
free( p_new_pkey );
FREENULL( p_update->p_pkey );
goto error;
}
'''
That error message seems incorrect. It failed to hash the new public key and not the signature, didn't it?
The verify_signature code block here seems suspect as it is actually a replace-old-key-with-new-key-fetched from the network - at the very least, it should be in a function and have a clearer name:
Looking at verify_signature(), specifically parse_public_key_packet() I see that it may allow for malformed public keys, why (/* some extra data eh ? */) is that? Or is the comment incorrect?
Additionally, the parse_public_key() function contains a call to pgp_unarmor(). The function pgp_unarmor() unpacks base 64 encoded data endlessly - it has no length or size restrictions that allows specifically crafted data to be resident in memory. That isn't a problem in itself but it may be useful to an attacker later.
After pgp_unarmor(), we see parse_public_key() walks through the packet types. If sets flags merely by comparing packet types but before verification of the content of the packet types. This feels like a bug.
Walking through the parse_public_key() switch statement and the two if conditions is interesting to say the least. We see that for the function to return VLC_SUCCESS, we need a few conditions to be true - we need a PUBLIC_KEY_FOUND or a USER_ID_FOUND to be true. That is an easy condition to meet because the USER_ID_PACKET is totally unauthenticated and of course, we can simply set one in a specially crafted packet. Shouldn't this code require both a PUBLIC_KEY_PACKET and a USER_ID_PACKET? Is there ever a case when it won't find both? And shouldn't they be known values (at least for the USER_ID_PACKET)?
Looking at the SIGNATURE_PACKET switch condition, if we have a null p_sig_issuer - I think we'll fall through to VLC_SUCCESS. If we have a p_sig_issuer, we'll fall into the body of the switch statement. This body effectively depends entirely on parse_signature_packet(). If it returns VLC_SUCCESS and the sig.version is 4, it will perform a memcmp, if the data in p_key->sig.issuer_logid matches the first 8 bytes of p_sig_issuer, it will set the flag to SIGNATURE_FOUND. If there are multiple SIGNATURE_PACKETS - say, one taken from a real key with a real signture, the flag will still be set. This feels mostly correct.
If we have a USER_ID_PACKET and a SIGNATURE_PACKET with the SIGNATURE_FOUND flag set, we'll return VLC_SUCCESS from this function.
In the error case for parse_public_key(), if the sig.version is 4, it will free data that may or may not exist. This depends on having a SIGNATURE_PACKET. A malformed public key could omit such a packet. What happens in that case? The code should catch a malformed public key as well.
Next we'll pop all the way back up the call tree to the place where download_key() was called. We'll have a key that isn't the key that we expected. That is a bug, isn't it? Shouldn't this fail before we verify the signature? This kind of quick check would be reasonable but probably isn't required.
From there - we'll hash_sha1_from_public_key( p_new_pkey ) and if the hash succeeds, which it will, we'll continue onward to verify_signature(). Inside of that we're now working with the incorrect key - but we've hashed it, so we just ignore which key we previously considered was the correct key and use the hash of the current key.
There is a check for p_hash[0] and p_hash[1] - this quick check seems fine but the error message should be clear that the quick check failed. It currently doesn't do that very clearly.
The actual signature verification code looks solid. I only looked at a high level but it seems like a reasonable implementation. If an attacker has handed you a malformed or a bad key that is not cross signed, it should fail. This is good news and ultimately means that the above issues don't matter much, if at at all.
Back to the bigger picture - older versions of VLC (e.g.: VLC 2.0.3) use a 1024bit DSA key. In vlc-master, I see extras/package/macosx/Resources/dsa_pub.pem which doesn't even parse with GnuPG (gpg: no valid OpenPGP data found.) and it is again, a 1024bit DSA key. Is this because VLC implemented their own signature verification code with libgcrypt? This isn't really GnuPG - it is a DSA signature scheme using libgcrypt with something resembling GnuPG ASCII armor.
This DSA key size makes freeze and roll back attacks a pressing issue. VLC should log any new keys that it sees in the wild. Right now, a key signed by the main VLC key could be in the wild and we'd never know it, nor would we catch it. It wouldn't be too tricky to exploit a VLC client with such a key but it again, requires at least a cross signed key.
I also found in include/vlc_pgpkey.h a differently formatted key that does parse with GnuPG:
'''
pub 1024D/E58D1ADC 2013-01-21 VideoLAN Release Signing Key (2013)
sig E58D1ADC 2013-01-21 [selfsig]
sig 59ED2987 2013-01-21 [User ID not found]
'''
This key doesn't expire but it is clearly indiciated that this key should be valid for only 2013. Shouldn't there be a new signing key for 2014?
If that key is compromised - through attack or computation - updates may be served to users. This key should be an RSA key and likely 2048bit at the very least, if not completely changed to ECC keys.
I see that the code has been updated allow for 3k DSA keys, why? why not say, large RSA or safe ECC keys?
All in all, I think the update code is as strong as that DSA key. In my very short read of the code, I didn't find anything completely awful - most things are as expected and they're not totally unreasonable. The overall process could use some thinking about defense in depth. The original bug reporter clearly didn't read any of this code and was spooked by usage of port 80. When considering the TUF spec, it is reasonable to be spooked. There are some issues but almost none of them are as bad as implied by the original report.
AFAIK Mac version of VLC is relying on Sparkle framework for updates. If you have any concerns about security of the update process, please file a bug at https://github.com/sparkle-project/Sparkle
I'm one of maintainers of the Sparkle framework.
Although Sparkle reads "appcasts" over plain HTTP and will download update archives over HTTP, it will NOT install any code without verifying its integrity and origin first. App developers can choose to use DSA signatures (host app verifies update archive against public key it has shipped with) or rely on Apple's Code Signing (integrity and authenticity check is done by the OS).
I'm pretty sure we could do better (e.g. have cryptographically-secure protection from downgrade attacks), so please help us improve Sparkle.
The original bug reporter clearly didn't read any of this code and was spooked by usage of port 80. When considering the TUF spec, it is reasonable to be spooked. There are some issues but almost none of them are as bad as implied by the original report.
Yep, that is sure...
So, there are issues:
the key is probably too weak, but we must keep this for older clients. So, if we use a new longer key, we need to point the update to a different URL, this is a work ongoing for 3.0.x.
you could block the updater, by blackholing the udpate.videolan.org website, and there is not much we can do about it, because you can disable the updates anyway or firewall VLC from updates.
Forcing a downgrade is not really possible, in the end, a contrario from 4 and 27:
if you serve an older status file and the older associated VLC, then the updater will refuse it, because it does version checking, so VLC will see the older status file, and authenticate it, and will refuse to do it.
if you keep the current status file, but serve an older version of VLC (by spoofing get or the associated mirror), the installer will warn about downgrading (and in older versions of VLC, will just not do it)...
Finally, we serve all the files and keys over HTTPS on get.videolan.org...
Therefore I very much dislike the way the original bug reporter screamed over twitter and tumblr, just without doing the minimum of checks and research...
The original bug reporter clearly didn't read any of this code and was spooked by usage of port 80. When considering the TUF spec, it is reasonable to be spooked. There are some issues but almost none of them are as bad as implied by the original report.
Yep, that is sure...
:-)
So, there are issues:
the key is probably too weak, but we must keep this for older clients. So, if we use a new longer key, we need to point the update to a different URL, this is a work ongoing for 3.0.x.
That isn't true is it? VLC only needs to ship a second longer key that is also valid and ensure that it is cross signed for older clients. I really suggest departing from DSA keys - either switch to ECC or use a larger RSA key (~2048bit is the smallest that is reasonable in 2014).
you could block the updater, by blackholing the udpate.videolan.org website, and there is not much we can do about it, because you can disable the updates anyway or firewall VLC from updates.
This is a case where VLC should alert the user and thus they can know that something is wrong.
Forcing a downgrade is not really possible, in the end, a contrario from 4 and 27:
if you serve an older status file and the older associated VLC, then the updater will refuse it, because it does version checking, so VLC will see the older status file, and authenticate it, and will refuse to do it.
Where is the version checking? I didn't see logic that prevents an upgrade based on version numbers as well as signatures - it looked like it just inspected signatures and did nothing else.
if you keep the current status file, but serve an older version of VLC (by spoofing get or the associated mirror), the installer will warn about downgrading (and in older versions of VLC, will just not do it)...
Ok, so a downgrade is possible, as I thought...
Finally, we serve all the files and keys over HTTPS on get.videolan.org...
These certs are not pinned in VLC - thus, a MITM can mess with this process. Please pin these certs. This blog post by Moxie lays out the issue well as does TACK.
Therefore I very much dislike the way the original bug reporter screamed over twitter and tumblr, just without doing the minimum of checks and research...
Sure - I think that is fair - though there are a few issues that have been uncovered. If someone opens bugs for each one, I bet they'll get some attention as well.
the key is probably too weak, but we must keep this for older clients. So, if we use a new longer key, we need to point the update to a different URL, this is a work ongoing for 3.0.x.
That isn't true is it? VLC only needs to ship a second longer key that is also valid and ensure that it is cross signed for older clients. I really suggest departing from DSA keys - either switch to ECC or use a larger RSA key (~2048bit is the smallest that is reasonable in 2014).
I'm not sure that is doable. But patches and testing is welcome.
And sorry, but is our key broken?
you could block the updater, by blackholing the udpate.videolan.org website, and there is not much we can do about it, because you can disable the updates anyway or firewall VLC from updates.
This is a case where VLC should alert the user and thus they can know that something is wrong.
No way. A lot of corporate users disable the updater, and we allow people to change the interval of the updates checks, at the first start. A lot of VLC are firewalled from accessing the internet.
I don't see how it's doable to alert the user.
Forcing a downgrade is not really possible, in the end, a contrario from 4 and 27:
if you serve an older status file and the older associated VLC, then the updater will refuse it, because it does version checking, so VLC will see the older status file, and authenticate it, and will refuse to do it.
Where is the version checking? I didn't see logic that prevents an upgrade based on version numbers as well as signatures - it looked like it just inspected signatures and did nothing else.
In the core and in the UI.
if you keep the current status file, but serve an older version of VLC (by spoofing get or the associated mirror), the installer will warn about downgrading (and in older versions of VLC, will just not do it)...
Ok, so a downgrade is possible, as I thought...
But the updater will warn you about that.
This is very far from the posts are complaining about.
Finally, we serve all the files and keys over HTTPS on get.videolan.org...
These certs are not pinned in VLC - thus, a MITM can mess with this process. Please pin these certs. This blog post by Moxie lays out the issue well as does TACK.
Therefore I very much dislike the way the original bug reporter screamed over twitter and tumblr, just without doing the minimum of checks and research...
Sure - I think that is fair - though there are a few issues that have been uncovered. If someone opens bugs for each one, I bet they'll get some attention as well.
Sorry, which security issues:
the fact that DSA 1024 is a bit weak?
the fact that you can block the update check?
the fact that you could make people download an older version (of the same year), but that will be blocked at the installer anyway?
I'm sorry, but those are "nice to have", if you want, but very far from the big security issues claimed... So yes, this bug is correctly closed as "invalid". If you have specific issues, please open a bugreport.
Personally, seeing the tone of the OP and the discussion on twitter and on tumblr, I'm not interested in working on those... Patches are welcome.
Not to mention that VLC is a media player and the number of security exploits are very likely numerous...
DISCLAIMER: I am not volunteering to implement that.
Since this came up again... With the benefit of hindsight and age, what I would do:
Build the manifest URL such that it contains:
the operating system (as it already does),
the major version of the operating system - to drop support for old OS versions cleanly,
the architecture (as it already does),
an incremental number for the update system revision - to force incremental updates, notably when changing the update system itself.
Nowadays, the manifest could probably be served over HTTP/TLS, though that does not really achieve anything other than silencing the trollz.
Put in the manifest:
the version number (already there),
some description (already there),
the download URL (already there),
a secure hash of the download (e.g. SHA-512) or even two (i.e. two different hash algorithms),
the signature of the manifest (currently a separate file).
Using GPG format may be overkill; we really just need to carry the digital signature.
We need to check the update version number before we start the download. I don't know if it's currently done or not. Some comments imply that it is not.
The download itself would no longer need to be signed separately, since we can trust the hash(es) within the manifest.
EDIT: Obviously, it is assumed that the updater would check the hash before running the installer.
Hoping to restart this discussion based on the recent Sparkle vulnerabilities (https://vulnsec.com/2016/osx-apps-vulnerabilities/) and the fact that VLC is still using http:// for its update host, making older versions without the recent patch vulnerable to remote code execution.
The vulnerability is not related to signing of binaries. It's in XML parsing and handling of release notes URL redirects, which are not protected by Sparkle's code signing.
You really, really need to upgrade Sparkle to 1.13.1 ASAP.