? ? ? ? Pending

User tests: Successful: Unsuccessful:

avatar zero-24
zero-24
15 May 2020

Summary of Changes

Introduce sitesecret / sitekey to Joomla 3.10 that replaces the secret value in 4.0

This PR here introduce two new global configuration values called sitesecret and sitekey.
In the past we had the problem that core and extensions used the secret for operations that could leak the secret to the public as they require an unique by site id.

So @SniperSister came up with the following proposal:

  • add 2 new configuration values to 3.x, named sitekey and sitesecret
  • provide document for developers to let them know that:
    1. the secret and JApplicaton::getHash is not secret in 3.x
    2. they should use sitekey for using a key that is unique and persistent per site but not secret
    3. they should use sitesecret for using a key that is unique and persistent per site and secret
  • add the new config values to 4.x too, remove $secret there and refactor the existing usages to either use sitesecret or sitekey, depending on the actual place used; for getHash, use sitekey

Based on that we worked on the core usage and came up with this list:

Usage oldValue newValue
getOtpConfigEncryptionKey (administrator/components/com_users/models/user.php) secret privatesalt
loadSession (installation/application/web.php) secret sitekey
__construct (libraries/legacy/simplecrypt/simplecrypt.php) secret sitekey
getHash (libraries/src/Application/ApplicationHelper.php) secret sitekey
initialiseApp (libraries/src/Application/SiteApplication.php) secret sitekey
loadSession (libraries/src/Application/WebApplication.php) secret sitekey
__construct (libraries/src/Cache/CacheStorage.php) secret sitekey
generateMediaVersion (libraries/src/Version.php) secret sitekey
getFormToken (libraries/vendor/joomla/application/src/AbstractWebApplication.php) secret sitekey

This PR here implements the migration path form 3.x to 3.10 and to not break existing sites keep the two values the same for upgraded sites. New sites generate two independed values already.

Testing Instructions

First testcase

  • install joomla with this patches applyed
  • make sure the install runs good
  • check the configuration.php that the new sitesecret and sitekey is there and use different values
  • make sure we still have a secret value in the configuration.php
  • make sure you still can enable and setup 2fa

Seccond tescase

  • install 3.9.x
  • configure 2fa
  • apply the changes via an update package
  • check the configuration.php and make sure there are now the sitesecret and sitekey values that are equal to the secret value
  • make sure 2fa still works

Expected result

We have a real secret value for the CMS and extension devs to rely on ;)

Actual result

We have a secret that might not be that secret that we think it is.

Documentation Changes Required

As mention above docs about that stuff are required as well as adding it to the 4.0 BC breaks doc page.

to-do

  • change md5 to HMAC-MD5 where nesessarry
  • only copy secret to sitekey
  • secret maps to sitekey through Factory::getConfig()
    -- [ ] add a deprecated notice when the mapping happens.
    -- [ ] add the link to the docs to the deprecation message
  • generate a new value called privatesalt on upgrade
  • use sitekey for the crypt class Crypt
  • write docs that explain the correct usage of sitekey / secret and privatesalt
avatar zero-24 zero-24 - open - 15 May 2020
avatar zero-24 zero-24 - change - 15 May 2020
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 15 May 2020
Category Administration com_admin com_users Installation Libraries External Library Composer Change Unit Tests
avatar brianteeman
brianteeman - comment - 15 May 2020

The issue is not if the key is secret but if it is unique. If I copy a site from one domain to another does the key change? If it does not change then it is not unique and we are no different to how we are today.

avatar zero-24
zero-24 - comment - 15 May 2020

The issue is not if the key is secret but if it is unique. If I copy a site from one domain to another does the key change? If it does not change then it is not unique and we are no different to how we are today.

Well I have to disagree. We have both issues that secret to be unique and secret. ;)

That unique issue is something outside of the control of Joomla and your backup / restore component has to take care of that and as far as i know already does that.

The second part of being a real secret is the thing we are going to implement here.

avatar brianteeman
brianteeman - comment - 15 May 2020

That unique issue is something outside of the control of Joomla and your backup restore component has to take care of that and as far as i know already does that already.

Akeeba does that and I dont know if other backup software does. A simple copy certainly doesnt ;)

The second part of being a real secret is the thing we are going to implement here.

If it is not unique then it is not secret

avatar zero-24 zero-24 - change - 15 May 2020
Labels Added: ? ? ? ?
avatar zero-24
zero-24 - comment - 15 May 2020

If it is not unique then it is not secret

agree. But we make sure here that you can use a unique value that not has to be secret for things where you just need something to be unique.

Akeeba does that and I dont know if other backup software does. A simple copy certainly doesnt ;)

When they don't it is a bug. Same is true for a manual copy.

avatar brianteeman
brianteeman - comment - 15 May 2020

It is not a bug if it has never been communicated that it should be unique. If this PR doesnt ensure that it is unique then I fail to see how it is any improvement

avatar brianteeman
brianteeman - comment - 15 May 2020

Final comment is that I am massively against deprecating something in a patch release and removing it immediately in the next release. That is guaranteed to break any site that is using the secret

avatar zero-24
zero-24 - comment - 15 May 2020

It is not a bug if it has never been communicated that it should be unique

As mention above this is the intention to be documented to make sure we don't repeat the issue from the past.

If this PR doesnt ensure that it is unique then I fail to see how it is any improvement

Lets turn the question do you have any suggestions how we could do that?

avatar zero-24
zero-24 - comment - 15 May 2020

Final comment is that I am massively against deprecating something in a patch release and removing it immediately in the next release. That is guaranteed to break any site that is using the secret

It will be deprecated in a minor release 3.10.0 not with a patch release and will be removed and mention on the 4.0 B/C break lists like all other b/c breaks we did?

avatar brianteeman
brianteeman - comment - 15 May 2020

Final comment is that I am massively against deprecating something in a patch release and removing it immediately in the next release. That is guaranteed to break any site that is using the secret

It will be deprecated in a minor release 3.10.0 not with a patch release and will be removed and mention on the 4.0 B/C break lists like all other b/c breaks we did?

Which are both released on the same day

avatar brianteeman
brianteeman - comment - 15 May 2020

If this PR doesnt ensure that it is unique then I fail to see how it is any improvement

Lets turn the question do you have any suggestions how we could do that?

I dont but I am yet to understand the reason for changing it. I thought I did but if it is not unique and can be copied from one site to another I fail to see the improvement.

Maybe @nikosdion can explain to me

avatar zero-24
zero-24 - comment - 15 May 2020

Which are both released on the same day

Sure that is the reason we document all the things in the B/C breaking docs page so everyone who wants to support 4.0 can take a look and patch the code.

The great thing is that you can patch the code and this could would run in 3.10 and 4.0 :)

So it is up to the developer that claims to support 4.0 that he correctly uses the new secret like all the other stuff we removed in 4.0

avatar zero-24
zero-24 - comment - 15 May 2020

I dont but I am yet to understand the reason for changing it. I thought I did but if it is not unique and can be copied from one site to another I fail to see the improvement.

And I tried to explain why I think this is something we can not do anything from the core perspective about that.

Maybe @nikosdion can explain to me

Would be awesome to get your thoughts on that @nikosdion for sure.

avatar nikosdion
nikosdion - comment - 15 May 2020

