Add GitHub workflow for Brim releases #1

nwt · 2020-02-28T20:42:36Z

When a tag matching /^v.*brim.*$/ is pushed, the workflow creates a
GitHub release with a macOS artifact called zeek-TAG.darwin-amd64.zip
with these contents:

zeek/
  bin/zeek           Zeek executable
  share/zeek/base/   Zeek base scripts
  share/zeek/policy/ Zeek policy scripts
  share/zeek/site/   Zeek site scripts
  zeek               executable shell script to set ZEEKPATH and
                     ZEEK_PLUGIN_PATH and then exec bin/zeek

nwt · 2020-02-28T20:56:06Z

.github/workflows/brim-release.yml

+      # Prefer static libraries.  Appending to this file is hacky but effective.
+      - run: echo 'set(CMAKE_FIND_LIBRARY_SUFFIXES .a ${CMAKE_FIND_LIBRARY_SUFFIXES})' >> cmake/CommonCMakeConfig.cmake


I did this to keep CMakeList.txt out of this change, but if it strikes anyone as too dicey, we could instead add the set(...) to that file.

We can do this after I move henridf/cmake to brimsec/cmake (and try to contribute it to upstream zeek and cmake, possibly enabled by a configure flag?).

Would you please add the changes to the CMakeList.txt, along with a comment like: # Brim: Prefer static ....

…cket An attacker can make Zeek crash by posting the KEX packet twice, which will result in an assertion failure in binpac::datastring::init(): #0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50 #1 0x00007ffff5196535 in __GI_abort () at abort.c:79 #2 0x00007ffff519640f in __assert_fail_base (fmt=0x7ffff52f86e0 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", assertion=0x1d33530 "!data_", file=0x1d33537 "aux/binpac/lib/binpac_bytestring.h", line=108, function=<optimized out>) at assert.c:92 #3 0x00007ffff51a3b92 in __GI___assert_fail (assertion=0x1d33530 "!data_", file=0x1d33537 "aux/binpac/lib/binpac_bytestring.h", line=108, function=0x1d3356c "void binpac::datastring<unsigned char>::init(const T *, int) [T = unsigned char]") at assert.c:101 #4 0x0000000000c1e970 in binpac::datastring<unsigned char>::init (this=0x608000d609d0, begin=0x603001bdd1d0 "diffie-hellman-group16-sha512", length=29) at aux/binpac/lib/binpac_bytestring.h:108 #5 0x0000000000e9ab60 in binpac::SSH::SSH_Conn::update_kex (this=0x608000d609a0, algs=..., orig=true) at src/analyzer/protocol/ssh/ssh_pac.cc:205 #6 0x0000000000ea0d06 in binpac::SSH::SSH2_KEXINIT::Parse (this=0x60b000734680, t_begin_of_data=0x621000004753 "\200\275\a%\223\023Y8\204t\235\363!\031I.", t_end_of_data=0x621000004b85 "ޭ\276", <incomplete sequence \357>, t_context=0x603001bdcc90, t_byteorder=0) at src/analyzer/protocol/ssh/ssh_pac.cc:1598 #7 0x0000000000e9f8f4 in binpac::SSH::SSH2_Message::Parse (this=0x608000d60ea0, t_begin_of_data=0x621000004753 "\200\275\a%\223\023Y8\204t\235\363!\031I.", t_end_of_data=0x621000004b85 "ޭ\276", <incomplete sequence \357>, t_context=0x603001bdcc90, t_byteorder=0) at src/analyzer/protocol/ssh/ssh_pac.cc:1326 #8 0x0000000000e9d7e1 in binpac::SSH::SSH2_Key_Exchange::Parse (this=0x604001779850, t_begin_of_data=0x621000004751 "\006\024\200\275\a%\223\023Y8\204t\235\363!\031I.", t_end_of_data=0x621000004b85 "ޭ\276", <incomplete sequence \357>, t_context=0x603001bdcc90, t_byteorder=0) at src/analyzer/protocol/ssh/ssh_pac.cc:1210 #9 0x0000000000e9c981 in binpac::SSH::SSH_Key_Exchange::ParseBuffer (this=0x603001bdccc0, t_flow_buffer=0x608000d60a20, t_context=0x603001bdcc90, t_byteorder=0) at src/analyzer/protocol/ssh/ssh_pac.cc:628 #10 0x0000000000e9c26c in binpac::SSH::SSH_PDU::ParseBuffer (this=0x604001779810, t_flow_buffer=0x608000d60a20, t_context=0x603001bdcc90) at src/analyzer/protocol/ssh/ssh_pac.cc:446 #11 0x0000000000ea6f04 in binpac::SSH::SSH_Flow::NewData (this=0x604001774690, t_begin_of_data=0x62100000474d "", t_end_of_data=0x621000004b85 "ޭ\276", <incomplete sequence \357>) at src/analyzer/protocol/ssh/ssh_pac.cc:3071 #12 0x0000000000e9a38f in binpac::SSH::SSH_Conn::NewData (this=0x608000d609a0, is_orig=true, begin=0x62100000474d "", end=0x621000004b85 "ޭ\276", <incomplete sequence \357>) at src/analyzer/protocol/ssh/ssh_pac.cc:63 #13 0x0000000000e98335 in analyzer::SSH::SSH_Analyzer::DeliverStream (this=0x7fffffffdd40, len=1080, data=0x62100000474d "", orig=true) at src/analyzer/protocol/ssh/SSH.cc:68 With assertions turned off, this would "only" be a memory leak. This commit fixes the vulnerability by freeing and clearing the `binpac::datastring` before assigning a new value. (cherry picked from commit 98c5053)

