Bug 1420 - BSM support on Mac OS X
Summary: BSM support on Mac OS X
Status: CLOSED FIXED
Alias: None
Product: Portable OpenSSH
Classification: Unclassified
Component: Miscellaneous (show other bugs)
Version: 4.7p1
Hardware: Other Mac OS X
: P2 enhancement
Assignee: Assigned to nobody
URL:
Keywords:
Depends on:
Blocks: V_4_8
  Show dependency treegraph
 
Reported: 2007-12-21 15:47 AEDT by Disco Vince Giffin
Modified: 2008-03-31 15:23 AEDT (History)
1 user (show)

See Also:


Attachments
Adds BSM support. (2.74 KB, patch)
2007-12-21 15:47 AEDT, Disco Vince Giffin
no flags Details | Diff
Implement aug_get_machine for BSM audit support (3.63 KB, patch)
2007-12-24 06:40 AEDT, Darren Tucker
no flags Details | Diff
Add aug_get_machine, make AU_IPv6 optional (3.67 KB, patch)
2008-01-03 21:47 AEDT, Darren Tucker
djm: ok+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Disco Vince Giffin 2007-12-21 15:47:49 AEDT
Created attachment 1417 [details]
Adds BSM support.

Attached is a patch for building OpenSSH 4.7p1 on Mac OS X.

This patch adds part of the BSM support for Mac OS X.
Comment 1 Darren Tucker 2007-12-22 01:23:43 AEDT
Comment on attachment 1417 [details]
Adds BSM support.


>+#if defined(__APPLE__)
>+	/* don't have a aug_get_machine */
>+	tid->at_addr[0] = inet_addr(host);
>+	tid->at_type = 0;
>+	snprintf(buf, sizeof(buf), "%08x", tid->at_addr[0]);
>+	debug3("BSM audit: machine ID %s", buf);

A cleaner way to do this is implement aug_get_machine as a static function in this file, and activate it based on the presence (or not) of aug_get_machine.  I believe FreeBSD would also benefit from this.

I will post an alternative patch.