The secret key is meant to be a unique value that is different on every site installation (even if the same site is cloned). That's how it was documented at least since Joomla 1.5 and that is the context that 3PDs use it for. The only valid case to keep the same secret is when you move the site from one host to another, keeping the same domain name.

Akeeba Backup does change the secret every time a site is restored, no matter where it's restored. It does update the Two Factor Authentication encryption when it changes the secret to prevent getting in the way of the user logging into the restored site.

Does changing the site secret break things? Yes, and it should! Even Akeeba Backup's own configuration is encrypted by default with the site secret. Change it on restoration and your settings are reset. This is a good security policy for obvious reasons – especially if your backups are set up to not include the configuration.php file to begin with. If you absolutely need to keep that data then of course you can reintroduce the old secret in your configuration.php. Whether that makes sense is for the site owner to decide.

In any case, what you are proposing is wrong on every conceivable level!

First of all, you are treating sitesecret as an immutable, persisting value which can be shared among several installation of the same site. This can cause security cock ups. Unless the intention for that is to replace the current secret in which case renaming is just asking for trouble and serves no real purpose – more on that later.

Second, your plan to address the core using the secret in insecure ways is to... let the core still use a key in an insecure way?! This sounds backwards. I disagree that a unique, per site value is not meant to be secret. I also disagree with your saying that getHash is not meant to be secure since it description literally reads "Provides a secure hash based on a seed". Of course it doesn't deliver on its promise since it does a stupid md5 hash of the seed and the secret instead of using an HMAC function. Therefore it promises something it's not and now you're telling 3PDs it's their fault for trusting Joomla core. Pathetic! As for loadSession, it suffers from the same affliction and its result is important for the site's security since it's used for the cookie name.

There are simple and effective ways to prevent the secret from leaking starting with the simple fact of never ever using the actual secret itself, especially not in an MD5 hash. Instead of doing md5($someCrapICanPossiblyGuess . $config->get('secret')) you should instead be doing hash_hmac('md5', $someCrapICanPossiblyGuess, $config->get('secret')). Before you start raising hell about using md5 in the HMAC do note that 1. you can easily use a different algorithm and 2. current security research has proven that in the context of an HMAC even md5 is secure enough for the foreseeable future. But I agree that using md5 is kinda lame – the only reason I am using md5 in my example is that the length remains the same, it's just that it makes it practically impossible to derive the secret from the HMAC in contrast with what happens with a plain old MD5 hash.

Third problem, your solution – even if it made sense from a technical and security perspective (which it doesn't!) – is a b/c break with extremely serious repercussions to existing software i.e. people upgrading from Joomla 3 to 4. There's data that's already been encrypted or hashed using the existing secret. By removing this key and reintroducing it as sitesecret with a new value you are making all that information inaccessible. I will list just off the top of my head where I am encrypting data using the site's secret:

  • Akeeba Backup configuration
  • Akeeba Ticket System, manager notes
  • Akeeba LoginGuard, all Two Step Verification options
  • and more which I'd need to spend 1-2 weeks to go through and address.
    This is not just a b/c break. It's a HARD break which completely eliminates data stored in the database. It's permanent data loss. That's unacceptable. If you are screwing over developers who used a core feature in the security context it's provided and documented then you should shut up and no longer complain when every 3PD starts rolling their own crypto, cookie storage or whatnot because the Joomla core is untrustworthy. If you break user and 3PD trust to the core you are creating a dangerous and insecure ecosystem, like WordPress.

Fourth and most important. You are drilling a hole in the water. You have two major problems you want to solve. 1. The core is using the secret in unsafe ways and 2. 3PDs are using the secret in unsafe ways. I already explained the first problem in my second reason why your solution is wrong. Now let's see the thing about 3PDs. The underlying issue is not a technical one, it's one of education and developer competence (and I will raise my hand first as the idiot 3PD who was leaking the site secret by using it unsafely until 8 years ago). What makes you believe that documenting the two new keys will have a different outcome than documenting how secret should be used? In the end of the day the same 3PDs who use secret unsafely will use either of the two new keys unsafely. You can't prevent them from using sitesecret unsafely e.g. for a media version query.

In short you are not solving any problem but you are introducing major new ones which will undermine the effort to upgrade sites to Joomla 4, creating a lot of development and support load for 3PDs who will be balling you for the objectively pointless decision to create two new keys, effectively breaking existing data already stored in the database.

Better approach:

  • Document how secret should be used WITH EXAMPLES. Don't expect developers to understand the subtle but massive difference between HMAC-MD5 and an MD5 hash.
  • Update the core to use the secret safely.
  • As a rule, do not allow core contributions which use secret unsafely. If you spot something like that point the developer to the documentation page of secret so they can use it safely.

This at least solves the problem of the core doing stupid things with the site secret. It doesn't solve 3PDs doing stupid things with it nor can you ever fully solve that. You can, however, have the JED unpublished their extensions if you get reports that point to a 3PD extension leaking secrets by using them unsafely AND point the developer to the documentation page that has actual examples of using the site secret safely. That's how you help make Joomla secure. Not by breaking it.

avatar zero-24
zero-24 - comment - 15 May 2020

Thanks for your feedback @nikosdion !

Akeeba Backup does change the secret every time a site is restored, no matter where it's restored. It does update the Two Factor Authentication encryption when it changes the secret to prevent getting in the way of the user logging into the restored site.

What do you do about possible other extensions using the secret similiar to how the 2FA plugins use them? Do you fire a event or is this not an problem at all in the real world?

First of all, you are treating sitesecret as an immutable, persisting value which can be shared among several installation of the same site. This can cause security cock ups. Unless the intention for that is to replace the current secret in which case renaming is just asking for trouble and serves no real purpose – more on that later.

Sorry it seems i have expressed me wrong here lets please clear that up

sitesecret

  • unique per site / should never be shared
  • should be threated as secure / secret.
  • should only be used when neseserry for encryption stuff.
  • can be regenerated (i guess we need a process for that but your input from the real world would help a lot on this!)

sitesekey

  • unique per site / should never be shared
  • should NOT be threated as secure / secret.
  • should NEVER be used when nesseserry for encryption stuff.
  • can be regenerated (i guess we need a process for that but your input from the real world would help a lot on this!)

Second, your plan to address the core using the secret in insecure ways is to... let the core still use a key in an insecure way?! This sounds backwards. I disagree that a unique, per site value is not meant to be secret. I also disagree with your saying that getHash is not meant to be secure since it description literally reads "Provides a secure hash based on a seed". Of course it doesn't deliver on its promise since it does a stupid md5 hash of the seed and the secret instead of using an HMAC function. Therefore it promises something it's not and now you're telling 3PDs it's their fault for trusting Joomla core. Pathetic! As for loadSession, it suffers from the same affliction and its result is important for the site's security since it's used for the cookie name.

There is no intention to blame 3rd Partys. As you clearly state there are serveral places where the core has the exact same issue and for that reason we took that to get it fixed. I'm sorry when I offended you or other 3rd party developers by my words above.

Third problem, your solution – even if it made sense from a technical and security perspective (which it doesn't!) – is a b/c break with extremely serious repercussions to existing software i.e. people upgrading from Joomla 3 to 4. There's data that's already been encrypted or hashed using the existing secret. By removing this key and reintroducing it as sitesecret with a new value you are making all that information inaccessible.

Please take a look into the code proposed here. On Upgrades the value of the old secret is copied to the sitesecret and sitekey. And there is no plan right now to do any kind of changes to that values on upgrades. As we are aware of the issue you mention.

There are simple and effective ways to prevent the secret from leaking starting with the simple fact of never ever using the actual secret itself, especially not in an MD5 hash. Instead of doing md5($someCrapICanPossiblyGuess . $config->get('secret')) you should instead be doing hash_hmac('md5', $someCrapICanPossiblyGuess, $config->get('secret')). Before you start raising hell about using md5 in the HMAC do note that 1. you can easily use a different algorithm and 2. current security research has proven that in the context of an HMAC even md5 is secure enough for the foreseeable future. But I agree that using md5 is kinda lame – the only reason I am using md5 in my example is that the length remains the same, it's just that it makes it practically impossible to derive the secret from the HMAC in contrast with what happens with a plain old MD5 hash.

Agree and I'm sure that core and most of the 3rd party code can be moved into that direction but to start in this direction we have to start somewhere. And we would like to start with the things we can control and this is the core and the docs page.

In the end of the day the same 3PDs who use secret unsafely will use either of the two new keys unsafely. You can't prevent them from using sitesecret unsafely e.g. for a media version query.

Well I totally get your point but without the seccond variable the 3rd party did not had a chance to use a different variable than secret for such things other than doing its own stuff. With that seccond config option in place we allow the 3rd party developers to do a restart as with the support of 4.0 they have to choose which variable they want to use and I can here only speak for myself and when one thing is deprecated / removed and replaced by something else I'm happy for any kind of documentation to make a informed chooise. Well i agree we can't fix stupid thats right but we can provide the docs so an good developer can take a look into them and use them the right way. And with this PR here we are doing it for the core so we lead by example.

In short you are not solving any problem but you are introducing major new ones which will undermine the effort to upgrade sites to Joomla 4, creating a lot of development and support load for 3PDs who will be balling you for the objectively pointless decision to create two new keys, effectively breaking existing data already stored in the database.

As explained above this is not the case. And upgraded sites will still hold the secret value only new sites will not have the old secret starting from 4.0.

Document how secret should be used WITH EXAMPLES. Don't expect developers to understand the subtle but massive difference between HMAC-MD5 and an MD5 hash.

Thanks for the feedback! Can you share some examples so we can include them in the initial version of the doc page. I would be happy to send you an proposal on the docs page when ready so we get it right in the beginng.

Update the core to use the secret safely.

This is what is on the to-do list too but out-of scope for this PR.

As a rule, do not allow core contributions which use secret unsafely. If you spot something like that point the developer to the documentation page of secret so they can use it safely.

Sure makes sense.

This at least solves the problem of the core doing stupid things with the site secret. It doesn't solve 3PDs doing stupid things with it nor can you ever fully solve that. You can, however, have the JED unpublished their extensions if you get reports that point to a 3PD extension leaking secrets by using them unsafely AND point the developer to the documentation page that has actual examples of using the site secret safely.

Will check with the JED and VEL Team. I'm sure this makes sense to be implemented when this is not already the case.

That's how you help make Joomla secure. Not by breaking it.

Yes that is the aim as explained above no site or encryption is breaking because of this change :)

Thanks again for your long and detailed feedback on this.

avatar brianteeman
brianteeman - comment - 15 May 2020

Please can you correct the title as I have tried 4 times to apply this patch to joomla 4 ;)