When a tag matching /^v.*brim.*$/ is pushed, the workflow creates a GitHub release with a macOS artifact called zeek-TAG-darwin-x64.zip with these contents: zeek/ bin/zeek Zeek executable share/zeek/base/ Zeek base scripts share/zeek/policy/ Zeek policy scripts share/zeek/site/ Zeek site scripts zeek executable shell script to set ZEEKPATH and ZEEK_PLUGIN_PATH and then exec bin/zeek

mikesbrown

It looks reasonable to me. Do you mind creating a tag that will trigger the workflow?

nwt · 2020-02-28T22:29:20Z

@mikesbrown:I triggered https://github.com/brimsec/zeek/actions/runs/46998542 by pushing tag vBrimReleaseWorkflowTest-brim.

mikesbrown

Thanks. I was able to run the downloaded zeek on my machine.

alfred-landrum

Would you please:

add a top level directory in the repo, brim
create a script inside that directory that calls the run statements you have in the workflow job (configure ... , make -j ...)
call that script in this release workflow

Without a script to create the build locally, it will be hard to debug or create a modified zeek for local testing. Maybe you were trying to avoid making changes to the repo, but I believe we discussed having a top level directory for our changes. (as well as a README-brim.md that explains the local mods, which we can separately.)

alfred-landrum · 2020-03-02T02:59:30Z

.github/workflows/brim-zeek-wrapper

@@ -0,0 +1,7 @@
+#!/usr/bin/env bash


Would you please move this script into the top level brim folder I mentioned in my earlier comment?

alfred-landrum · 2020-03-02T22:38:35Z

.github/workflows/brim-release.yml

+    runs-on: macos-10.15
+    steps:
+      - uses: actions/checkout@v2
+      - run: ./brim/release


nit: Would you put the string macos somewhere in the release script name? It calls out to brew, so it's mac specific.

alfred-landrum · 2020-03-03T00:10:31Z

brim/zeek

@@ -0,0 +1,7 @@
+#!/usr/bin/env bash
+
+file=$(realpath "${BASH_SOURCE[0]}")


I don't have realpath in my path on my mac; I think that's from gnu coreutils? I'm used to seeing it on linux.

…cket An attacker can make Zeek crash by posting the KEX packet twice, which will result in an assertion failure in binpac::datastring::init(): #0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50 #1 0x00007ffff5196535 in __GI_abort () at abort.c:79 #2 0x00007ffff519640f in __assert_fail_base (fmt=0x7ffff52f86e0 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", assertion=0x1d33530 "!data_", file=0x1d33537 "aux/binpac/lib/binpac_bytestring.h", line=108, function=<optimized out>) at assert.c:92 #3 0x00007ffff51a3b92 in __GI___assert_fail (assertion=0x1d33530 "!data_", file=0x1d33537 "aux/binpac/lib/binpac_bytestring.h", line=108, function=0x1d3356c "void binpac::datastring<unsigned char>::init(const T *, int) [T = unsigned char]") at assert.c:101 #4 0x0000000000c1e970 in binpac::datastring<unsigned char>::init (this=0x608000d609d0, begin=0x603001bdd1d0 "diffie-hellman-group16-sha512", length=29) at aux/binpac/lib/binpac_bytestring.h:108 #5 0x0000000000e9ab60 in binpac::SSH::SSH_Conn::update_kex (this=0x608000d609a0, algs=..., orig=true) at src/analyzer/protocol/ssh/ssh_pac.cc:205 #6 0x0000000000ea0d06 in binpac::SSH::SSH2_KEXINIT::Parse (this=0x60b000734680, t_begin_of_data=0x621000004753 "\200\275\a%\223\023Y8\204t\235\363!\031I.", t_end_of_data=0x621000004b85 "ޭ\276", <incomplete sequence \357>, t_context=0x603001bdcc90, t_byteorder=0) at src/analyzer/protocol/ssh/ssh_pac.cc:1598 #7 0x0000000000e9f8f4 in binpac::SSH::SSH2_Message::Parse (this=0x608000d60ea0, t_begin_of_data=0x621000004753 "\200\275\a%\223\023Y8\204t\235\363!\031I.", t_end_of_data=0x621000004b85 "ޭ\276", <incomplete sequence \357>, t_context=0x603001bdcc90, t_byteorder=0) at src/analyzer/protocol/ssh/ssh_pac.cc:1326 #8 0x0000000000e9d7e1 in binpac::SSH::SSH2_Key_Exchange::Parse (this=0x604001779850, t_begin_of_data=0x621000004751 "\006\024\200\275\a%\223\023Y8\204t\235\363!\031I.", t_end_of_data=0x621000004b85 "ޭ\276", <incomplete sequence \357>, t_context=0x603001bdcc90, t_byteorder=0) at src/analyzer/protocol/ssh/ssh_pac.cc:1210 #9 0x0000000000e9c981 in binpac::SSH::SSH_Key_Exchange::ParseBuffer (this=0x603001bdccc0, t_flow_buffer=0x608000d60a20, t_context=0x603001bdcc90, t_byteorder=0) at src/analyzer/protocol/ssh/ssh_pac.cc:628 #10 0x0000000000e9c26c in binpac::SSH::SSH_PDU::ParseBuffer (this=0x604001779810, t_flow_buffer=0x608000d60a20, t_context=0x603001bdcc90) at src/analyzer/protocol/ssh/ssh_pac.cc:446 #11 0x0000000000ea6f04 in binpac::SSH::SSH_Flow::NewData (this=0x604001774690, t_begin_of_data=0x62100000474d "", t_end_of_data=0x621000004b85 "ޭ\276", <incomplete sequence \357>) at src/analyzer/protocol/ssh/ssh_pac.cc:3071 #12 0x0000000000e9a38f in binpac::SSH::SSH_Conn::NewData (this=0x608000d609a0, is_orig=true, begin=0x62100000474d "", end=0x621000004b85 "ޭ\276", <incomplete sequence \357>) at src/analyzer/protocol/ssh/ssh_pac.cc:63 #13 0x0000000000e98335 in analyzer::SSH::SSH_Analyzer::DeliverStream (this=0x7fffffffdd40, len=1080, data=0x62100000474d "", orig=true) at src/analyzer/protocol/ssh/SSH.cc:68 With assertions turned off, this would "only" be a memory leak. This commit fixes the vulnerability by freeing and clearing the `binpac::datastring` before assigning a new value.

The following source code: function foo(foo: int) {} function foo() {} ... first produces this error: error in crash.zeek, line 1 and crash.zeek, line 2: incompatible types (function(foo:int;) : void and function() : void) ... and then crashes: Thread 1 "zeek" received signal SIGSEGV, Segmentation fault. 0x000055555617d970 in RecordType::FieldDecl (this=0x555557cbdd80, field=0) at ../src/Type.cc:735 735 return (*types)[field]; (gdb) bt #0 0x000055555617d970 in RecordType::FieldDecl (this=0x555557cbdd80, field=0) at ../src/Type.cc:735 #1 0x000055555619c0e2 in transfer_arg_defaults (args=0x555557cbf270, recv=0x555557cbdd80) at ../src/Var.cc:315 #2 0x000055555619c40c in begin_func (id=0x555557cbf070, module_name=0x5555579dd4a0 "GLOBAL", flavor=FUNC_FLAVOR_FUNCTION, is_redef=0, t=0x555557cbde00, attrs=0x0) at ../src/Var.cc:371 #3 0x0000555555f5723b in yyparse () at parse.y:1174 #4 0x0000555556038bf6 in main (argc=5, argv=0x7fffffffe658) at ../src/main.cc:646 This is because `begin_func()` checks if the old and new functions have the same type via same_type(), but continues anyway, and then transfer_arg_defaults() crashes because both `Args()` have different lengths.

nwt requested review from alfred-landrum, henridf and mikesbrown February 28, 2020 20:42

nwt commented Feb 28, 2020

View reviewed changes

nwt force-pushed the brim-release branch from 9834822 to 5860751 Compare February 28, 2020 21:16

mikesbrown reviewed Feb 28, 2020

View reviewed changes

mikesbrown approved these changes Feb 28, 2020

View reviewed changes

henridf approved these changes Feb 29, 2020

View reviewed changes

alfred-landrum suggested changes Mar 2, 2020

View reviewed changes

alfred-landrum reviewed Mar 2, 2020

View reviewed changes

PR feedback

d14b63e

alfred-landrum approved these changes Mar 2, 2020

View reviewed changes

use "git describe --dirty --tags"

21dc7b4

alfred-landrum approved these changes Mar 2, 2020

View reviewed changes

make the install/strip target

9bfcfb4

nwt force-pushed the brim-release branch from c01bc38 to 9bfcfb4 Compare March 2, 2020 23:01

change artifact name to zeek-TAG.darwin-amd64.zip

f05c18a

alfred-landrum reviewed Mar 3, 2020

View reviewed changes

alfred-landrum mentioned this pull request Mar 3, 2020

download zeek and make available for zqd brimdata/zui#417

Merged

remove realpath

2db8f2c

nwt merged commit 434a7c2 into master Mar 3, 2020

nwt deleted the brim-release branch March 3, 2020 21:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add GitHub workflow for Brim releases #1

Add GitHub workflow for Brim releases #1

nwt commented Feb 28, 2020 •

edited

Loading

nwt Feb 28, 2020

henridf Feb 29, 2020

alfred-landrum Mar 2, 2020

nwt Mar 2, 2020

mikesbrown left a comment

nwt commented Feb 28, 2020

mikesbrown left a comment

alfred-landrum left a comment

alfred-landrum Mar 2, 2020

nwt Mar 2, 2020

alfred-landrum Mar 2, 2020 •

edited

Loading

alfred-landrum Mar 3, 2020

		# Prefer static libraries. Appending to this file is hacky but effective.
		- run: echo 'set(CMAKE_FIND_LIBRARY_SUFFIXES .a ${CMAKE_FIND_LIBRARY_SUFFIXES})' >> cmake/CommonCMakeConfig.cmake

		@@ -0,0 +1,7 @@
		#!/usr/bin/env bash

		file=$(realpath "${BASH_SOURCE[0]}")

Add GitHub workflow for Brim releases #1

Add GitHub workflow for Brim releases #1

Conversation

nwt commented Feb 28, 2020 • edited Loading

nwt Feb 28, 2020

Choose a reason for hiding this comment

henridf Feb 29, 2020

Choose a reason for hiding this comment

alfred-landrum Mar 2, 2020

Choose a reason for hiding this comment

nwt Mar 2, 2020

Choose a reason for hiding this comment

mikesbrown left a comment

Choose a reason for hiding this comment

nwt commented Feb 28, 2020

mikesbrown left a comment

Choose a reason for hiding this comment

alfred-landrum left a comment

Choose a reason for hiding this comment

alfred-landrum Mar 2, 2020

Choose a reason for hiding this comment

nwt Mar 2, 2020

Choose a reason for hiding this comment

alfred-landrum Mar 2, 2020 • edited Loading

Choose a reason for hiding this comment

alfred-landrum Mar 3, 2020

Choose a reason for hiding this comment

nwt commented Feb 28, 2020 •

edited

Loading

alfred-landrum Mar 2, 2020 •

edited

Loading