Changes to improve compatibility with OS X #13

Closed
wants to merge 4 commits into
from

Projects

None yet

3 participants

@akerl
akerl commented Dec 6, 2012

On Macs, both with system tools and brew, glibtoolize exists but libtoolize does not. I've adjusted the autogen.sh script to account for this so it works in either case.

In the config file (.fwknoprc), there didn't appear to be a way to enable gpg-agent usage. I've added a "USE_GPG_AGENT" option to allow this to be set via the config file.

OSX appears to fail when trying to stat a directory which needs to be expanded, such as "~/.gnupg". This causes the validation check to fail. I've adjusted the check to first expand the path to resolve this issue.

I'm available on Freenode in #cipherdyne if there's anything I need to adjust/fix to get this pulled in.

@damienstuart
Collaborator

Hello,

I like the updates to autogen.sh and the addition of the GPG_AGENT option in the rc file (good catch).

As for the path expansion, I'm not sure we would want to put that functionality into libfko. Instead, I think it may be better to leave the responsibility or option of performing expansion on a path to the client (or other user of the library).

The notion of shell expansion of a string is a very Unix/Linux-specific thing. My goal is to have libfko to stay somewhat lean and OS-agnostic.

If we were to add this to the fwknop code (libfko and/or the client / server) then we would need to add a check for wordexp in configure.ac and put the appropriate '#ifdef HAVE_WORDEXP' around that code so we can handle systems that don't have wordexp as well.

That being said however, I'm cool with leaving this open for further discussion…

Thoughts?

Thanks,

-Damien

On Dec 6, 2012, at 2:16 AM, Les Aker notifications@github.com wrote:

On Macs, both with system tools and brew, glibtoolize exists but libtoolize does not. I've adjusted the autogen.sh script to account for this so it works in either case.

In the config file (.fwknoprc), there didn't appear to be a way to enable gpg-agent usage. I've added a "USE_GPG_AGENT" option to allow this to be set via the config file.

OSX appears to fail when trying to stat a directory which needs to be expanded, such as "~/.gnupg". This causes the validation check to fail. I've adjusted the check to first expand the path to resolve this issue.

I'm available on Freenode in #cipherdyne if there's anything I need to adjust/fix to get this pulled in.

You can merge this Pull Request by running:

git pull https://github.com/akerl/fwknop mac
Or view, comment on, or merge it at:

#13

Commit Summary

fixed autogen for systems with glibtoolize
Added GPG Agent config file option
fixed path expansion for gpg_home_dir
Added catch to ensure reliable operations
File Changes

M autogen.sh (9)
M client/config_init.c (6)
M lib/fko_encryption.c (12)
Patch Links

https://github.com/mrash/fwknop/pull/13.patch
https://github.com/mrash/fwknop/pull/13.diff

Reply to this email directly or view it on GitHub.

@mrash
mrash commented on 8ea6344 Dec 7, 2012

Hello Les,

Minor comment on the above - gpg_home_dir doesn't point to heap allocated memory, so calling free() against it results in the following from valgrind:

Invalid free() / delete / delete[] / realloc()
at 0x4C2A739: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x4E36ED3: fko_set_gpg_home_dir (fko_encryption.c:665)
by 0x10AC29: main (fwknop.c:243)
Address 0x7feffeee4 is on thread 1's stack

@akerl
akerl commented Dec 7, 2012

Admittedly, I am not a C programmer; I pieced together the above mostly by trial and error and a bit of brain-picking of a coworker. Is the solution to simply not call free() on it? My coworker suggested the free() and 9d477bd commit for the purposes of preventing Bad Things Happening To Memory.

With regards to the path expansion change: my primary concern was that with GPG_HOMEDIR set to /.gnupg, fwknop errors out at that point in the code despite the fact that the rest of the code seems to properly handle "/.gnupg". If I comment out the two return(FKO_ERROR_GPGME_BAD_HOME_DIR) lines, it proceeds just fine. I've done further testing and it seems that the further code also defaults to ~/.gnupg, so removing that line bypasses the problem. If the rest of the code supports expansion of paths, in my opinion the check should be adjusted in some fashion so that it doesn't fail for paths that need expansion; I'm not opposed to allowing that in another way.

@mrash
Owner
mrash commented Dec 9, 2012

Hello Les,

Damien merged in a slight modified version of your gpg agent patch: 5f598bb

@akerl
akerl commented Feb 15, 2013

Works for me :)

Thanks!

@akerl akerl closed this Feb 15, 2013
@akerl akerl deleted the akerl:mac branch May 27, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment