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

libgcrypt 1.9.1 [URGENT SECURITY ISSUE] #69980

Closed
wants to merge 1 commit into from
Closed

libgcrypt 1.9.1 [URGENT SECURITY ISSUE] #69980

wants to merge 1 commit into from

Conversation

FiloSottile
Copy link
Sponsor Contributor

@FiloSottile FiloSottile commented Jan 29, 2021

The patch was removed because it was applied upstream.

  • Have you followed the guidelines for contributing?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
  • Is your test running fine brew test <formula>, where <formula> is the name of the formula you're submitting?
  • Does your build pass brew audit --strict <formula> (after doing brew install <formula>)?

Please merge ASAP. Version 1.9.0 has an exploitable heap overflow.

https://dev.gnupg.org/T5275

https://lists.gnupg.org/pipermail/gnupg-announce/2021q1/000455.html

The patch was removed because it was applied upstream.

@FiloSottile FiloSottile changed the title libgcrypt: update to 1.9.1 [URGENT SECURITY ISSUE] libgcrypt 1.9.1 [URGENT SECURITY ISSUE] Jan 29, 2021
@SeekingMeaning
Copy link
Contributor

Compilation error with Intel CPUs:

keccak.c:907:23: error: use of undeclared identifier 'HWF_INTEL_FAST_SHLD'
  else if (features & HWF_INTEL_FAST_SHLD)
                      ^

@SeekingMeaning SeekingMeaning requested a review from a team January 29, 2021 13:51
@SeekingMeaning SeekingMeaning added the build failure CI fails while building the software label Jan 29, 2021
@FiloSottile
Copy link
Sponsor Contributor Author

Looking into it. If I can't work it out I might just backport a patch for the security issue to 1.9.0, if that's acceptable. It's a small patch.

@FiloSottile
Copy link
Sponsor Contributor Author

Introduced by https://dev.gnupg.org/rC8d404a629167d67ed56e45de3e65d1e0b7cdeb24. Somehow HAVE_CPU_ARCH_X86 isn't set.

From configure.ac:

case "$mpi_cpu_arch" in
     x86)
        AC_DEFINE(HAVE_CPU_ARCH_X86, 1,   [Defined for the x86 platforms])
        GCRYPT_HWF_MODULES="libgcrypt_la-hwf-x86.lo"
        ;;

@dawidd6
Copy link
Member

dawidd6 commented Jan 29, 2021

Try removing --disable-asm in configure.

Ref: macports/macports-ports@f75b194

@FiloSottile
Copy link
Sponsor Contributor Author

Yeah, $mpi_cpu_arch is not set for --disable-asm builds. --disable-asm builds look just broken on x86.

Dropping --disable-asm would be a big change. @SeekingMeaning's PR to rollback to 1.8.7 seems like the right call.

The previous patch was removed because it was applied upstream.

The new patch fixes the build on Intel.
@FiloSottile
Copy link
Sponsor Contributor Author

The fix was actually pretty small, just reverted part of that upstream commit.

The current version should build, up to you if you want to fix forward or rollback.

Upstream report: https://dev.gnupg.org/T5277

@SMillerDev
Copy link
Member

The current version should build, up to you if you want to fix forward or rollback.

If possible we should definitely fix forward.

@fxcoudert fxcoudert removed the build failure CI fails while building the software label Jan 29, 2021
@SeekingMeaning SeekingMeaning added the ready to merge PR can be merged once CI is green label Jan 29, 2021
@FiloSottile
Copy link
Sponsor Contributor Author

Pulled out an Intel MacBook to test the patch, and this PR installed successfully on my macOS 11.2. 🎉

@carlocab
Copy link
Member

Yea, libgcrypt definitely built on all the CI nodes. I've looked. They're just testing recursive dependents now.

@BrewTestBot
Copy link
Member

:shipit: @dawidd6 has triggered a merge.

@SMillerDev
Copy link
Member

Thanks @FiloSottile ! Without contributions like yours it'd be impossible to keep homebrew going with the high standards that users have come to expect from the project. You can feel good knowing that you've made the world a tiny bit better for homebrew users around the world! 👍 🎉

@BrewTestBot BrewTestBot added the outdated PR was locked due to age label Mar 1, 2021
@Homebrew Homebrew locked as resolved and limited conversation to collaborators Mar 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated PR was locked due to age ready to merge PR can be merged once CI is green
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants