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

minor automake fixes plus tweetnacl logic change #1834

Merged
merged 2 commits into from
Mar 2, 2016

Conversation

garlick
Copy link
Contributor

@garlick garlick commented Mar 2, 2016

Happy to rebase this without the tweetnacl logic change, which was not enthusiastically received on mailing list. Submitting with it included first just to give the idea one more chance. Hope it doesn't seem like I'm trying to sneak one in through the back door.

Move AM_CONDITIONAL for --disable-curve outside of shell
conditional (per sec 20.1 of automake manual) and fix its
second argument to be a test rather than a literal zero.
Brackets around defaults in configure --help strings for
--with-libsodium and --disable-curve were not displayed.

Solution:  Add m4 quotes.
@hintjens
Copy link
Member

hintjens commented Mar 2, 2016

I can't agree with the problem "Too easy to select tweetnacl"... there is no proven disadvantage to this, and many known costs to not building in security by default.

I'll merge this if you want yet the first thing I'll do tomorrow is fix the problem "libzmq is insecure by default"... :)

@garlick
Copy link
Contributor Author

garlick commented Mar 2, 2016

I'll force push without the offending commit in a sec but wanted to respond to your comments.

My concern about tweetnacl is not the code (though lack of upstream repo is concerning).
I provides libzmq RPM packages to several US national labs that run our in-house redhat-based distro. I worry about

  • accidentally packaging libzmq with tweetnacl not libsodium, pushing that out (low probability event)
  • we become aware of security issue with tweetnacl (2nd low probabilty event)
  • I have to push out emergency updates and get a big WTF from my peers (big ouch)

Roger the cmake concern, was only dimly aware that libzmq had grown an extra build system.

My proposed change does make a hard stop if libsodium is not found, e.g.

configure: error: libsodium is not installed.  Install it, then run
    configure again, or configure --with-builtin-tweetnacl or --disable-curve.

so this doesn't really break "insecure by default".

@garlick
Copy link
Contributor Author

garlick commented Mar 2, 2016

OK that commit is gone. Thanks.

hintjens added a commit that referenced this pull request Mar 2, 2016
minor automake fixes plus tweetnacl logic change
@hintjens hintjens merged commit 36abdf7 into zeromq:master Mar 2, 2016
@hintjens
Copy link
Member

hintjens commented Mar 2, 2016

Thanks!

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

Successfully merging this pull request may close these issues.

2 participants