salt: 2016.11.5 -> 2017.7.1, patch fix

[danbst]

Motivation for this change

Apart from regular update, I've changed the libcrypto patch. It didn't work well with salt-ssh (that code failed on remote machines)

saltstack/salt#43350

ping @aneeshusa for review

Things done

• Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
• Built on platform(s)
  • NixOS
  • macOS
  • Linux
• Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
• Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
• Tested execution of all binary files (usually in ./result/bin/)
• Fits CONTRIBUTING.md.

[mention-bot]

@danbst, thanks for your PR! By analyzing the history of the files in this pull request, we identified @aneeshusa, @globin and @dezgeg to be potential reviewers.

[danbst]

I do realise now, that patch will work not as intended. It may break salt installs on non-NixOS machines (because it will find the libcrypto impurely, potentially wrong version)

Unfortunately, original Salt doesn't distinguish host and target when running libcrypto code, so the best solution would be to patch it to copy-over libcrypto.so to target machines. That is a tricky way, and I don't think it should be done on NixOS side. On other hand, Salt devs would say Meh for such a change...

[Mic92]

What if libcrypto would be in LD_PRELOAD of salt executable on nixos?

[danbst]

Unfortunately, python dll loading library doesn't support LD_PRELOAD and LD_LIBRARY_PATH. @domenkozar has found it's implementation when investigated #7307. We should patch python then...

[aneeshusa]

@danbst Thanks for the patch! I haven't yet gotten a chance to take it for a spin but will try to do so soon.
In the interest of getting the 2017.7.1 update in, can you split the libcrypto patch change to a separate PR? It looks like it is unrelated to the update and might take some bikeshedding.

[aneeshusa]

Also I haven't tried this at all, but maybe something like this this is an easier approach than fixing Python dll loading:

try:
   return cdll.LoadLibrary('@libcrypto@')
except OSError:
    if not lib:
        raise OSError('Cannot locate OpenSSL libcrypto')
    return cdll.LoadLibrary('@libcrypto@')

[aneeshusa]

Or we could add a check for if __file__.startswith('/nix/store'), module the storeDir Nix argument which would need to be propagated through for correctness. I don't think we can reference the final output path (which is safer) without causing a recursion/self-reference error.

[danbst]

I'll try the exception catch way, thanks for the pointer. Python except: is almost same as else:, I should never forget that.

But still, I see this as a defect in Salt itself.

• on master node, rsax931.py uses libcrypto with API version, say, X
• salt-ssh copies that file to target node, which may run, for example, Ubuntu 12
• and runs that file. But rsax931.py finds libcrypto with API version, say Y, which doesn't have some features. And fails with some random error

And another problem, now in setup with NixOS on both master and target:

• on master node, rsax931.py uses /nix/store/XXXX-libcrypto.so
• salt-ssh copies that file to target node, which runs NixOS
• and runs that file. But >50% chance that /nix/store/XXXX-libcrypto.so file would be absent there, and real name is /nix/store/YYYY-libcrypto.so.

I don't know whether I should report this upstream. First example is very minor ("update libcrypto on target, what's the problem?"), and second is very Nix specific ("why should we solve your bugs?" and "why the hell would you saltify NixOS?")...

[aneeshusa]

Turns out I already had patched Salt to 2017.7.1 locally and have been using it for a while and forgot, so the Salt update itself LGTM.

In general it's reasonable to run Salt between machines of differing OSes and/or libcrypto versions, so I think the fix here will need to be on the NixOS side. There is already an currently-buggy upstream patch to make find_library return absolute paths (https://bugs.python.org/issue21042) - IMO the best path forward would be to fix that patch up enough to make it work for NixOS. Whatever we do is going to be involved, so please split that to a separate issue or PR so we can get the Salt update merged.