avatar nikosdion
nikosdion - comment - 15 May 2020

What do you do about possible other extensions using the secret similiar to how the 2FA plugins use them? Do you fire a event or is this not an problem at all in the real world?

There is no event, nor can there be one since it'd require access to the old secret after the site is fully restored.

The reason we replace 2FA is that if we didn't the user would be unable to log in at all. If the user logs in and finds that some of their extensions lost important data due to the secret change they can at worst redo the restoration and at the end of it change the secret back to its original value and remove 2FA through the database. An option to not change the secret can be provided, the only reluctance here owning to the fact that it can be abused. I've done a few experiments on making this kind of options more "scary" so definitely something to pursue.

Well I totally get your point but without the seccond variable the 3rd party did not had a chance to use a different variable than secret for such things other than doing its own stuff.

I disagree with your assessment that having one secret key is bad per se. There is no such thing as a "bad" secret, there's only such thing a bad use of the secret.

And with this PR here we are doing it for the core so we lead by example.

This is my most important objection. We are STILL setting a BAD EXAMPLE in the core, the same bad example we were setting since at least Joomla 1.5 i.e. "concatenate the secret with random crap and call it secure". This is patently wrong. It made sense 13 years ago but not anymore. The core should be leading by example by using HMAC-MD5. Otherwise how can we possibly tell 3PDs "you're doing it wrong" when Joomla itself is still doing it wrong?!

With that seccond config option in place we allow the 3rd party developers to do a restart as with the support of 4.0 they have to choose which variable they want to use

This also raises a point of objection. I would agree with that statement AS LONG AS only ONE new key was introduced, sitekey. This would indeed allow developers to migrate their data. However, your solution is to completely ditch the secret and replace it with a new key called sitesecret which has the same purpose of existence as how secret is mostly used today, has the same ambiguous word ("secret") in its name but the whole name is different, making it impossible to do a clean migration UNLESS a developer goes through every single instance of using secret (a really common word in software) and try to change it. This is bound to cause hard to detect bugs. There's no point doing this, especially when it's OK to use secret / sitesecret in insecure contexts per your admission.

In other words, renaming something for the sake of renaming it is a disservice to the developers and the Joomla community, especially when it comes with zero deprecation time.

As explained above this is not the case. And upgraded sites will still hold the secret value only new sites will not have the old secret starting from 4.0.

And exactly how is this better?! As a developer I can no longer do if (version_compare(JVERSION, '3.999.999', 'lt')). Instead, I have to try and read all three values from the configuration and figure out if the new keys are set. This adds even more complexity to what is already a complex change. If you are trying to maximise the number of bugs 3PDs will encounter then yeah, that's the way to do it. But it's a really stupid thing to do which will lead all 3PDs ignore the secret, site key, site secret or whatever and invent their own. Which of course has the immediate problem that these keys don't ever change when moving the site between servers. Which has security implications.

Actually, looking again at my code I did move away from using secret in my software. I only use secret when determining a media query. The reason was indeed that the secret wasn't so secret and I couldn't use it reliably to encrypt critical information. So ignore my previous reply about where I use the secret :)

Thanks for the feedback! Can you share some examples so we can include them in the initial version of the doc page. I would be happy to send you an proposal on the docs page when ready so we get it right in the beginng.

The absolute minimum of examples are the following:

  1. Media version queries. Don't just md5() the site secret. At worst md5() it with the version and release date of your software (example media version query service: https://github.com/akeeba/fof/blob/development/fof/Utils/MediaVersion.php). At best use HMAC-MD5 of the last modification time of a file that changes on every version (e.g. the extension's manifest) with the site's secret as the key.
  2. Create a hash, in general. Use HMAC-MD5 of your data with the secret as the key, not a straight up MD5 hash. In fact you can change CMSApplication::getHash to do that, therefore leading by example.
  3. Encrypting data. I'd need to look into the Crypt package before coming up with a recommendation for an example. At the very least, I would NOT use the site's secret as the key; it's unsuitable and an implementation detail that is non-obvious to the average developer. Use PBKDF2 key expansion, e.g. see https://github.com/akeeba/fof/blob/a0048808eced5ea094eb78bae97f59f27b651837/fof/Encrypt/Aes.php#L211

I could write the content. It's something I wanted to write a blog about anyway... but I ended up having to write a comments extension first so that I could have a functioning blog to begin with.

This is what is on the to-do list too but out-of scope for this PR.

I believe that a minimum of core changes should be an integral part of this PR. Otherwise we are giving the wrong impression that md5() is OK which is definitely not the case. Using HMAC-MD5 for the few places you already identified is a 2 hour work item, doesn't have a measurable performance impact and helps secure Joomla. It serves as a great example of WHY you are implementing two different keys.

Will check with the JED and VEL Team. I'm sure this makes sense to be implemented.

Thank you!

Yes that is the aim as explained above no site or encryption is breaking because of this change :)

I understand the intent, but I'm weary of the implementation. At the very least you should keep secret named as is and documented that it's insecure and should not be used to generate secure hashes or encrypt information; for these use cases sitekey should be used instead.

Thanks again for your long and detailed feedback on this.

No problem. I want Joomla to be more secure without unnecessarily breaking anything :)

Side note - Joomla 3

I would not introduce new keys for Joomla 3. Instead, I'd simply rig Factory::getConfig to set sitekey to the current secret. This way developers can possibly write code like this:

public function needsEncryptionUpgrade(): bool
{
  $app = Factory::getApplication();

  return $app->get('sitekey') == $app->get('secret');
}

public function getEncryptionKey(): string
{
  $app = Factory::getApplication();
  $siteKey = $app->get('sitekey');

  return $siteKey ?? $app->get('secret');
}

This is a more elegant solution than trying to read three values and figure out which two to use. Moreover, insecure contexts can still do $app->get('secret') without having to write gnarly, error prone code to deal with them across a multitude of Joomla 3.9 or lower (only secret is available), 3.10 (all available, but probably only use secret...?) and 4.0 (secret not available, use sitesecret instead) installations. Do remember that 3PDs will need to support multiple Joomla versions at the same time and they'll definitely need to support 3.9 and 3.10 until at least one year after 3.10 is EOL – that's July 2023 at the earliest.

avatar zero-24 zero-24 - change - 15 May 2020
Title
[4.0] Introduce sitesecret / sitekey to Joomla 3.10 that replaces the secret value in 4.0
[3.10] Introduce sitesecret / sitekey to Joomla 3.10 that replaces the secret value in 4.0
avatar zero-24 zero-24 - edited - 15 May 2020
avatar zero-24
zero-24 - comment - 15 May 2020

Please can you correct the title as I have tried 4 times to apply this patch to joomla 4 ;)

Sorry just done that :)

avatar zero-24
zero-24 - comment - 15 May 2020

The reason we replace 2FA is that if we didn't the user would be unable to log in at all. If the user logs in and finds that some of their extensions lost important data due to the secret change they can at worst redo the restoration and at the end of it change the secret back to its original value and remove 2FA through the database. An option to not change the secret can be provided, the only reluctance here owning to the fact that it can be abused. I've done a few experiments on making this kind of options more "scary" so definitely something to pursue.

Thanks for the insights.

I disagree with your assessment that having one secret key is bad per se. There is no such thing as a "bad" secret, there's only such thing a bad use of the secret.

The problem with that bad secret is that it might be used already bad in the code. With this change here we force everyone who wants to use the secret in 4.0 to take a decision whether using the sitesecret or using the sitekey.

This also raises a point of objection. I would agree with that statement AS LONG AS only ONE new key was introduced, sitekey. This would indeed allow developers to migrate their data. However, your solution is to completely ditch the secret and replace it with a new key called sitesecret which has the same purpose of existence as how secret is mostly used today, has the same ambiguous word ("secret") in its name but the whole name is different, making it impossible to do a clean migration UNLESS a developer goes through every single instance of using secret (a really common word in software) and try to change it. This is bound to cause hard to detect bugs.

Well i know the impact but this forces the dev to take that definitive (hopefully informed) decision. I have just looked up how it would affect the public versions of FoF (10 times the us of secret where only one time it is the secret from the config) and the core version of akeeba backup (version 7.1.2) (found 3 times the value 'secret' where only at two cases it is the actual secret)

That does not sound to be a that scary change to me? Do you know another extension that havily uses the secret? We might contact them upfont to get feedback on this change as you did?

There's no point doing this, especially when it's OK to use secret / sitesecret in insecure contexts per your admission.

just to be clear the sitesecret should be threated as secure / secret as mention above. The sitekey is intended to be used in insecure contexts.

And exactly how is this better?! As a developer I can no longer do if (version_compare(JVERSION, '3.999.999', 'lt')). Instead, I have to try and read all three values from the configuration and figure out if the new keys are set.

Why should you use that conditional in the first place?

Starting with 3.10+ compatibility you just have to use the sitescrete and sitekey and ignore the old secret even when it is set.

The old secret value is there only in case some not updated extension would sill use it on a upgraded site. Any checks for the secret in a newly installed 4.0.0 stable will fail as it is not there anymore.
And as on upgrades the value of the old secret is just copied to the both new keys it does not matter wich you use as developer on upgraded sites.

The absolute minimum of examples are the following:

Awesome thank you ?

I could write the content. It's something I wanted to write a blog about anyway... but I ended up having to write a comments extension first so that I could have a functioning blog to begin with.

Would be awesome. Feel free to send it to me via Mail (tobias.zulauf[at]cjo) when you have something so we don't duplicate the work on this.

I believe that a minimum of core changes should be an integral part of this PR. Otherwise we are giving the wrong impression that md5() is OK which is definitely not the case. Using HMAC-MD5 for the few places you already identified is a 2 hour work item, doesn't have a measurable performance impact and helps secure Joomla. It serves as a great example of WHY you are implementing two different keys.

Ok good argument. Will take a look into that and add it to the to-do list in the inital post.

I understand the intent, but I'm weary of the implementation. At the very least you should keep secret named as is and documented that it's insecure and should not be used to generate secure hashes or encrypt information; for these use cases sitekey should be used instead.

That is the plan no changes to the secret value nor documentation ?

I would not introduce new keys for Joomla 3. Instead, I'd simply rig Factory::getConfig to set sitekey to the current secret. This way developers can possibly write code like this:

Good Idea. That sounds like a good plan. And when someone hits save on the global config we even can persist the values without having to touch them on update time. Right?

Do remember that 3PDs will need to support multiple Joomla versions at the same time and they'll definitely need to support 3.9 and 3.10 until at least one year after 3.10 is EOL – that's July 2023 at the earliest.

Why should 3rd party be required to support 3.9 until at least one year after 3.10 is EOL? Everyone is free to do it for sure but i would be interested why that should be necessary? Supporting 3.10 and 4.0 at the same time with the same codebase for sure but that is possible as outlined here.

avatar zero-24 zero-24 - edited - 15 May 2020
avatar zero-24 zero-24 - change - 15 May 2020
The description was changed
avatar nikosdion
nikosdion - comment - 15 May 2020

The problem with that bad secret is that it might be used already bad in the code. With this change here we force everyone who wants to use the secret in 4.0 to take a decision whether using the sitesecret or using the sitekey.

OK. Let's hold that thought. You are trying to solve a problem by creating two separate secrets.

Let's forget for a minute that the decision won't be any more informed as it is now for the people who were abusing the secret and will do a search and replace for secret to sitesecret, carrying forward the same problem. I will address it further below (it is perfectly fixable). First I want to discuss this:

And as on upgrades the value of the old secret is just copied to the both new keys it does not matter wich you use as developer on upgraded sites.

Your problem is that the current secret could be abused and leak. So you want to create two separate configuration variables to solve that problem. But on upgrade you are just using the same value as the old secret in both places?! ? You are just perpetuating the same problem on upgraded sites, man!

Here's a better approach for upgrades. Copy the current secret to sitesecret. Create a new, random sitekey (I'd say at least 64 truly random characters). Map secret to sitesecret in Factory::getConfig. The developers who are using secret on 3.9 can still have their code work on 3.10. But they can also detect if sitekey and sitesecret are different (and sitekey is non-empty) even without a version check, therefore they know if they have to upgrade their stored data. And yes, we can communicate that clearly with a code example so people have all the time in the world (3.10 is a 2 year LTS) to deal with it.

just to be clear the sitesecret should be threated as secure / secret as mention above. The sitekey is intended to be used in insecure contexts.

I see and I can tell you why I was confused.

The new names are inscrutable. There is no intuitive difference between a secret and a key. Think about it. A secret is like a password that lets you in. A key is a mechanical implement that lets you in. Even if you talk cryptography – which most developers don't – a secret would be something you expand into a key. The words you chose are unfit for purpose.

Let's think about it a bit more. The secret isn't that secret, it's a unique salt used for hashes. Why not call it salt then? The difference between salt and sitekey is stark and obvious.

Starting with 3.10+ compatibility you just have to use the sitescrete and sitekey and ignore the old secret even when it is set.

That was not what you had told me up to this point. Reading your clarifications further below helped. Thank you!

Well i know the impact but this forces the dev to take that definitive (hopefully informed) decision. I have just looked up how it would affect the public versions of FoF (10 times the us of secret where only one time it is the secret from the config) and the core version of akeeba backup (version 7.1.2) (found 3 times the value 'secret' where only at two cases it is the actual secret)

I disagree about the informed decision part. Don't expect 3PDs to go hunting for a documentation page that's hard to find. Using better named variables, as I said above, makes much more sense. It would also help writing documentation with examples and linking to it from docblocks ;)

Regarding your searches, I disagree with you. You are seriously overestimating how much I use Joomla's secret ?

On FOF it's used in two places:

  • FOF Token. This is the original code that was later adapted and contributed to Joomla 4 as the Joomla API Token. The token is an HMAC-SHA1 of a seed stored in the database with the site's secret as the key. This would need to use sitekey now (something you missed about Joomla itself).
  • Media Version Query. It is the MD5 sum of the component's version, creation date and the site secret. This would need to use sitesecret. Not a big deal. In fact, I could use any site-specific data point to not leak the exact version number instead of using the secret. For example, I could use the file modification time of the extension's entry point file which is guaranteed to have a different timestamp on every site.

All other instances you found are irrelevant uses of the word "secret" e.g. the TOTP implementation or FOF's Encrypt service which uses a per-component secret that's stored in a file because Joomla's secret was considered untrustworthy for that purpose.

As for Akeeba Backup I don't use Joomla's secret anywhere except for the restoration. The settings are now encrypted using FOF's Encrypt service. The only change I'd need to do is in the restoration.

Seriously, though, I am not complaining about the work I have to do on my own software. I am abstracting Joomla a great deal if I'm not bypassing it altogether. What I'm trying to do here is help you avoid the pitfalls I fell for in the past. I even created more work for me, e.g. documentation and examples. So to answer your question, no, it's not a big deal for me. But if you don't implement this elegantly it can be a big deal for other 3PDs and users.

Why should 3rd party be required to support 3.9 until at least one year after 3.10 is EOL? Everyone is free to do it for sure but i would be interested why that should be necessary? Supporting 3.10 and 4.0 at the same time with the same codebase for sure but that is possible as outlined here.

THANK YOU FOR ASKING! For real. I've been saying things like that for 12 years and you are the first one to ask "why" instead of dismissing it with "nah, that doesn't make sense".

Let's break this down.

Minor versions (e.g. 3.9 to 3.10). I only need to provide support for approximately 6 months after they go EOL. There's an update lag because real world sites use a multitude of extensions from 3PDs which are not simultaneously ready for the new minor Joomla version. Comically, that's because Joomla introduces intentional or unintentional b/c breaks in minor releases – sometimes even in patch releases, see 3.7.1. When all extensions are available for the new version someone needs to sign off for the site upgrade fee and the site integrator needs to pull a copy of the site, upgrade all extensions to a version that supports the old and the new version of Joomla, then update Joomla itself and see if anything needs fixing. This means that if I drop support for the old J version as soon as the new one comes out I am doing a disservice to my clients who can no longer update without duplicating work (they'd have to update to an old version of my software, update Joomla, test everything, update to a new version of my software, test everything again).

Major versions (e.g. 1.5 to 1.6, 2.5 to 3, 3 to 4). The update lag is even bigger. It's typically 9 to 12 months. In most cases the old site was using an extension that's no longer available so there's the need of a partial reimplementation of the site.

To make matters worse, Joomla decided to EOL 3.10 two years after 4.0.0 is released. This means that I will need to support 3.10 until three years after 4.0.0 is released, i.e. late 2023 at the earliest. Think about that when you introduce any b/c breaks in 4.1, 4.2 etc because me and other extension developers will have to support 3.10 and 4.x at the same time.

Remember that 3PDs are not crazy people. We write software to help other people get work done. We enable and empower people. We have a social responsibility towards them. This means supporting old Joomla and PHP versions because we can't reasonably expect all users to update their sites the moment a new Joomla or PHP version is out.

Sorry for the long reply, here's a potato ?

avatar zero-24
zero-24 - comment - 16 May 2020

OK. Let's hold that thought. You are trying to solve a problem by creating two separate secrets.

Well one secret (used for encryption etc) and one unique but maybe public value. (used for media hashing)

Your problem is that the current secret could be abused and leak. So you want to create two separate configuration variables to solve that problem. But on upgrade you are just using the same value as the old secret in both places?! ? You are just perpetuating the same problem on upgraded sites, man!

Agree. Do you have a Idea how to fix it. The problem I see is that we break the sites when we silentyl change the values. We might can do the 2fa stuff but that would still break 3rd partys using the old values right?

Here's a better approach for upgrades. Copy the current secret to sitesecret. Create a new, random sitekey (I'd say at least 64 truly random characters). Map secret to sitesecret in Factory::getConfig. The developers who are using secret on 3.9 can still have their code work on 3.10. But they can also detect if sitekey and sitesecret are different (and sitekey is non-empty) even without a version check, therefore they know if they have to upgrade their stored data. And yes, we can communicate that clearly with a code example so people have all the time in the world (3.10 is a 2 year LTS) to deal with it.

With this patch here we change the form token generation to use the new sitekey so they would break right?

I see and I can tell you why I was confused.
The new names are inscrutable. There is no intuitive difference between a secret and a key. Think about it. A secret is like a password that lets you in. A key is a mechanical implement that lets you in. Even if you talk cryptography – which most developers don't – a secret would be something you expand into a key. The words you chose are unfit for purpose.
Let's think about it a bit more. The secret isn't that secret, it's a unique salt used for hashes. Why not call it salt then? The difference between salt and sitekey is stark and obvious.

Sounds good to me. What do you think about secretsalt and sitekey maybe so we make clear that the salt shoud be keept secret by its name?

That was not what you had told me up to this point. Reading your clarifications further below helped. Thank you!

?

I disagree about the informed decision part. Don't expect 3PDs to go hunting for a documentation page that's hard to find. Using better named variables, as I said above, makes much more sense. It would also help writing documentation with examples and linking to it from docblocks ;)

Good point. The examples part as explained above as well as linking the docs from the code makes totally sense. ?

All other instances you found are irrelevant uses of the word "secret" e.g. the TOTP implementation or FOF's Encrypt service which uses a per-component secret that's stored in a file because Joomla's secret was considered untrustworthy for that purpose.

Yes that is why i wrote.

versions of FoF (10 times the us of secret where only one time it is the secret from the config)

and

core version of akeeba backup (version 7.1.2) (found 3 times the value 'secret' where only at two cases it is the actual secret)

;)

Seriously, though, I am not complaining about the work I have to do on my own software. I am abstracting Joomla a great deal if I'm not bypassing it altogether. What I'm trying to do here is help you avoid the pitfalls I fell for in the past. I even created more work for me, e.g. documentation and examples. So to answer your question, no, it's not a big deal for me. But if you don't implement this elegantly it can be a big deal for other 3PDs and users.

I choose your software as i knew that you use the secret. I don't know what other extensions we can check. Do you have a suggestion? Maybe we can include this change here with examples and docs etc in a JED newsletter too to make everyone aware?

THANK YOU FOR ASKING! For real. I've been saying things like that for 12 years and you are the first one to ask "why" instead of dismissing it with "nah, that doesn't make sense".

Ok got your point. Makes sense to me. To get that right after that 6 months of Joomla 4 and 3.10 is out you can drop 3.9 support right?

Remember that 3PDs are not crazy people. We write software to help other people get work done. We enable and empower people. We have a social responsibility towards them. This means supporting old Joomla and PHP versions because we can't reasonably expect all users to update their sites the moment a new Joomla or PHP version is out.

I know that but that is also true for the developers of the core. We are no crazy people too. Our aim is to support as many users as possible and provide the extension devs a stable platform the build the extensions on. Yes we break stuff but we are humans too.

When more extension dev would run a quick test against the RC version we publish most of the b/c issues would be found beforehand and could be fixed before stable.

Sorry for the long reply, here's a potato ?

I'm very happy to get such long replies from people having more real world experience than me.

So thank you for such long replies!

avatar nikosdion
nikosdion - comment - 16 May 2020

Agree. Do you have a Idea how to fix it. The problem I see is that we break the sites when we silentyl change the values. We might can do the 2fa stuff but that would still break 3rd partys using the old values right?

I already told you :) The old secret is copied over to sitekey. Moreover, secret maps to sitekey through Factory::getConfig(). Therefore nothing breaks. We preserve b/c in version 3 and add forward compatibility to Joomla 4.

A new, securely random sitesecret is generated on upgrade. This is used for 2FA.

As for Crypt, I'd recommend the constructor using sitekey in J3 to preserve b/c. If the developer wants to make this forward-compatible with J4 the can instantiate it with $crypt = new Crypt(null, Factory::getApplication()->get('sitekey')) instead. This will work on J3.10 and J4.0. Add an @deprecated notice in the DocBlock to let developers know that in J4.0 you'll be instantiating Crypt with sitekey by default so they can figure it out. Link to the documentation page of this change in the DocBlock.

With this patch here we change the form token generation to use the new sitekey so they would break right?

I don't see why they'd break. The form token is something temporary, not something you store anywhere. At worst the user would have to log out and back in. Do you have something different in mind?

Sounds good to me. What do you think about secretsalt and sitekey maybe so we make clear that the salt shoud be keept secret by its name?

I'd use privatesalt instead. My point was that secret and key have too much affinity to intuitively distinguish between them. Private, on the other hand, has the meaning that I should keep this to myself. Another possible name would be securesalt which also conveys that this salt is meant to be used in a secure context.

Good point. The examples part as explained above as well as linking the docs from the code makes totally sense. ?

Good! We also need to remember to link to that page from the @deprecated notices to make these things easy to find.

I choose your software as i knew that you use the secret. I don't know what other extensions we can check. Do you have a suggestion? Maybe we can include this change here with examples and docs etc in a JED newsletter too to make everyone aware?

I have not dived into other extensions' code that deep. The several plugins I had to dissect to see why they were causing compatibility issues with my software didn't use Joomla's secrets. Short of sending out a mass communication through JED I don't any other partial way for 3PDs to become aware of this change.

Ok got your point. Makes sense to me. To get that right after that 6 months of Joomla 4 and 3.10 is out you can drop 3.9 support right?

Correct!

Fun fact: according to the usage stats I collect I see the update lag typically being 3-4 months. If a site is not upgraded in 6 months it's almost certain to remain in the EOL version forever. These are typically the sites that don't have funds for ongoing maintenance. Within 6 months of a new Joomla release I see that 90% of the sites have upgraded to it and within 12 months I see that 98% of the sites have upgraded to it (that's for MINOR versions – for major versions multiply everything by 2).

I know that but that is also true for the developers of the core. We are no crazy people too. Our aim is to support as many users as possible and provide the extension devs a stable platform the build the extensions on. Yes we break stuff but we are humans too.

I know :) Sometimes I see a disconnect between core contributors and the real world, frankly quite weird, use of the software which is understandable. By spending more time improving core code you have less time to talk to end users. Extension developers, like yours truly, usually live in the middle of both worlds. Tapping into out experience can help you understand the weird ways people use the software without having to spend too much time talking to them. I want us to have that kind of rapport. It makes the Joomla experience better for everyone: end users, site integrators, extension developers and core contributors.

avatar zero-24
zero-24 - comment - 19 May 2020

Sorry that it took that long to come back to you.

Based on your comments i will add the following to the first issue as to-do items

  • only copy secret to sitekey
  • secret maps to sitekey through Factory::getConfig()
    -- add a deprecated notice when the mapping happens.
    -- add the link to the docs to the deprecation message
  • generate a new value called privatesalt on upgrade
  • use sitekey for the crypt class Crypt
  • write docs that explain the correct usage of sitekey / secret and privatesalt

Did i miss one?

Short of sending out a mass communication through JED I don't any other partial way for 3PDs to become aware of this change.

That is planed already as mention in my mail now we need to get the docs and text for that message done ;)

I know :) Sometimes I see a disconnect between core contributors and the real world, frankly quite weird, use of the software which is understandable. By spending more time improving core code you have less time to talk to end users. Extension developers, like yours truly, usually live in the middle of both worlds. Tapping into out experience can help you understand the weird ways people use the software without having to spend too much time talking to them. I want us to have that kind of rapport. It makes the Joomla experience better for everyone: end users, site integrators, extension developers and core contributors.

Agree. And I'm happy to keep that communication with you up. I'm not sure what is the best medium for that for sure but I guess eMail is a good start.

So here the happy invite to any Joomla extension developer contact me via tobias.zulauf[at]community.joomla.org so we can start the discussion. I'm not sure howto share that invite to people not listening here. As I have no twitter so any ideas / help would be great :)

This at least solves the problem of the core doing stupid things with the site secret. It doesn't solve 3PDs doing stupid things with it nor can you ever fully solve that. You can, however, have the JED unpublished their extensions if you get reports that point to a 3PD extension leaking secrets by using them unsafely AND point the developer to the documentation page that has actual examples of using the site secret safely.

Will check with the JED and VEL Team. I'm sure this makes sense to be implemented when this is not already the case.

As promised I have checked and now an update on this. I have talked with JED and VEL and confirmed that this process is already handled that way. So I would say we are good, now we have to write the docs and get the word out via that newsletter thing. :)

Thanks :)

avatar zero-24 zero-24 - change - 19 May 2020
The description was changed
avatar zero-24 zero-24 - edited - 19 May 2020
avatar zero-24 zero-24 - change - 19 May 2020
The description was changed
avatar zero-24 zero-24 - edited - 19 May 2020
avatar nikosdion
nikosdion - comment - 21 May 2020

@zero-24 This sounds like an excellent plan to me! Have a great day :)

avatar zero-24 zero-24 - change - 23 Aug 2020
The description was changed
avatar zero-24 zero-24 - edited - 23 Aug 2020
avatar zero-24 zero-24 - change - 23 Aug 2020
The description was changed
avatar zero-24 zero-24 - edited - 23 Aug 2020
avatar zero-24 zero-24 - change - 23 Aug 2020
The description was changed
avatar zero-24 zero-24 - edited - 23 Aug 2020
avatar zero-24
zero-24 - comment - 23 Aug 2020

Ok finally could make some progress here but I have some additional questions to you @nikosdion

secret maps to sitekey through Factory::getConfig()

After looking again in here I'm not 100% sure how that should work given that on upgrade we have than two keys with the same value can you suggest where i should place the code you had in mind to implement that usecase?

write docs that explain the correct usage of sitekey / secret and privatesalt

Do you have suggestions on text for an docs page from an extension developer perspective?

A new, securely random sitesecret is generated on upgrade. This is used for 2FA.

Just to be sure right now with the code we have now this would break right?
As on upgrade we have privatesalt new generated but we have not encrypted the Otp cofing with the new key only the old. So right now this would fail when I got it correctly.

So the idea would be decrypt the OTP config via sitekey / secret and than do a new encryption with the new privatesalt right? Or do I miss something here?

Thanks!

avatar zero-24 zero-24 - change - 23 Aug 2020
The description was changed
avatar zero-24 zero-24 - edited - 23 Aug 2020
avatar zero-24 zero-24 - change - 23 Aug 2020
Labels Added: ?
Removed: ?
avatar zero-24
zero-24 - comment - 23 Aug 2020

change md5 to HMAC-MD5 where nesessarry

One more hash_hmac wants to have a scret value for the md5 generation as third parameter so we might use the privatesalt here right?

avatar nikosdion
nikosdion - comment - 24 Aug 2020

secret maps to sitekey through Factory::getConfig()

Factory::getConfig() returns a Registry object. Let's call it $r. Before returning it you could do $r->set('config') = $r->get('sitekey'). Therefore legacy code doesn't break.

A new, securely random sitesecret is generated on upgrade. This is used for 2FA.
Just to be sure right now with the code we have now this would break right?
As on upgrade we have privatesalt new generated but we have not encrypted the Otp cofing with the new key only the old. So right now this would fail when I got it correctly.

Wrong. Read again our discussion from here onwards so I don't have to repeat myself.

TL;DR: The way that naming the two new secrets doesn't suck balls is that sitekey (formerly secret) is used for encrypting PRIVATE content which doesn't make it to the public site. It's the site's default Crypt key. Therefore TFA doesn't break. If you break TFA on upgrade with what you're doing, you're doing it wrong...

The key privatesalt, with a new value generated on update to 3.10/4.0, is used for generating hashes that will be used in a PUBLIC context such as session keys, media version queries and anto-CSRF tokens.

One more hash_hmac wants to have a scret value for the md5 generation as third parameter so we might use the privatesalt here right?

Yes. Instead of doing md5($something . Factory::getApplication()->get('secret')) we need to be doing hash_hmac('md5', $something, Factory::getApplication()->get('privatesalt')).

In fact, you should do some feature detection here. If the hash_hmac function is not available (e.g. the server admin stupidly disabled it through disabled_functions) OR md5 is not in the array returned by hash_hmac_algos (this detection works ONLY on PHP 7.2 or later) you should fall back to plain old MD5, i.e. md5($something . Factory::getApplication()->get('privatesalt')).

This implementation does not fix the short term problem of developers using the wrong secret for publicly exposed hashes. Developers who want backwards compatibility with Joomla 3.9 or earlier will have to still use secret (which maps to secretkey) on their code that doesn't go through ApplicationHelper::getHash for any reason. Developers who can drop backwards compatibility OR do backwards compatibility right can do it right. An example on writing code compatible with Joomla 3.8 through 4.0 inclusive:

$app = Factory::getApplication();
$key = $app->get('privatesalt', $app->get('secret')) ?? '';
$myHash = hash_hmac('sha1', $something, $key);

Your original idea was diametrically opposite, i.e. having the new privatesalt value map to secret. However, that would break EVERYTHING relying on Joomla's secret key to remain unchanged, including Joomla's TFA itself. Trying to update potentially thousands of records on detecting an update to Joomla would lead to PHP timeouts and break sites. That would be catastrophically bad for a CMS with a declining market share whose remaining selling points is that it's secure and minor version upgrades don't break sites. The other solution would be asking everyone to write their code in a way that tries to decrypt values using EITHER secret OR privatesalt which is even worse for security as it can lead to inadvertent yet practical padding oracle attacks against the privatesalt value.

No matter how you look at it, this is a breaking change. I would never do it on 3.10 but it's your prerogative as core maintainers to try and introduce it to the current major version, hoping shit won't hit the fan.

If I was maintaining the software I'd leave secret as-is for 3.10. In 4.0 I'd copy secret to sitekey and introduce the new value privatesalt for public hashes, with no fallback for secret. Code not updated for 4.0 would fail since Factory::getApplication->get('secret') would return null. That's OK! It's a major version, breaking changes are expected. Even so, developers aren't left hanging out to dry. Those who'd want to write code compatible for J3 and J4 could still easily do it with a single line of code! For code that encrypts private data that never appears encrypted/hashed in a public context the developer could do $encryptionKey = Factory::getApplication()->get('secretkey', Factory::getApplication()->get('secret')) ?? '';. For creating public hashes the developer could either go through ApplicationHelper::getHash or do $hashSalt = Factory::getApplication()->get('privatesalt', Factory::getApplication()->get('secret')) ?? '';. That's how you deal with major changes in mass distributed apps without introducing backwards incompatible changes within the same major version. That is, if you really care to follow semver instead of it being mentioned in developer docs is nothing more than an exercise in buzzword bingo as Joomla's current developer docs seem to be...

avatar nikosdion
nikosdion - comment - 24 Aug 2020

I'll sum up my previous reply in a sentence. Meaningfully improve Joomla's security OR prevent backwards incompatible changes which would lock people of their sites and/or break their sites irreversibly on update: pick one.

avatar zero-24 zero-24 - change - 24 Aug 2020
Labels Added: ?
Removed: ?
avatar zero-24
zero-24 - comment - 24 Aug 2020

TL;DR: The way that naming the two new secrets doesn't suck balls is that sitekey (formerly secret) is used for encrypting PRIVATE content which doesn't make it to the public site. It's the site's default Crypt key. Therefore TFA doesn't break. If you break TFA on upgrade with what you're doing, you're doing it wrong...

Ok I was doing it wrong than good that I asked for confirmation.

With the latest changes we use the sitekey for 2fa and crypt (so nothing breaks on upgrade) and for all other cases privatesalt.

The key privatesalt, with a new value generated on update to 3.10/4.0, is used for generating hashes that will be used in a PUBLIC context such as session keys, media version queries and anto-CSRF tokens.

Done by the last commit, thanks.

No matter how you look at it, this is a breaking change. I would never do it on 3.10 but it's your prerogative as core maintainers to try and introduce it to the current major version, hoping shit won't hit the fan.

Well the aim to do it against 3.10 was that extension devs can adopt the change in 3.10 compatible code already when this is causing more pain for extension devs I'm happy to move this PR here against 4.0 only.

Right now with the latest code in this PR we should not break 2fa nor crypt but update the core code to use the new stuff. Please take a look at the code that I'm changing when there are still issues with it i'm porting this PR to 4.0 where we can break B/C in that matter.

avatar zero-24 zero-24 - change - 24 Aug 2020
Labels Added: ?
Removed: ?
avatar nikosdion
nikosdion - comment - 24 Aug 2020

@zero-24 With the changes I proposed and you are implementing I don't think there will be a b/c break. But we're doing that by sacrificing a little bit of security. Upgrading a site to 3.10 will resume the old secret as the new secretkey to prevent breakage. However, code which is not updated for Joomla 3.10 will still be making public hashes with secret (therefore secretkey) instead of privatesalt. This perpetuates the existing problem in Joomla 3.9 an earlier.

The major ramification that I see is that upgrading a site to 4.0 does NOT (and cannot!) change the secretkey. Therefore if you belong to that extremely rare case where your secret could be extracted by intercepting publicly emitted hashes your site will still be vulnerable even after upgrading to Joomla 4.

What I would do is a breaking change, i.e. get rid of secret and introduce secretkey (with the same value as the old secret on upgrade to prevent broken sites on upgrade) and privatesalt. Since this is a b/c break it cannot happen on 3.x but it can definitely happen on 4.0. Yes, software targeting BOTH 3.10 AND 4.0 can still work, as I've demonstrated above.

Here's something that bugs me. I was told a couple of months ago that no new features with potential security issues or b/c breaks would make it into 3.10. That came after a production leadership vote. Fair enough, even though my contribution was neither. I understand erring on the side of caution. Yet, this is a new feature which will most definitely EITHER break b/c OR have security implications and you're jumping into it with both feet, contradicting yourselves. So what I understand is that there is no policy and you were just trying to get rid of me. As of this moment consider me having been gotten ridden of.

avatar zero-24
zero-24 - comment - 24 Aug 2020

@zero-24 With the changes I proposed and you are implementing I don't think there will be a b/c break. But we're doing that by sacrificing a little bit of security. Upgrading a site to 3.10 will resume the old secret as the new secretkey to prevent breakage. However, code which is not updated for Joomla 3.10 will still be making public hashes with secret (therefore secretkey) instead of privatesalt. This perpetuates the existing problem in Joomla 3.9 an earlier.

Yes i dont see us fixing that issue in 3rd party extensions at any point to be fair

The major ramification that I see is that upgrading a site to 4.0 does NOT (and cannot!) change the secretkey. Therefore if you belong to that extremely rare case where your secret could be extracted by intercepting publicly emitted hashes your site will still be vulnerable even after upgrading to Joomla 4.

Correct.

What I would do is a breaking change, i.e. get rid of secret and introduce secretkey (with the same value as the old secret on upgrade to prevent broken sites on upgrade) and privatesalt. Since this is a b/c break it cannot happen on 3.x but it can definitely happen on 4.0. Yes, software targeting BOTH 3.10 AND 4.0 can still work, as I've demonstrated above.

Well as mention the aim was to backport the changes to 3.10 maybe someone else can give the opinions here @wilsonge or @SniperSister or @HLeithner ?

Here's something that bugs me. I was told a couple of months ago that no new features with potential security issues or b/c breaks would make it into 3.10.

Yes and this still stands, no new features are expected in 3.10.

That came after a production leadership vote.

Right.

Fair enough, even though my contribution was neither.

I guess it is about the SVG stuff? As mention there the best case to accept that would be to do a PR against 4.x for that validation and when this got merged we can look how and if we backport that to 3.10. That has also been latly discussed in production and that should be published in the production reports soon.

I understand erring on the side of caution. Yet, this is a new feature which will most definitely EITHER break b/c OR have security implications and you're jumping into it with both feet, contradicting yourselves.

I personally would not count this here as new feature but as backport of changes that we want to do in 4.x anyway. But lets see what the other mention people say. As mention I'm happy to move this against 4.x only too.

So what I understand is that there is no policy and you were just trying to get rid of me.

I can ashure you that this is not the case at all JSST and Production have been working on how we would accept that feature.

As of this moment consider me having been gotten ridden of.

This are sad news.

avatar zero-24 zero-24 - change - 4 Sep 2020
Labels Added: ?
Removed: ?
avatar zero-24 zero-24 - change - 4 Sep 2020
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2020-09-04 09:32:28
Closed_By zero-24
avatar zero-24 zero-24 - close - 4 Sep 2020
avatar zero-24 zero-24 - change - 4 Sep 2020
Status Closed New
Closed_Date 2020-09-04 09:32:28
Closed_By zero-24
avatar zero-24 zero-24 - change - 4 Sep 2020
Status New Pending
avatar zero-24 zero-24 - reopen - 4 Sep 2020
avatar zero-24
zero-24 - comment - 17 Oct 2020

I do not have to time to complete this PR so I'm going to close here for now. Anyone is free to take this one over based on the discussion above.

avatar zero-24 zero-24 - change - 17 Oct 2020
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2020-10-17 09:34:26
Closed_By zero-24
Labels Added: ?
Removed: ?
avatar zero-24 zero-24 - close - 17 Oct 2020
avatar zero-24 zero-24 - change - 17 Oct 2020
Status Closed New
Closed_Date 2020-10-17 09:34:26
Closed_By zero-24
avatar zero-24 zero-24 - change - 17 Oct 2020
Status New Pending
avatar zero-24 zero-24 - reopen - 17 Oct 2020
avatar zero-24 zero-24 - change - 17 Oct 2020
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2020-10-17 09:34:41
Closed_By zero-24
avatar zero-24 zero-24 - close - 17 Oct 2020

Add a Comment

Login with GitHub to post a comment