[openssh/openssl] : Apple keychain patch update should have blocked openssl upgrade

[RJVB (René Bertin)]

Evidently I only discover this after upgrading myself:

        # TODO: Update patch 0002-Apple-keychain-integration-other-changes.patch to use OpenSSL 1.1 APIs.

(Introduced during [c15ce48157fd32bd5362ce868b9e32a54ea4d089/macports-ports])

I never realised until now that this patch isn't applied outside of any variants because it so useful, and once you are used to the possibility to store and fetch certificates (or their passphrases) from the keychain you become dependent on it very quickly. (Here's why my local efforts to keep OpenSSL 1.0x and 1.1x installed in parallel pay off, I can simply revert OpenSSH temporarily).

I'll try to find some time to update the patch but will appreciate if someone beats me to it.

[kencu (Ken)]

macports is keeping openssl 1.0 and 1.1 in parallel also.

check out old_openssl PG, for ex.

[RJVB (René Bertin)]

True, I didn't think of that - but was it done in such a way that you can install port:openssl10 and then upgrade port:openssl without an immediate need to rebuild all dependents? ;)

Awkward name for the PG, BTW. I think I mentioned it on the 1.1 PR: this would have been an occasion to write an ssl PG. Such a general PG could also provide the depspecs in an appropriate fashion and contain whatever glue is required to opt in to LibreSSL and whatever other alternative implementations there are.

[neverpanic (Clemens Lang)]

Feel free to update this patch to use OpenSSL 1.1 APIs or find a version somewhere else upstream that has done so already. Maybe Apple did actually update it already and it's on opensource.apple.com?

Given the nature of the OpenSSL update, we can't test every single variant of every port and wait for these to be adapted, I'm sorry.

[RJVB (René Bertin)]

