Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[CVE-2021-46900] Obsolete cookie parameter that is not secure enough #1091

Closed
2 tasks done
ikedas opened this issue Jan 21, 2021 · 21 comments · Fixed by sympa-community/sympa-community.github.io#80
Closed
2 tasks done

Comments

@ikedas
Copy link
Member

ikedas commented Jan 21, 2021

Expected Behavior

cookie parameter will be obsoleted.

Possible Solution

  • Modify codes that are using cookie parameter appropriately.
  • Update documentation.

Context

As far as I investigated with current stable (6.2.60), cookie parameter is used at least in these things:

  • bin/upgrade_send_spool.pl#sympa_checksum()
    to migrate older bulk spool
    This is needed for upgrading from versions prior to 6.2.

  • bin/upgrade_sympa_password.pl#_decrypt_rc4_password()
    to migrate older user password
    This is needed for upgrading from versions prior to 6.2.26 by which RC4 encryption was deprecated.

  • Sympa::Tools::Password#tmp_passwd()
    to create temporary password for new users;
    This process should be removed after careful review.

  • Sympa::Archive#_get_tag()
    to generate randomized tags for $tag$ notation in mhonarc_ressources.tt2
    This process should be removed by adopting another randomization.

  • Sympa::List.pm#get_cookie()
    to get List's (not the default in sympa.conf) cookie parameter
    This function is no longer used.

@racke
Copy link
Contributor

racke commented Jan 21, 2021

Thanks a lot for the thorough investigation, @ikedas.

For the first item ... I think it is about time to retire direct upgrades from 6.1 to the latest 6.2 release.

@ikedas
Copy link
Member Author

ikedas commented Jan 21, 2021

It's nice idea to give up direct upgrade from 6.1. (You might mean of Debian packages.)

I think sympa.pl --upgrade would be better to stop processing when sympa.conf contains backtick. By this, the sites not using this syntax will be upgraded safely.

@racke
Copy link
Contributor

racke commented Jan 21, 2021

It's nice idea to give up direct upgrade from 6.1. (You might mean of Debian packages.)

This suggestion was for Sympa itself only. Debian supports updates to the next release only.

I think sympa.pl --upgrade would be better to stop processing when sympa.conf contains backtick. By this, the sites not using this syntax will be upgraded safely.

Yes, that makes sense to me.

@dverdin
Copy link
Contributor

dverdin commented Jan 21, 2021

For the first item ... I think it is about time to retire direct upgrades from 6.1 to the latest 6.2 release.

@racke: I don't think simply dropping upgrade from versions prior to 6.1 is such a good idea. From my experience, People tend to operate their Sympa during a long time before upgrading, because as long as it makes the job, they don't bother upgrading. And when they decide to do it, what's nice with Sympa is that the whole history of sympa upgrade operations is handled by sympa.pl --upgrade and a handful of scripts. This simplifies making people move to the new version.

I think sympa.pl --upgrade would be better to stop processing when sympa.conf contains backtick. By this, the sites not using this syntax will be upgraded safely.

Makes sense to me too.

@racke
Copy link
Contributor

racke commented Jan 21, 2021

For the first item ... I think it is about time to retire direct upgrades from 6.1 to the latest 6.2 release.

@racke: I don't think simply dropping upgrade from versions prior to 6.1 is such a good idea. From my experience, People tend to operate their Sympa during a long time before upgrading, because as long as it makes the job, they don't bother upgrading. And when they decide to do it, what's nice with Sympa is that the whole history of sympa upgrade operations is handled by sympa.pl --upgrade and a handful of scripts. This simplifies making people move to the new version.

But that's not really our problem. 6.2 was released in 2015, so we did lots of incompatible changes since then. There is no straightforward upgrade from 6.1 any way in my opinion.

@ikedas
Copy link
Member Author

ikedas commented Jan 21, 2021

For the first item ... I think it is about time to retire direct upgrades from 6.1 to the latest 6.2 release.