>-		AC_CHECK_FUNCS(getaudit_addr)
>+		AC_CHECK_FUNCS(getaudit_addr,
>+				[
>+				case "$host" in
>+				*-*-darwin7* | *-*-darwin8*)
>+					AC_MSG_CHECKING(for getaudit_addr usability)
>+					AC_MSG_RESULT(no)
>+					;;
>+					*) AC_DEFINE(HAVE_GETAUDIT_ADDR,

Could you please explain why this is needed?  darwin7 and darwin8 have a getaudit_addr but it doesn't work?
Comment 2 Darren Tucker 2007-12-22 02:59:58 AEDT
Comment on attachment 1417 [details]
Adds BSM support.

>+#if defined(__APPLE__)
>+	/* don't have a aug_get_machine */
>+	tid->at_addr[0] = inet_addr(host);
>+	tid->at_type = 0;
>+	snprintf(buf, sizeof(buf), "%08x", tid->at_addr[0]);
>+	debug3("BSM audit: machine ID %s", buf);
>+#else

How does this cope with connections from IPv6 addresses?
Comment 3 Darren Tucker 2007-12-24 06:40:29 AEDT
Created attachment 1422 [details]
Implement aug_get_machine for BSM audit support

This should work on OS X (modulo the query I have about handling IPv6 connections) and FreeBSD/OpenBSM (untested).
Comment 4 Disco Vince Giffin 2008-01-03 08:28:22 AEDT
(In reply to comment #2)
> (From update of attachment 1417 [details])
> >+#if defined(__APPLE__)
> >+	/* don't have a aug_get_machine */
> >+	tid->at_addr[0] = inet_addr(host);
> >+	tid->at_type = 0;
> >+	snprintf(buf, sizeof(buf), "%08x", tid->at_addr[0]);
> >+	debug3("BSM audit: machine ID %s", buf);
> >+#else
> 
> How does this cope with connections from IPv6 addresses?

This, unfortunately, does not cope with IPv6 addresses.  I have filed a bug in our system to have this addressed.
Comment 5 Darren Tucker 2008-01-03 08:40:55 AEDT
(In reply to comment #4)
> This, unfortunately, does not cope with IPv6 addresses.  I have filed a
> bug in our system to have this addressed.

The code I posted (attachment #1422 [details]) does cope with IPv6 addresses, but since the original patch doesn't I'm not sure that it will write a record of the correct format.

On Solaris, AU_IPv6 = 16 and AU_IPv4 = 4.  (There's also  comment in the header about them being the sizes of the addresses which might cause trouble down the track if code assumes that's always the case and another address types happen to be 16 bytes...)

From patch 1417, it appears that Apple have made record type 0 to be IPv4, the question is what's IPv6?  My patch assumes 1.
Comment 6 Disco Vince Giffin 2008-01-03 11:32:43 AEDT
(In reply to comment #5)
> (In reply to comment #4)
> > This, unfortunately, does not cope with IPv6 addresses.  I have filed a
> > bug in our system to have this addressed.
> 
> The code I posted (attachment #1422 [details]) does cope with IPv6 addresses, but
> since the original patch doesn't I'm not sure that it will write a
> record of the correct format.
> 
> On Solaris, AU_IPv6 = 16 and AU_IPv4 = 4.  (There's also  comment in
> the header about them being the sizes of the addresses which might
> cause trouble down the track if code assumes that's always the case and
> another address types happen to be 16 bytes...)
> 
> From patch 1417, it appears that Apple have made record type 0 to be
> IPv4, the question is what's IPv6?  My patch assumes 1.

When this patch was created, we only supported IPv4 so the type was just set to 0 (and not used).  I expect that we will adopt the types that are used in OpenBSM (AU_IPv6 = 16 and AU_IPv4 = 4) when IPv6 support is added.
Comment 7 Darren Tucker 2008-01-03 21:47:51 AEDT
Created attachment 1433 [details]
Add aug_get_machine, make AU_IPv6 optional

In that case I think this is the way to do it.  The patch should have equivalent behaviour to your original patch on current platforms, and should also work when AU_IPv6 is added.  Could you please confirm?

Also: if there are any users of OpenBSM and/or FreeBSD out there that could also test that would be appreciated.

Thanks.
Comment 8 Disco Vince Giffin 2008-01-04 06:41:47 AEDT
(In reply to comment #7)
> In that case I think this is the way to do it.  The patch should have
> equivalent behaviour to your original patch on current platforms, and
> should also work when AU_IPv6 is added.  Could you please confirm?

This looks good.

Thank you, sir.
Comment 9 Darren Tucker 2008-01-08 15:21:27 AEDT
(In reply to comment #8)
> This looks good.

By "looks good" do you mean that it behaves correctly when tested?  If it has been tested then I'm ok with putting it in 4.8.

> Thank you, sir.

You're welcome.
Comment 10 Damien Miller 2008-01-20 06:58:51 AEDT
Comment on attachment 1433 [details]
Add aug_get_machine, make AU_IPv6 optional

ok
Comment 11 Disco Vince Giffin 2008-01-22 08:13:10 AEDT
(In reply to comment #9)
> (In reply to comment #8)
> > This looks good.
> 
> By "looks good" do you mean that it behaves correctly when tested?  If
> it has been tested then I'm ok with putting it in 4.8.

Yes.  Our normal build process doesn't run autoconf, so I did have to run that manually and augment the patch (with hunks for configure and config.h.in).  After that it worked as expected.
Comment 12 Darren Tucker 2008-02-25 21:06:33 AEDT
patch #1433 has been applied (with a couple of extra headers spotted by csjp at FreeBSD org) and will be in 4.8.  Thanks.
Comment 13 Damien Miller 2008-03-31 15:23:22 AEDT
Fix shipped in 4.9/4.9p1 release.