But this issue was identified as evidenced by the comment, and the OSSL update procedure took so long that this could easily have been taken care of it (I presume, I'm not familiar with the code).

I know there's some urgency to security-related upgrades but if they break too many key features you only achieve that users hold off from upgrading, or roll back.

[RJVB (René Bertin)]

opensource.apple.com does have 7.9p1 (identified by Apple's own weird versioning, fortunately there's a version.h file that contains the real version): ​https://opensource.apple.com/source/OpenSSH/OpenSSH-220.231.1/

However, there are no patch files in there. Where did the 0002* patchfile come from?

[yan12125 (Chih-Hsuan Yen)]

# TODO: Update patch 0002-Apple-keychain-integration-other-changes.patch to use OpenSSL 1.1 APIs.

This comment is misleading as that patch is disabled due to incompatibility with OpenBSD's OpenSSH 7.9p1, not incompatibility with OpenSSL 1.1. The last sentence in the commit message of [c15ce48157fd32bd5362ce868b9e32a54ea4d089/macports-ports] is clearer:

Temporarily disabled macOS keychain integration until this can be updated to 7.9p1 APIs.

For example, the main file keychain.c has #include "key.h", but there is no key.h in OpenBSD's OpenSSH 7.9p1.

---

Where did the 0002* patchfile come from?

From ​https://trac.macports.org/ticket/27250

Patch regenerated in more clean format with git

So, most likely it's manually generated.

To get the Apple keychain patch back, there are a few options:

• Comparing OpenSSH-220.231.1 and OpenBSD's OpenSSH 7.9p1 to generate a new patch
• Rebasing the existing patch against OpenSSH 7.9p1
• Combining the above two approaches - generate a new patch and rebase the new patch against OpenSSH 8.0p1, which fixes several security issues found in OpenSSH 7.9 (1) - adding the 'security' keyword to this ticket.

I may or may not have a look into this after fixing other ports that cannot be built with openssl 1.1.

(1) ​https://www.cvedetails.com/vulnerability-list/vendor_id-97/product_id-585/year-2019/Openbsd-Openssh.html

[Ionic (Mihai Moldovan)]

I'm working on that + an update to 8.1p1.

Should come up with something sensible in the next few days.

[RJVB (René Bertin)]

Great, I was getting ready to put myself to that but I don't mind being beat to it :)

If possible it'd probably be a good idea to extract this patch from the variant it is currently coupled with (or else make that variant default).

[Ionic (Mihai Moldovan)]

Yeah, I'll make it a default patch. Makes rebasing the HPN pachset a bit more difficult, but not by much. It's probably worth to always have the keychain integration.

Note that I'll adopt Apples new implementation though, that WILL change parameters (from -m/-M to -A/-K because upstream now uses the previously unused former flags) and might not compile/work on older platforms. Will have to see how that goes.

[Ionic (Mihai Moldovan)]

I could have released the update today, if life were easy.

Sadly, it's not. Apple's new implementation of the keychain integration depends upon new features in their Security framework, including, what seems to be, a private header. I don't know if that's even installed on user systems.

I wasn't able to find a file called SecItemPriv.h anywhere within /System/ on a 10.13 box, but Apple's openssh implementation uses it. They probably can do this because they have special SDK/Frameworks and build everything in an Xcode environment, but for MacPorts, that behavior is not ideal.

I'll need more time to figure out what to do. Maybe I can ship shim definitions just for the needed parts, but I'll have to see whether the Security frameworks (esp. on older platforms) even provide the needed functionality. If not, I'll have to revert the patch to an older, pre-Security-framework version and adapt it myself.

[yan12125 (Chih-Hsuan Yen)]

This might be useful: ​https://opensource.apple.com/source/Security/Security-58286.260.20/keychain/SecItemPriv.h.auto.html

[RJVB (René Bertin)]

In my experience it's near impossible to build the bundles from that site because at the least the assume you have Apple's build environment set up. But who knows, maybe that framework is different ... the whole security (keychain) functionality has not seen much (if any) evolution at all for years. (not in functionality either, which is probably why Google Chrome stopped using it and definitely is the reason why I didn't bother adapting my backend for the KDE5 version of KWallet.)

Has any attempt ever been made to upstream this patch, to your knowledge? IMHO Apple should have done that themselves, so I would find it amusing if they find they have to work around upstreamed functionality (that's compatible with older OS versions) for some future release O:-)

that WILL change parameters (from -m/-M to -A/-K)

I must have been using an older version then, because I'm used to the -A/-K flags.

[Mihai Moldovan <ionic@…>]

In 715635bdfb881e287a52e23b298e379a4e9c03ac/macports-ports (master):

net/{openssh,ssh-copy-id}: update to 8.1p1.

Fixes: #56331
Fixes: #57025
Fixes: #58047
Fixes: #59009
Fixes: #59016

Changes:

• Rebase patches.
• Update to newer HPN patchset version. Based upon the 8.0p1 version 14.18 patch. Add a rebased OpenSSL-1.1-compat patch.
• Switch to new ObjC-based Keychain integration as provided by Apple. Might fail on older platforms. If it does, we will need to bring back the old C-based implementation as an alternative for these.
• Made the keychain integration and launchd startup patch a default one based upon request (and to be consistent with Apple's shipped OpenSSH version).
• Portfile cleanup, don't define compile constants from outside - have autotools do that correctly.
• Clarify where some of the patches come from - and especially for the gsskex patch that it is NOT a single patch taken from one location and rebased against the current OpenSSH version.
• Renamed (now used) -m/-M options to -A/-K for the keychain integration.

[RJVB (René Bertin)]

Great work, thanks. This built flawlessly on 10.9.8 (I did use port:clang-5.0 without even trying /usr/bin/clang but I guess even that wasn't actually necessary).

[Ionic (Mihai Moldovan)]

I worry more about 10.7, 10.6 and such. For 10.9, everything should work out of the box (with the few changes I had to make due to undefined dictionary values).

I admit to not really having tested this, though. I still use the Apple-provided ssh-agent on my machine and using that, the client throws a warning message (invalid/unknown signature(?)), but otherwise seems to work. The ssh-agent typically isn't a huge security problem so changing it doesn't benefit a lot, other than supporting newer features/key types.

[RJVB (René Bertin)]

Replying to Ionic:

I worry more about 10.7, 10.6 and such. For 10.9, everything should work out of the box (with the few changes I had to make due to undefined dictionary values).

I suppose you mean dictionary keys? Those (and other constants that are in fact NSStrings) can usually just be defined in an appropriate place, as long as the code using them can handle not getting the expected result from using them.

I admit to not really having tested this, though. I still use the Apple-provided ssh-agent on my machine and using that, the client throws a warning message (invalid/unknown signature(?)), but otherwise seems to work. The ssh-agent typically isn't a huge security problem so changing it doesn't benefit a lot, other than supporting newer features/key types.

And I admit that I don't really know which ssh-agent I'm using. The keychain feature is (or used to be) implemented solely in the control application, ssh-add. What I reckon happens is that ssh-add fetches the certificates it stored in the keychain and then feeds them to ssh-agent as if you were reading them from file.

[RJVB (René Bertin)]

I just noticed this...

> /opt/local/bin/ssh-add -A
/opt/local/bin/ssh-add: illegal option -- A
usage: ssh-add [options] [file ...]
Options:
  -l          List fingerprints of all identities.
  -E hash     Specify hash algorithm used for fingerprints.
  -L          List public key parameters of all identities.
  -k          Load only keys and not certificates.
  -c          Require confirmation to sign using identities
  -m minleft  Maxsign is only changed if less than minleft are left (for XMSS)
  -M maxsign  Maximum number of signatures allowed (for XMSS)
  -t life     Set lifetime (in seconds) when adding identities.
  -d          Delete identity.
  -D          Delete all identities.
  -x          Lock agent.
  -X          Unlock agent.
  -s pkcs11   Add keys from PKCS#11 provider.
  -e pkcs11   Remove keys provided by PKCS#11 provider.
  -T pubkey   Test if ssh-agent can access matching private key.
  -q          Be quiet after a successful operation.
  -v          Be more verbose.
  -A          Add all identities stored in your macOS keychain.
  -K          Store passphrases in your macOS keychain.
              With -d, remove passphrases from your macOS keychain.
Exit 1

> /usr/bin/ssh-add -A
Added keychain identities.

Seems you missed a small detail ;)

[Ionic (Mihai Moldovan)]

Sync up and update the port. I fixed that about a week ago.

[RJVB (René Bertin)]

Did something change in the way identities are stored in the keychain?

> /usr/bin/ssh-add -A
Added keychain identities.
> /opt/local/bin/ssh-add -A
No identity found in the keychain.