@racke: I don't think simply dropping upgrade from versions prior to 6.1 is such a good idea. From my experience, People tend to operate their Sympa during a long time before upgrading, because as long as it makes the job, they don't bother upgrading. And when they decide to do it, what's nice with Sympa is that the whole history of sympa upgrade operations is handled by sympa.pl --upgrade and a handful of scripts. This simplifies making people move to the new version.

But that's not really our problem. 6.2 was released in 2015, so we did lots of incompatible changes since then. There is no straightforward upgrade from 6.1 any way in my opinion.

It is true that "There is no straightforward upgrade from 6.1". However, currently administrators can upgrade from 6.1.x by intervening manually according to Upgrading notes. --- In fact, recently some administrators posted questions about this process.

We may drop support for releases prior to 6.2.xx (Of course I agree). This means that we won't provide more fixes for these earlier versions, but doesn't mean that we will stop providing measure to upgradse from earlier versions.

@dverdin
Copy link
Contributor

dverdin commented Jan 21, 2021

we did lots of incompatible changes since then.

Certainly but the upgrade takes care of the incompatible changes.
Listen, I agree on the removal of useless code but, on my opinion, anything used to upgrade from previous versions should not be considered useless.

Maybe we should create an issue about which version upgrade we support?

If old version upgrade is dropped, I'd be in favor of moving old upgrade code to contributions; This way, we get rid of old code and don't abandon old timers.

EDIT: rephrase.

@dverdin
Copy link
Contributor

dverdin commented Jan 21, 2021

We may drop support for releases prior to 6.2.xx (Of course I agree). This means that we won't provide more fixes for these earlier versions, but doesn't mean that we will stop providing measure to upgradse from earlier versions.

I agree with Soji.

@racke
Copy link
Contributor

racke commented Jan 21, 2021

No one with a sane mind would do the upgrade of an old Sympa production installation in place. You are going to start with a test system and run the upgrades until your satisfied with the result. So having to run multiple steps of upgrades is really insignificant compared to the whole progress.

That leaves us to decide which is the version we support with the sympa upgrade command.

@ikedas
Copy link
Member Author

ikedas commented Jan 21, 2021

No one with a sane mind would do the upgrade of an old Sympa production installation in place. You are going to start with a test system and run the upgrades until your satisfied with the result. So having to run multiple steps of upgrades is really insignificant compared to the whole progress.

However your proposal also affect to the people upgrading from 6.2.58.

That leaves us to decide which is the version we support with the sympa upgrade command.

As I wrote before, we may not worry about whether code for upgrading process really supports earlier versions.

Good night for now.

@racke
Copy link
Contributor

racke commented Jan 21, 2021

@ikedas Can you please explain why it affects upgrading from 6.2.58?

ikedas added a commit to ikedas/sympa that referenced this issue Jan 22, 2021
…e_send_spool.pl and upgrade_sympa_password.pl to read the obsoleted parameter by themselves
@ikedas
Copy link
Member Author

ikedas commented Jan 22, 2021

@ikedas Can you please explain why it affects upgrading from 6.2.58?

It's you who tell "to retire direct upgrades from 6.1 to the latest 6.2 release". You also told user may upgrade "having to run multiple steps". I think these mean that users using earlier version must take the first step to upgrade to 6.2.60 (or later?), then the second step to the most recent version: Now the two versions with 6.2.60 in between are allowed to be discontinuous. Thus, anything that has been included in any version prior to 6.2.60 may be removed or changed on the future versions without notice or measures. --- That's why your proposal will also affect to users upgrading from 6.2.58.

@ikedas
Copy link
Member Author

ikedas commented Jan 22, 2021

@racke, though I haven't debugged, with the change like a285918, I think it is possible to both obsolete a parameter and to process upgrading using that obsoleted parameter. Even by such change, do we still have to remove these scripts?

@racke
Copy link
Contributor

racke commented Jan 22, 2021

