#59397 closed defect (fixed)
openssh @8.1_1: fails to build on 10.6: audit-bsm.c:66:10: fatal error: 'bsm/audit_session.h' file not found
Reported by: | grumpybozo (Bill Cole) | Owned by: | Ionic (Mihai Moldovan) |
---|---|---|---|
Priority: | Normal | Milestone: | |
Component: | ports | Version: | 2.6.1 |
Keywords: | snowleopard lion legacy-os | Cc: | iEFdev |
Port: | openssh |
Description
Root cause appears to be the result of expecting a newer version of the Apple BSM package than Snow Leopard includes.
Attachments (2)
Change History (40)
Changed 5 years ago by grumpybozo (Bill Cole)
comment:1 Changed 5 years ago by mf2k (Frank Schima)
Keywords: | snowleopard added |
---|
comment:2 Changed 5 years ago by kencu (Ken)
The update to 8.1 was expected to cause wreckage. Perhaps Mihai has an idea how to fix it.
comment:3 Changed 5 years ago by kencu (Ken)
Cc: | Ionic added |
---|
I'm not yet sure just where the cutoff for failure is...
comment:4 follow-up: 6 Changed 5 years ago by grumpybozo (Bill Cole)
FWIW: It builds just fine without this patch: 0002-Apple-keychain-integration-other-changes.patch
comment:5 follow-up: 8 Changed 5 years ago by Ionic (Mihai Moldovan)
Cc: | Ionic removed |
---|---|
Owner: | set to Ionic |
Status: | new → accepted |
Summary: | openssh 8.1 will not build on Snow Leopard → openssh @8.1_1: fails to build on 10.6: audit-bsm.c:66:10: fatal error: 'bsm/audit_session.h' file not found |
Not really expected, but there was a possibility.
Yeah, Apple's new implementation of the keychain integration is different nowadays.
Interestingly, 10.6 misses bsm/audit_session.h
. Digging into that might turn out to be difficult... what header files do you have in /usr/include/bsm
?
Also, this warning is a bit concerning, but AFAIK unpatched code:
:info:build sshbuf-misc.c:225:11: warning: implicit declaration of function 'memmem' is invalid in C99 [-Wimplicit-function-declaration] 2274 :info:build if ((p = memmem(sshbuf_ptr(b) + start_offset, 2275 :info:build ^ 2276 :info:build sshbuf-misc.c:225:9: warning: incompatible integer to pointer conversion assigning to 'void *' from 'int' [-Wint-conversion] 2277 :info:build if ((p = memmem(sshbuf_ptr(b) + start_offset, 2278 :info:build ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 2279 :info:build 2 warnings generated.
Something that I should look into as well.
comment:6 Changed 5 years ago by kencu (Ken)
Replying to grumpybozo:
FWIW: It builds just fine without this patch: 0002-Apple-keychain-integration-other-changes.patch
Thanks! That is very helpful information. I was trying to figure out why it kept forcing the use of the keychain.m file even with the -use-keychain=apple
flag taken away.
memmem is replacable via legacysupport
if that is needed.
comment:7 Changed 5 years ago by Ionic (Mihai Moldovan)
Thanks! That is very helpful information. I was trying to figure out why it kept forcing the use of the keychain.m file even with the
-use-keychain=apple
flag taken away.
Oh, yeah, that source file is used unconditionally, even though we'd have a configure option for the integration. Only flags are added conditionally.
That's probably something that I should fix up, but then again the whole patch is OS-X-centric.
memmem is replacable via legacysupport if that is needed.
Nope, we don't need this. Openssh ships a memmem
compat implementation and it's actually also built and linked correctly. It's just the declaration that declaration that is missing, so not a very bad issue.
comment:8 Changed 5 years ago by grumpybozo (Bill Cole)
Replying to Ionic:
Not really expected, but there was a possibility.
Yeah, Apple's new implementation of the keychain integration is different nowadays.
Interestingly, 10.6 misses
bsm/audit_session.h
. Digging into that might turn out to be difficult... what header files do you have in/usr/include/bsm
?
# ls /usr/include/bsm audit.h audit_errno.h audit_filter.h audit_kevents.h audit_socket_type.h libbsm.h audit_domain.h audit_fcntl.h audit_internal.h audit_record.h audit_uevents.h
comment:9 follow-up: 10 Changed 5 years ago by Ionic (Mihai Moldovan)
Okay... I think that audit_session.h
was introduced with 10.7, if I read launchd and other source code correctly.
Just for fun: does grep -HRE 'IS_REMOTE|HAS_TTY|HAS_AUTHENTICATED' /usr/include/bsm
return anything useful?
comment:10 Changed 5 years ago by grumpybozo (Bill Cole)
Replying to Ionic:
Okay... I think that
audit_session.h
was introduced with 10.7, if I read launchd and other source code correctly.
Definitely somewhere between 10.7 and 10.9
Just for fun: does
grep -HRE 'IS_REMOTE|HAS_TTY|HAS_AUTHENTICATED' /usr/include/bsm
return anything useful?
It returns nothing. There are no AU_SESSION_* constants defined in any bsm/* headers on 10.6.8
comment:11 Changed 5 years ago by Mihai Moldovan <ionic@…>
Resolution: | → fixed |
---|---|
Status: | accepted → closed |
comment:12 follow-up: 13 Changed 5 years ago by Ionic (Mihai Moldovan)
Let's see how well this works.
If this doesn't fix building on 10.6, please re-open the bug report and upload the new main.log. We might need multiple iterations.
comment:13 Changed 5 years ago by grumpybozo (Bill Cole)
Replying to Ionic:
Let's see how well this works.
If this doesn't fix building on 10.6, please re-open the bug report and upload the new main.log. We might need multiple iterations.
Fixed the include problem, now there's a new issue. I will attach the new main.log but here's the key excerpt:
ccache /opt/local/bin/clang-mp-9.0 -pipe -Os -arch i386 -pipe -Wunknown-warning-option -Qunused-arguments -Wall -Wpointer-arith -Wuninitialized -Wsign-compare -Wformat-security -Wsizeof-pointer-memaccess -Wno-pointer-sign -Wno-unused-result -fno-strict-aliasing -D_FORTIFY_SOURCE=2 -ftrapv -fno-builtin-memset -fstack-protector-strong -fPIE -I. -I. -I/opt/local/include -I/opt/local/include -I/opt/local/include -DBROKEN_STRNVIS=1 -D__APPLE_SANDBOX_NAMED_EXTERNAL__ -D__APPLE_API_STRICT_CONFORMANCE -I/opt/local/include/editline -I/opt/local/include -I/opt/local/include -D__APPLE_KEYCHAIN__ -D__APPLE_MEMBERSHIP__ -D__APPLE_TMPDIR__ -D__APPLE_LAUNCHD__ -DSSHDIR=\"/opt/local/etc/ssh\" -D_PATH_SSH_PROGRAM=\"/opt/local/bin/ssh\" -D_PATH_SSH_ASKPASS_DEFAULT=\"/opt/local/libexec/ssh-askpass\" -D_PATH_SFTP_SERVER=\"/opt/local/libexec/sftp-server\" -D_PATH_SSH_KEY_SIGN=\"/opt/local/libexec/ssh-keysign\" -D_PATH_SSH_PKCS11_HELPER=\"/opt/local/libexec/ssh-pkcs11-helper\" -D_PATH_SSH_PIDDIR=\"/opt/local/var/run\" -D_PATH_PRIVSEP_CHROOT_DIR=\"/var/empty\" -DHAVE_CONFIG_H -c sshpty.c -o sshpty.o In file included from keychain.m:33: In file included from /System/Library/Frameworks/Foundation.framework/Headers/Foundation.h:81: In file included from /System/Library/Frameworks/Foundation.framework/Headers/NSURLError.h:17: In file included from /System/Library/Frameworks/CoreServices.framework/Headers/CoreServices.h:21: In file included from /System/Library/Frameworks/CoreServices.framework/Frameworks/AE.framework/Headers/AE.h:20: In file included from /System/Library/Frameworks/CoreServices.framework/Frameworks/CarbonCore.framework/Headers/CarbonCore.h:85: In file included from /System/Library/Frameworks/CoreServices.framework/Frameworks/CarbonCore.framework/Headers/Components.h:32: In file included from /System/Library/Frameworks/CoreServices.framework/Frameworks/CarbonCore.framework/Headers/Files.h:61: /usr/include/sys/acl.h:39:19: error: use of undeclared identifier 'KAUTH_VNODE_READ_DATA' ACL_READ_DATA = KAUTH_VNODE_READ_DATA, ^ /usr/include/sys/acl.h:40:23: error: use of undeclared identifier 'KAUTH_VNODE_LIST_DIRECTORY' ACL_LIST_DIRECTORY = KAUTH_VNODE_LIST_DIRECTORY, ^ /usr/include/sys/acl.h:41:20: error: use of undeclared identifier 'KAUTH_VNODE_WRITE_DATA' ACL_WRITE_DATA = KAUTH_VNODE_WRITE_DATA, ^ /usr/include/sys/acl.h:42:18: error: use of undeclared identifier 'KAUTH_VNODE_ADD_FILE' ACL_ADD_FILE = KAUTH_VNODE_ADD_FILE, ^ /usr/include/sys/acl.h:43:17: error: use of undeclared identifier 'KAUTH_VNODE_EXECUTE' ACL_EXECUTE = KAUTH_VNODE_EXECUTE, ^ /usr/include/sys/acl.h:44:16: error: use of undeclared identifier 'KAUTH_VNODE_SEARCH' ACL_SEARCH = KAUTH_VNODE_SEARCH, ^ /usr/include/sys/acl.h:45:16: error: use of undeclared identifier 'KAUTH_VNODE_DELETE'; did you mean 'DISPATCH_VNODE_DELETE'? ACL_DELETE = KAUTH_VNODE_DELETE, ^ /usr/include/dispatch/source.h:216:2: note: 'DISPATCH_VNODE_DELETE' declared here DISPATCH_VNODE_DELETE = 0x1, ^
Changed 5 years ago by grumpybozo (Bill Cole)
Attachment: | main.2.log added |
---|
build log after include fix.
comment:14 follow-up: 15 Changed 5 years ago by Ionic (Mihai Moldovan)
Resolution: | fixed |
---|---|
Status: | closed → reopened |
I hoped that these errors would only be follow-up errors due to the compiler stopping to include other files after the first error.
It looks like the CarbonCore framework headers are broken on 10.6.
I don't get what's wrong there, though. The code looks fine...
/System/Library/Frameworks/CoreServices.framework/Frameworks/CarbonCore.framework/Headers/Files.h
includes sys/acl.h, which:
#ifndef _SYS_ACL_H #define _SYS_ACL_H #include <sys/kauth.h>
and in sys/kauth.h, there's this hunk:
/* Actions, also rights bits in an ACE */ #if defined(KERNEL) || defined (_SYS_ACL_H) #define KAUTH_VNODE_READ_DATA (1<<1) #define KAUTH_VNODE_LIST_DIRECTORY KAUTH_VNODE_READ_DATA #define KAUTH_VNODE_WRITE_DATA (1<<2) #define KAUTH_VNODE_ADD_FILE KAUTH_VNODE_WRITE_DATA #define KAUTH_VNODE_EXECUTE (1<<3) #define KAUTH_VNODE_SEARCH KAUTH_VNODE_EXECUTE #define KAUTH_VNODE_DELETE (1<<4) #define KAUTH_VNODE_APPEND_DATA (1<<5) #define KAUTH_VNODE_ADD_SUBDIRECTORY KAUTH_VNODE_APPEND_DATA #define KAUTH_VNODE_DELETE_CHILD (1<<6) #define KAUTH_VNODE_READ_ATTRIBUTES (1<<7) #define KAUTH_VNODE_WRITE_ATTRIBUTES (1<<8) #define KAUTH_VNODE_READ_EXTATTRIBUTES (1<<9) #define KAUTH_VNODE_WRITE_EXTATTRIBUTES (1<<10) #define KAUTH_VNODE_READ_SECURITY (1<<11) #define KAUTH_VNODE_WRITE_SECURITY (1<<12) #define KAUTH_VNODE_TAKE_OWNERSHIP (1<<13)
So, theoretically, everything looks fine. The only catch is that the whole sys/kauth.h
file is essentially wrapped into an #ifdef __APPLE_API_EVOLVING
clause.
This should also not a problem, though, since sys/kauth.h
uses:
#include <sys/appleapiopts.h>
which then:
#ifndef __APPLE_API_EVOLVING #define __APPLE_API_EVOLVING #endif /* __APPLE_API_EVOLVING */
I'm a bit stumped. What is causing this? Everything looks alright.
comment:15 Changed 5 years ago by grumpybozo (Bill Cole)
Replying to Ionic:
I hoped that these errors would only be follow-up errors due to the compiler stopping to include other files after the first error.
It looks like the CarbonCore framework headers are broken on 10.6.
Nope :)
I don't get what's wrong there, though. The code looks fine...
[...]
So, theoretically, everything looks fine. The only catch is that the whole
sys/kauth.h
file is essentially wrapped into an#ifdef __APPLE_API_EVOLVING
clause.This should also not a problem, though, since
sys/kauth.h
uses:#include <sys/appleapiopts.h>which then:
#ifndef __APPLE_API_EVOLVING #define __APPLE_API_EVOLVING #endif /* __APPLE_API_EVOLVING */
Look before that...
I'm a bit stumped. What is causing this? Everything looks alright.
From the Portfile:
openssh root# grep -nC5 __APPLE_API_STRICT_CONFORMANCE Portfile 80- # the order of arguments to strnvis and considers everyone else to be broken. 81- configure.cppflags-append -DBROKEN_STRNVIS=1 82- 83- # Use Apple's sandboxing feature 84- configure.cppflags-append -D__APPLE_SANDBOX_NAMED_EXTERNAL__ \ 85: -D__APPLE_API_STRICT_CONFORMANCE
If I remove that second -D, the build gets further but still ultimately dies:
ccache /opt/local/bin/clang-mp-9.0 -pipe -Os -arch i386 -pipe -Wunknown-warning-option -Qunused-arguments -Wall -Wpointer-arith -Wuninitialized -Wsign-compare -Wformat-security -Wsizeof-pointer-memaccess -Wno-pointer-sign -Wno-unused-result -fno-strict-aliasing -D_FORTIFY_SOURCE=2 -ftrapv -fno-builtin-memset -fstack-protector-strong -fPIE -I. -I. -I/opt/local/include -I/opt/local/include -I/opt/local/include -DBROKEN_STRNVIS=1 -D__APPLE_SANDBOX_NAMED_EXTERNAL__ -I/include -I/opt/local/include/editline -I/opt/local/include -I/opt/local/include -D__APPLE_KEYCHAIN__ -D__APPLE_MEMBERSHIP__ -D__APPLE_TMPDIR__ -D__APPLE_LAUNCHD__ -DSSHDIR=\"/opt/local/etc/ssh\" -D_PATH_SSH_PROGRAM=\"/opt/local/bin/ssh\" -D_PATH_SSH_ASKPASS_DEFAULT=\"/opt/local/libexec/ssh-askpass\" -D_PATH_SFTP_SERVER=\"/opt/local/libexec/sftp-server\" -D_PATH_SSH_KEY_SIGN=\"/opt/local/libexec/ssh-keysign\" -D_PATH_SSH_PKCS11_HELPER=\"/opt/local/libexec/ssh-pkcs11-helper\" -D_PATH_SSH_PIDDIR=\"/opt/local/var/run\" -D_PATH_PRIVSEP_CHROOT_DIR=\"/var/empty\" -DHAVE_CONFIG_H -c auth2-kbdint.c -o auth2-kbdint.o In file included from keychain.m:35: ./SecItemPriv-shim.h:57:10: fatal error: 'xpc/xpc.h' file not found #include <xpc/xpc.h> ^~~~~~~~~~~ 1 error generated. make: *** [keychain.o] Error 1
There's no file or directory anywhere on 10.6 with 'xpc' in its name.
I'm not much of a C coder or MacPorts wizard, but is it possible to just make all the changes since the 7.9p1_2 version of the port conditional on the OS version being 10.7 or greater? Because this stuff just isn't working on 10.6 and likely never will.
comment:16 Changed 5 years ago by Ionic (Mihai Moldovan)
Right, the __APPLE_API_STRICT_CONFORMANCE
macro explains a lot.
Note that 10.7+ systems are not affected by this issue because Apple dropped usage of the KAUTH_
macros in later sys/acl.h headers and instead defines internal macros. It looks like they never actually intended to let __APPLE_API_STRICT_CONFORMANCE
influence this.
Luckily, we only really need this to unbreak the sandbox feature on older systems, c.f., #48981. Crucially, I can just drop it in keychain.m
and should be good.
There's no file or directory anywhere on 10.6 with 'xpc' in its name.
Thanks for the heads-up. XPC is a somewhat new (10.7+) interprocess communication mechanism backed by (the sadly closed-source) libSystem.
And again, luckily, we don't need anything that the library or header provides.
I'm not much of a C coder or MacPorts wizard, but is it possible to just make all the changes since the 7.9p1_2 version of the port conditional on the OS version being 10.7 or greater? Because this stuff just isn't working on 10.6 and likely never will.
Theoretically yes, but I don't want to. I'm pretty sure that it should work, even on older systems. If a real showstopper shows up, I can still revert to the old C-based keychain integration for older systems only. I'm not at that point yet.
comment:17 Changed 5 years ago by Mihai Moldovan <ionic@…>
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
comment:18 Changed 5 years ago by grumpybozo (Bill Cole)
Still broken: another missing header...
ccache /opt/local/bin/clang-mp-9.0 -pipe -Os -arch i386 -pipe -Wunknown-warning-option -Qunused-arguments -Wall -Wpointer-arith -Wuninitialized -Wsign-compare -Wformat-security -Wsizeof-pointer-memaccess -Wno-pointer-sign -Wno-unused-result -fno-strict-aliasing -D_FORTIFY_SOURCE=2 -ftrapv -fno-builtin-memset -fstack-protector-strong -fPIE -I. -I. -I/opt/local/include -I/opt/local/include -I/opt/local/include -DBROKEN_STRNVIS=1 -D__APPLE_SANDBOX_NAMED_EXTERNAL__ -D__APPLE_API_STRICT_CONFORMANCE -I/include -I/opt/local/include/editline -I/opt/local/include -I/opt/local/include -D__APPLE_KEYCHAIN__ -D__APPLE_MEMBERSHIP__ -D__APPLE_TMPDIR__ -D__APPLE_LAUNCHD__ -DSSHDIR=\"/opt/local/etc/ssh\" -D_PATH_SSH_PROGRAM=\"/opt/local/bin/ssh\" -D_PATH_SSH_ASKPASS_DEFAULT=\"/opt/local/libexec/ssh-askpass\" -D_PATH_SFTP_SERVER=\"/opt/local/libexec/sftp-server\" -D_PATH_SSH_KEY_SIGN=\"/opt/local/libexec/ssh-keysign\" -D_PATH_SSH_PKCS11_HELPER=\"/opt/local/libexec/ssh-pkcs11-helper\" -D_PATH_SSH_PIDDIR=\"/opt/local/var/run\" -D_PATH_PRIVSEP_CHROOT_DIR=\"/var/empty\" -DHAVE_CONFIG_H -c sandbox-rlimit.c -o sandbox-rlimit.o In file included from keychain.m:60: ./SecItemPriv-shim.h:63:10: fatal error: 'Security/SecTask.h' file not found #include <Security/SecTask.h> ^~~~~~~~~~~~~~~~~~~~ ./SecItemPriv-shim.h:63:10: note: did not find header 'SecTask.h' in framework 'Security' (loaded from '/System/Library/Frameworks') 1 error generated. make: *** [keychain.o] Error 1
comment:19 Changed 5 years ago by Ionic (Mihai Moldovan)
Resolution: | fixed |
---|---|
Status: | closed → reopened |
Yet another header we don't actually need and that I failed to check. So, good again.
comment:20 Changed 5 years ago by Mihai Moldovan <ionic@…>
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
comment:21 Changed 5 years ago by grumpybozo (Bill Cole)
Round Four!
Including <Security/SecItem.h> fixes some of this, but leaves kSecClassGenericPassword undefined, as it is only in 10.7+
ccache /opt/local/bin/clang-mp-9.0 -o sftp-server sftp-server.o sftp-common.o sftp-realpath.o sftp-server-main.o -L. -Lopenbsd-compat/ -L/opt/local/lib -L/opt/local/lib -L/opt/local/lib -Wl,-headerpad_max_install_names -Wl,-search_paths_first -arch i386 -fstack-protector-strong -pie -lssh -lopenbsd-compat -lcrypto -lbsm -lz -L/opt/local/lib -Wl,-headerpad_max_install_names -arch i386 -L/opt/local/lib -L/lib -lssl -lcrypto -lldns -lresolv ccache /opt/local/bin/clang-mp-9.0 -o sftp progressmeter.o sftp.o sftp-client.o sftp-common.o sftp-glob.o -L. -Lopenbsd-compat/ -L/opt/local/lib -L/opt/local/lib -L/opt/local/lib -Wl,-headerpad_max_install_names -Wl,-search_paths_first -arch i386 -fstack-protector-strong -pie -lssh -lopenbsd-compat -lcrypto -lbsm -lz -L/opt/local/lib -Wl,-headerpad_max_install_names -arch i386 -L/opt/local/lib -L/lib -lssl -lcrypto -lldns -lresolv -L/opt/local/lib -ledit clang: warning: argument unused during compilation: '-pie' [-Wunused-command-line-argument] clang: warning: argument unused during compilation: '-pie' [-Wunused-command-line-argument] ld: warning: directory not found for option '-L/lib' ld: warning: directory not found for option '-L/lib' keychain.m:83:15: error: use of undeclared identifier 'kSecClass' (id)kSecClass: (id)kSecClassGenericPassword, ^ keychain.m:94:8: warning: implicit declaration of function 'SecItemCopyMatching' is invalid in C99 [-Wimplicit-function-declaration] ret = SecItemCopyMatching((CFDictionaryRef)searchQuery, (CFTypeRef *)&passphraseData); ^ keychain.m:141:9: error: use of undeclared identifier 'kSecClass' (id)kSecClass: (id)kSecClassGenericPassword, ^ keychain.m:152:44: error: use of undeclared identifier 'kSecReturnRef' NSMutableDictionary *searchQuery = [@{(id)kSecReturnRef: @YES} mutableCopy]; ^ keychain.m:156:8: warning: implicit declaration of function 'SecItemCopyMatching' is invalid in C99 [-Wimplicit-function-declaration] ret = SecItemCopyMatching((CFDictionaryRef)searchQuery, &searchResults); ^ keychain.m:172:33: error: use of undeclared identifier 'kSecValueData' NSDictionary *changes = @{(id)kSecValueData: [NSData dataWithBytesNoCopy: (void *)passphrase length: strlen(passphrase) freeWhenDone: NO]}; ^ keychain.m:174:9: warning: implicit declaration of function 'SecItemUpdate' is invalid in C99 [-Wimplicit-function-declaration] ret = SecItemUpdate((CFDictionaryRef)updateQuery, (CFDictionaryRef)changes); ^ keychain.m:181:42: error: use of undeclared identifier 'kSecValueData' NSMutableDictionary *addQuery = [@{(id)kSecValueData: [NSData dataWithBytesNoCopy: (void *)passphrase length: strlen(passphrase) freeWhenDone: NO]} mutableCopy]; ^ keychain.m:184:9: warning: implicit declaration of function 'SecItemAdd' is invalid in C99 [-Wimplicit-function-declaration] ret = SecItemAdd((CFDictionaryRef)addQuery, NULL); ^ keychain.m:209:15: error: use of undeclared identifier 'kSecClass' (id)kSecClass: (id)kSecClassGenericPassword, ^ keychain.m:218:8: warning: implicit declaration of function 'SecItemDelete' is invalid in C99 [-Wimplicit-function-declaration] ret = SecItemDelete((CFDictionaryRef)searchQuery); ^ keychain.m:235:9: error: use of undeclared identifier 'kSecClass' (id)kSecClass: (id)kSecClassGenericPassword, ^ keychain.m:245:8: warning: implicit declaration of function 'SecItemCopyMatching' is invalid in C99 [-Wimplicit-function-declaration] err = SecItemCopyMatching((CFDictionaryRef)searchQuery, (CFTypeRef *)&searchResults); ^ keychain.m:255:48: error: use of undeclared identifier 'kSecAttrAccount' NSString *accountString = itemAttributes[(id)kSecAttrAccount]; ^ 6 warnings and 8 errors generated. make: *** [keychain.o] Error 1
comment:22 Changed 5 years ago by grumpybozo (Bill Cole)
Resolution: | fixed |
---|---|
Status: | closed → reopened |
comment:23 Changed 5 years ago by iEFdev
Cc: | iEFdev added |
---|
comment:24 Changed 5 years ago by iEFdev
If you haven't already, I assume you'll run into the same error I got with kSecAttrAccessGroup
(10.9+) in #59424 later on. Tried to solve it, but ran into another one.
comment:25 Changed 5 years ago by Ionic (Mihai Moldovan)
Okay: SecItem.h
is being included by Security.h
on later platforms, but not on 10.6. They might have forgotten the include or just didn't need it.
kSecClassGenericPassword
is declared in SecItemPriv.h on 10.6 (just like kSecAttrAccessGroup
), so I'll need to add it to the shim.
I wonder if that'll work, because SecItem.h includes this comment:
IMPORTANT: on Mac OS X 10.6, only items of class kSecClassInternetPassword are currently supported.
Hopefully that's not true for that part of the interface, or at least only accurate for the public interface, but I know that it can't generally be true because other functions actually were able to use GenericPasswords, c.f., SecKeychain.h.
Let's go for the next run...
comment:26 Changed 5 years ago by Mihai Moldovan <ionic@…>
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
comment:27 Changed 5 years ago by grumpybozo (Bill Cole)
Resolution: | fixed |
---|---|
Status: | closed → reopened |
We've reached the point where I'm entirely mystified by the errors, which I guess means we're almost there...
ccache /opt/local/bin/clang-mp-9.0 -o sftp progressmeter.o sftp.o sftp-client.o sftp-common.o sftp-glob.o -L. -Lopenbsd-compat/ -L/opt/local/lib -L/opt/local/lib -L/opt/local/lib -Wl,-headerpad_max_install_names -Wl,-search_paths_first -arch i386 -fstack-protector-strong -pie -lssh -lopenbsd-compat -lcrypto -lbsm -lz -L/opt/local/lib -Wl,-headerpad_max_install_names -arch i386 -L/opt/local/lib -L/lib -lssl -lcrypto -lldns -lresolv -L/opt/local/lib -ledit clang: warning: argument unused during compilation: '-pie' [-Wunused-command-line-argument] ld: warning: directory not found for option '-L/lib' clang: warning: argument unused during compilation: '-pie' [-Wunused-command-line-argument] clang: warning: argument unused during compilation: '-pie' [-Wunused-command-line-argument] ld: warning: directory not found for option '-L/lib' ld: warning: directory not found for option '-L/lib' keychain.m:100:32: error: unexpected type name 'BOOL': expected expression (id)kSecReturnData: @YES}; ^ /usr/include/objc/objc.h:49:26: note: expanded from macro 'YES' #define YES (BOOL)1 ^ keychain.m:160:60: error: unexpected type name 'BOOL': expected expression NSMutableDictionary *searchQuery = [@{(id)kSecReturnRef: @YES} mutableCopy]; ^ /usr/include/objc/objc.h:49:26: note: expanded from macro 'YES' #define YES (BOOL)1 ^ keychain.m:250:32: error: unexpected type name 'BOOL': expected expression (id)kSecReturnAttributes: @YES, ^ /usr/include/objc/objc.h:49:26: note: expanded from macro 'YES' #define YES (BOOL)1 ^ keychain.m:263:29: error: expected method to read dictionary element not found on object of type 'NSDictionary *' NSString *accountString = itemAttributes[(id)kSecAttrAccount]; ^ 4 errors generated. make: *** [keychain.o] Error 1 make: Leaving directory `/opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_net_openssh/openssh/work/openssh-8.1p1' Command failed: cd "/opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_net_openssh/openssh/work/openssh-8.1p1" && /usr/bin/make -j4 -w all Exit code: 2
comment:28 Changed 5 years ago by Ionic (Mihai Moldovan)
Keywords: | lion legacy-os added |
---|
This is so weird. I had hoped that these errors only come from Eric messing up the kSecAttrAccessGroup
hiding (mostly because he introduced parse errors by wrapping these elements when they were last and hence included the terminating curly brace }
).
I have to admit that ObjC always has been - and remains - a very weird language to me that I fail to understand.
My best guess currently is that 10.7- bail out because the code uses a new feature that is called "Object Subscripting" and requires explicit compiler and library support - crucially, stringifying stuff isn't the correct fix for that.
For instance, NSString *accountString = itemAttributes[(id)kSecAttrAccount];
is supposed to search the dictionary itemAttributes
for the given key and return exactly that key. Stringifying it won't do that but just assign a literal string to accountString
that does not contain actual data.
I guess I could either try to
- rewrite this stuff to not use Object Subscripting but to create and access elements directly, or
- implement Object Subscripting via a shim and depend upon a newer clang compiler that supports this feature.
I think that the latter option is better because it means less code changes (to the original code, at least), and, if I remember correctly, I already did something like that in the past and it did work, because Object Subscripting does not depend upon ARC (or libarclite).
Guess I have to dig into that again...
comment:29 Changed 5 years ago by Mihai Moldovan <ionic@…>
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
comment:30 Changed 5 years ago by iEFdev
Progress. :) That last one with itemAttributes
is gone, but the 3 @YES
are still erroring out.
Both ksecreturnattributes and ksecreturnref says CFString
, so @"YES"
? But also, CFString (itself) says 10.10+, so maybe thats what's missing?
comment:31 follow-ups: 32 33 Changed 5 years ago by Ionic (Mihai Moldovan)
CFString
is way old. The latter webpage lists 10.0(!) as the first supported version.
Hmm, kSecReturnAttributes
doesn't make sense... the (ObjC) documentation says "Boolean" and the discussion also mentions CFBooleanRef
, but the prototype really lists CFStringRef
. Oddly the code compiles fine on my 10.9 machine, and the actual prototype in the header file says const CFTypeRef
, which is an untyped version used as a placeholder. The bridgesupport file also lists its type as "@" which sounds like a CFBooleanRef
(or just any *Ref
, really?) to me...
Then again, looking at the most recent version of SecItem.h in the Security framework, they really did change the declaration to CFStringRef
for whatever reason. I really don't get it.
Since the code works on 10.8+, that's probably not an issue.
Can you please try to change the @YES
parts (for kSecReturnAttributes
and kSecReturnRef
only) to @(YES)
and see if that compiles on 10.7? (Maybe it's just syntactic sugar that is missing here.)
If it doesn't, replace that with [NSNumber numberWithBool:YES]
. (@YES
should be equivalent to this statement.)
And if that also doesn't, let's try [[NSNumber numberWithBool:YES] stringValue]
. (That's essentially @YES
converted to an NSString
.)
I can't test this reasonably myself.
comment:32 Changed 5 years ago by iEFdev
I'll try that. Thanks! :+1:
Replying to Ionic:
the @YES parts (for kSecReturnAttributes and kSecReturnRef only) to @(YES)
…and not on the kSecReturnData
on L116? (the 1st err)
@YES -> @(YES)
worked. If the same applies for the kSecReturnData
as well == no errs. It passed port build openssh
.
comment:33 Changed 5 years ago by grumpybozo (Bill Cole)
Replying to Ionic:
Can you please try to change the
@YES
parts (forkSecReturnAttributes
andkSecReturnRef
only) to@(YES)
and see if that compiles on 10.7? (Maybe it's just syntactic sugar that is missing here.)
Based on an explanation I found on StackExchange (a phrase that makes me cringe...) indicating that it was just a 'syntactic sugar' change in objc.h (which I confirmed) I changed all instances of @YES
to @(YES)
in 0002-Apple-keychain-integration-other-changes.patch and got a successful build on 10.6.
comment:34 Changed 5 years ago by Ionic (Mihai Moldovan)
Oh, I see, so that's the whole magic. On my 10.9 box, the macro substitutes ((BOOL)1)
, while on 10.7 and below it seems to substitute just (BOOL)1
. Meh.
Will fix that up for shared code paths.
Good that it compiles now, but that said, I have no idea if it also works correctly. It's entirely possible that it doesn't and that Eric just saw his system-provided ssh-agent
daemon working correctly (which is not surprising given it's the old Apple-provided version that came with the operating system, unless he changed it).
Thank you for all the testing sorry for the back-and-forth. This update took me more two weeks to complete, ugh.
If neither of you actually use the Keychain integration, don't go the extra mile to test it. I'm sure that someone will eventually complain if it doesn't work correctly.
comment:35 Changed 5 years ago by Mihai Moldovan <ionic@…>
comment:36 follow-up: 37 Changed 5 years ago by iEFdev
Thanks! It built fine now. :+1:
Been doing some quick tests to give some feedback on the install.
It installed fine. Made a testkey with ssh-keygen and was then about to add the key with ssh-add.
$ sudo port upgrade openssh ---> // ... // $ port installed openssh The following ports are currently installed: openssh @8.1p1_1+gsskex+kerberos5+ldns+xauth (active) $ ssh-keygen -t ed25519 -o -a 100 -f ~/.ssh/id_test -C "Eric F :: $(date "+%F")" Generating public/private ed25519 key pair. Enter passphrase (empty for no passphrase): Enter same passphrase again: Your identification has been saved in $HOME/.ssh/id_test. Your public key has been saved in $HOME/.ssh/id_test.pub. The key fingerprint is: SHA256:xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx Eric F :: 2019-10-26 The key's randomart image is: +--[ED25519 256]--+ | o.B=+ | | * +. . | | . +E . . | |+ . .. ..o. | | * o . oS+.. | | O o o.=o+ | | o +.o =.+. | | oo .+ =. . | | ... .o.=o | +----[SHA256]-----+ $ ssh-add -K "~/.ssh/id_test" 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.
So, it didn't want to add the key. It just returned the list of options.
But the output looks ok, with -A, -K [-d]
included.
I could ssh/login to a webhost, and push with git, run a backupscript (rsync) - so it seems like it can read the keychain anyway.
Notice 1: The man pages… The output is different between, man ssh-add
and if you open it in a separate window (like: open x-man-page://ssh-add
). Both with same date in the bottom (Oct 26, 2019). That was odd, or does the man pages use a dynamic date value (todays date)? ...like it's loading an old cached one with current date added.
Notice 2: I saw briefly duing the config/build that the process creates a folder with a symlink: ../work/include/security
. It looks like a dead symlink. When looking at it, it symlinks to /usr/include/pam
:
$ ls -Ahl /opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_net_openssh/openssh/work/include/security lrwxr-xr-x 1 macports admin 16B Oct 26 06:18 /opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_net_openssh/openssh/work/include/security -> /usr/include/pam $ ls -Ahl /usr/include/pam ls: /usr/include/pam: No such file or directory
There's nothing named *pam*
in /usr/include
, but there is a /usr/include/security
folder which includes the pam files.
Perhaps there are different locations in different OS X/macOS versions?
Don't know if that one is important or not, but thought I should mention it (as a notice).
So, adding keys doesn't work. (Perhaps we should make a separate ticket for that?)
Anyway… I tried to see where the code was added (for -K
), but it actually looks like the patches only fixed the man pages and never adds it as an option. Like it's not implemented.
Reproduce with:
sudo port patch openssh # goto: ../work/openssh-8.1p1/ssh-add.c # Line: 641 ->
Can you run: ssh-add -K /path/to/key
on your 10.9?
In worst case (until it's fixed) I can always use the old bundled one to add it to the keychain.
comment:37 follow-up: 38 Changed 5 years ago by Ionic (Mihai Moldovan)
Replying to iEFdev:
It installed fine. Made a testkey with ssh-keygen and was then about to add the key with ssh-add.
[...]
So, it didn't want to add the key. It just returned the list of options.
But the output looks ok, with
-A, -K [-d]
included.
Whoops, yes, it looks like I missed a few hunks in ssh-add.c
, especially the keychain disabling/enabling bit, so it's always disabled currently. The -A
/-K
options likewise aren't handled at all.
Will need to fix that up at some point. My machine just crashed, though, so I won't do it right away.
I could ssh/login to a webhost, and push with git, run a backupscript (rsync) - so it seems like it can read the keychain anyway.
That highly depends on where the key is coming from. If it came via ssh-agent, then that only tells us that the running ssh-agent either has the key already cached or can read the keychain. The running ssh-agent is typically the Apple-/system-provided one, though, and not the one installed via MacPorts, so this would be testing something completely different. If the key came from a file directly, neither an agent nor the keychain would have been involved either (maybe minus a passphrase stored within the keychain, iff the private key was encrypted).
Notice 1: The man pages… The output is different between,
man ssh-add
and if you open it in a separate window (like:open x-man-page://ssh-add
). Both with same date in the bottom (Oct 26, 2019). That was odd, or does the man pages use a dynamic date value (todays date)? ...like it's loading an old cached one with current date added.
Hm, I can't tell right now. man
itself shouldn't be caching anything as far as I remember, but I don't know what open
does with an x-man-page
URL.
Today's date isn't surprising, because that's when the man page was built. So at least that should be normal. It should, theoretically, be different tomorrow.
Notice 2: I saw briefly duing the config/build that the process creates a folder with a symlink:
../work/include/security
. It looks like a dead symlink. When looking at it, it symlinks to/usr/include/pam
:[...]
There's nothing named
*pam*
in/usr/include
, but there is a/usr/include/security
folder which includes the pam files.Perhaps there are different locations in different OS X/macOS versions?
Don't know if that one is important or not, but thought I should mention it (as a notice).
Good catch. We've been dragging this symlink around for quite some time now. According to the Portfile comments, it sounds like PAM stuff was located in /usr/include/pam
on (some?) OS X versions, instead of the more-standard /usr/include/security
directory.
I can't reproduce that on 10.9 though, and if even 10.7 uses /usr/include/security
, there's probably no sense in keeping that around. Not sure about 10.6 and specifically 10.5 and 10.4, though. Apple might have moved things to security
with 10.6 or something like that.
But, yes, that shouldn't cause any problems.
comment:38 Changed 5 years ago by iEFdev
Replying to Ionic:
That highly depends on where the key is coming from. If it came via ssh-agent, then that only tells us that the running ssh-agent either has the key already cached or can read the keychain. The running ssh-agent is typically the Apple-/system-provided one, though, and not the one installed via MacPorts, so this would be testing something completely different. If the key came from a file directly, neither an agent nor the keychain would have been involved either (maybe minus a passphrase stored within the keychain, iff the private key was encrypted).
Yes, the access might be from the builtin one. I don't think it's cached (at the time). I ran a restart before, (actually a safe start and then a restart). I don't have any passwordless keys. I always make the key(s) with a long password (128, gen by KeePassX) and then add it to keychain and then setup an ssh shortcut to use with it (eg ssh mySrv
). And of course, one key per login.
Hm, I can't tell right now.
man
itself shouldn't be caching anything as far as I remember, but I don't know whatopen
does with anx-man-page
URL.
open x-man-page://<prog>
, is what happens when you right-click on port
(for example) inside the terminal window, and chose “Open man Page” and it opens up in a new window. (I use it with a function instead, to be able stay on the keyboard.)
It's was probably a MANPATH
issue. I cleared it out today. Had some old paths that might could've interfered. It shows the same man page in both now.
Today's date isn't surprising, because that's when the man page was built. So at least that should be normal. It should, theoretically, be different tomorrow.
No, the date wasn't surpring, but it was with the same date on both - since one of them is showing the old one. Actually, today it show todays date: Oct 27, 2019. Even though the date is set in the file to: .Dd $Mdocdate: January 21 2019 $
. Found this post, and it seems like that is a bug with the macro expansion. Installed groff
and now it shows the correct date.
I can't reproduce that on 10.9 though, and if even 10.7 uses
/usr/include/security
, there's probably no sense in keeping that around. Not sure about 10.6 and specifically 10.5 and 10.4, though. Apple might have moved things tosecurity
with 10.6 or something like that.
Yes, 10.6+ seems right. This one suggests that to.
main.log for openssh build attempt