Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#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)

main.log (374.3 KB) - added by grumpybozo (Bill Cole) 5 years ago.
main.log for openssh build attempt
main.2.log (1.0 MB) - added by grumpybozo (Bill Cole) 5 years ago.
build log after include fix.

Download all attachments as: .zip

Change History (40)

Changed 5 years ago by grumpybozo (Bill Cole)

Attachment: main.log added

main.log for openssh build attempt

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.

Last edited 5 years ago by kencu (Ken) (previous) (diff)

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 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 Changed 5 years ago by Ionic (Mihai Moldovan)

Cc: Ionic removed
Owner: set to Ionic
Status: newaccepted
Summary: openssh 8.1 will not build on Snow Leopardopenssh @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 in reply to:  4 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 in reply to:  5 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 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 in reply to:  9 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: acceptedclosed

In c324770cbf372776c5aa882d989747af73bec080/macports-ports (master):

net/openssh: (hopefully) fix build failures on 10.6-.

Only 10.7 introduced the audit-session.h header, so we can't rely on it
on older platforms. Shouldn't terribly matter, since older Apple patches
didn't set the newly introduced flags either.

Also, while at it, fix a compiler warning due to memmem() not being
declared - just a typo in the code.

No revbump since this only fixes build failures or warnings.

Fixes: #59397

comment:12 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 in reply to:  12 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 Changed 5 years ago by Ionic (Mihai Moldovan)

Resolution: fixed
Status: closedreopened

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 in reply to:  14 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: reopenedclosed

In 31b5e09ff6d6ac53e6e4ea85bd8ff8b366ade980/macports-ports (master):

net/openssh: (hopefully) fix build failures on 10.6-. Round two.

sys/acl.h (implicitly pulled in by other headers) breaks with
APPLE_API_STRICT_CONFORMANCE set. Since we don't need it in the
keychain integration code, just drop it in that file.

xpc/xpc.h is only available on 10.7+. Since we don't need any
declarations or macros from there, don't include it on older platforms.

No revbump since this only fixes build failures.

Fixes: #59397

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: closedreopened

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: reopenedclosed

In dee8b9df7aaeed094b7de3f49b0309d6debbcd6b/macports-ports (master):

net/openssh: (hopefully) fix build failures on 10.6-. Round three.

Security/SecTask.h is part of libsecurity_codesigning (and might have
seen quite some moves around the tree during its lifetime, including a
move back into the Security framework in a sectask subdirectory), but
not available on 10.6-. We don't actually need it, so drop it on 10.6-.

No revbump since this only fixes build failures.

Fixes: #59397

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: closedreopened

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: reopenedclosed

In d6e05adab97e8e64939011a7885de3ecf4a55b69/macports-ports (master):

net/openssh: (hopefully) fix build failures on 10.6-. Round four.

Security/Security.h pulls Security/SecItem.h in on 10.7+, but not so on
older platforms, so import that header manually.

kSecAttrAccessGroup is private data up until 10.8, hence we'll need to
include it in our SecItemPriv.h shim.

kSecClassGenericPassword likewise is a private data up until 10.7, so
also add it to the shim.

No revbump since this only fixes build failures.

Fixes: #59397
Fixes: #59424

comment:27 Changed 5 years ago by grumpybozo (Bill Cole)

Resolution: fixed
Status: closedreopened

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: reopenedclosed

In 3c41e9203c65aa682b0d9f51efdddba222232776/macports-ports (master):

net/openssh: (hopefully) fix build failures on 10.7-. Round five.

The new Keychain integration code uses Object Subscripting, a newer
feature requiring explicit compiler and library support.

We can provide an implementation for the affected NS types directly, but
also need to make sure that a suitable compiler is chosen.

That means either Apple LLVM 4.0 (>= 421) or FOSS clang/llvm 3.1. Since
all older versions of clang/llvm were evicted from MacPorts a while ago,
any macports-clang version will do nowadays.

No revbump since this only fixes build failures.

Fixes: #59397

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 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.

Last edited 5 years ago by Ionic (Mihai Moldovan) (previous) (diff)

comment:32 in reply to:  31 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 in reply to:  31 Changed 5 years ago by grumpybozo (Bill Cole)

Replying to Ionic:

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.)

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@…>

In 10282bf4b5b49925d40eaa687604eed1235d58de/macports-ports (master):

net/openssh: fix build failures on 10.7-. Round six.

YES and NO are macros defined in /usr/include/objc/objc.h - but their
definitions differs based on OS version.

10.8+ define them as "((BOOL)X)", while earlier versions define them as
"(BOOL)X".

When later used in an expression such as "@X", the latter (older)
definition is causing compile errors and needs to be wrapped in an extra
pair of parentheses.

Newer versions are not negatively affected by the extra pair of
parentheses, so we can keep them around in shared code paths (but not
unnecessarily for code paths only enabled for newer OS versions).

This change should finally allow successful building on 10.7 and 10.6,
and potentially on even older platforms.

I cannot, however, guarantee that it actually works correctly.

Additionally, fix up copyright header of the Object Subscripting compat
implementation.

No revbump since this only fixes build failures.

Fixes: #59397

comment:36 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 in reply to:  36 ; 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 in reply to:  37 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 what open does with an x-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 to security with 10.6 or something like that.

Yes, 10.6+ seems right. This one suggests that to.

Note: See TracTickets for help on using tickets.