#4254 CVE-2019-7628: Do not leak partial API keys.
Merged 5 years ago by pingou. Opened 5 years ago by bowlofeggs.
bowlofeggs/pagure dont_email_secrets  into  master

file modified
+2 -3
@@ -45,7 +45,6 @@ 

              user = token.user

              username = user.fullname or user.username

              user_email = user.default_email

-             api_key = token.id

              days_left = (token.expiration - datetime.utcnow()).days

              subject = 'Pagure API key expiration date is near!'

              if token.project:
@@ -57,7 +56,7 @@ 

  Thanks,

  Your Pagure Admin. ''' % (

                      username,

-                     api_key[:5],

+                     token.description,

                      token.project.fullname,

                      days_left

                  )
@@ -69,7 +68,7 @@ 

  Thanks,

  Your Pagure Admin. ''' % (

                      username,

-                     api_key[:5],

+                     token.description,

                      days_left)

              if not check:

                  msg = pagure.lib.notify.send_email(text, subject, user_email)

It was discovered that Pagure was leaking API keys by e-mailing
them to users. Few e-mail servers validate TLS certificates, so
it is possible for man-in-the-middle attacks to read these e-mails
and gain access to Pagure on the behalf of other users. The
vulnerability was introduced in [0].

This problem was partially addressed in a prior commit[1], but
that commit still leaks the first 5 characters of the key which
weakens the secret.

This commit uses the description of the API key instead of any part
of the secret in the e-mail sent to users so that none of the key
is e-mailed over the Internet.

[0] 57975ef
[1] 9905fb1

fixes #4253

Signed-off-by: Randy Barlow randy@electronsweatshop.com

rebased onto 904c0fc479a3ebf48d7ec9b0e7a9fc8c4979058d

5 years ago

Workaround

This issue can be worked around by disabling the cron job. After disabling the cron job, it would be wise to delete any API keys you think may have been e-mailed. You can delete them all to be safe. Users will have to generate new ones if you take this step.

rebased onto c7a41f0b09f7b599f88d0ffb8ad89f9d6163a36c

5 years ago

rebased onto d4dff60efbf3a521819396ba0cc6ffde5061cfc4

5 years ago

pretty please pagure-ci rebuild

5 years ago

rebased onto d18f42d

5 years ago

The tests failed but because of repoSpanner which is broken for everyone.

I'll adjust the wording of the email sent a little bit as well but that can come in another PR

Thanks for the patch and for catching this and doing all the work to get the CVE number and all!

Pull-Request has been merged by pingou

5 years ago