@ikedas Can you please explain why it affects upgrading from 6.2.58?

It's you who tell "to retire direct upgrades from 6.1 to the latest 6.2 release". You also told user may upgrade "having to run multiple steps". I think these mean that users using earlier version must take the first step to upgrade to 6.2.60 (or later?), then the second step to the most recent version: Now the two versions with 6.2.60 in between are allowed to be discontinuous. Thus, anything that has been included in any version prior to 6.2.60 may be removed or changed on the future versions without notice or measures. --- That's why your proposal will also affect to users upgrading from 6.2.58.

No, that is not my intention. I definitely want to select an early version of 6.2 to be upgradeable to 6.2 latest version and not 6.2.58. For example we choose 6.2.16. Users from 6.1 and 6.2 < 6.2.16 need to upgrade 6.2.16 first. Direct upgrades from 6.2.16 and newer will be supported for the foreseeable future.

@ikedas
Copy link
Member Author

ikedas commented Jan 22, 2021

@ikedas Can you please explain why it affects upgrading from 6.2.58?

It's you who tell "to retire direct upgrades from 6.1 to the latest 6.2 release". You also told user may upgrade "having to run multiple steps". I think these mean that users using earlier version must take the first step to upgrade to 6.2.60 (or later?), then the second step to the most recent version: Now the two versions with 6.2.60 in between are allowed to be discontinuous. Thus, anything that has been included in any version prior to 6.2.60 may be removed or changed on the future versions without notice or measures. --- That's why your proposal will also affect to users upgrading from 6.2.58.

No, that is not my intention. I definitely want to select an early version of 6.2 to be upgradeable to 6.2 latest version and not 6.2.58. For example we choose 6.2.16. Users from 6.1 and 6.2 < 6.2.16 need to upgrade 6.2.16 first. Direct upgrades from 6.2.16 and newer will be supported for the foreseeable future.

As you know, all of earlier versions are proven vulnerable and buggy. Only version we can recommend is always the latest version at that time. Thus, IMO we cannot recommend particular version of earlier 6.2.x as transient version.

# NB: Besides, assuming if you wanted to drop upgrading from earlier versions by getting rid of upgrade_sympa_password.pl, the version you could choose is not earlier than 6.2.28. See the initial post. However, why 6.2.28? This version is never slightly more stable than any other versions.

Anyway, recent some posts are not the discussion about "obsoleting 'cookie' parameter". I'm sorry but I'm going to exercise my authority so that we may save time for discussion which seems not having a quick conclusion:

As the Release Manager, I don't allow removal of existing scripts and codes which will help upgrading from earlier versions, during my term, i.e. at least in this year. (see also a thread on discussion)

@racke
Copy link
Contributor

racke commented Jan 22, 2021

So be it. I will restrict my contributions to unanimous subjects and keep out discussions like this.

@ikedas ikedas added this to the 6.2.62 milestone Jan 29, 2021
ikedas added a commit that referenced this issue Feb 4, 2021
@ikedas
Copy link
Member Author

ikedas commented Feb 5, 2021

Code has been modified. Next, the documentation should be updated.

ikedas added a commit to ikedas/sympa that referenced this issue Feb 9, 2021
…e file) under sympa-community#1091

  - Update POTFILES for changing name of a file
  - Update xgettext.pl for the new tags
@ikedas
Copy link
Member Author

ikedas commented Apr 25, 2021

Now documentation has been prepared.

@racke
Copy link
Contributor

racke commented Apr 26, 2021

Great work @ikedas. 👍

@carnil
Copy link

carnil commented Dec 31, 2023

This issue appears to have CVE-2021-46900 assigned.

@ikedas ikedas changed the title Obsolete cookie parameter [CVE-2021-46900] Obsolete cookie parameter that is not secure enough Dec 31, 2023
@ikedas
Copy link
Member Author

ikedas commented Dec 31, 2023

@carnil , thank you for information!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants