Conversation
audit.c
Outdated
| #include <unistd.h> | ||
|
|
||
| #ifdef SSH_AUDIT_EVENTS | ||
| #if defined(SSH_AUDIT_EVENTS) || defined(USE_AIX_AUDIT) |
There was a problem hiding this comment.
conditionally define SSH_AUDIT_EVENTS in defines.h and don't sprinkle platform specific ifdefs everywhere. see USE_BSM_AUDIT and USE_LINUX_AUDIT in defines.h for examples.
audit.c
Outdated
| * audit_connection_from() is called and MAY NOT be initialized when | ||
| * audit_event(CONNECTION_ABANDON) is called. Test for NULL before using. | ||
| */ | ||
| #ifdef DEBUG_AUDIT_EVENTS |
There was a problem hiding this comment.
this part is incorrect. see the description of how it's supposed to work elsewhere
| return(event_lookup[i].name); | ||
| } | ||
|
|
||
| # ifndef CUSTOM_SSH_AUDIT_EVENTS |
There was a problem hiding this comment.
this is where the shims start. revert this part.
defines.h
Outdated
| /* | ||
| * Definitions for IP type of service (ip_tos) | ||
| */ | ||
| #include <netinet/in_systm.h> |
defines.h
Outdated
| # endif /* gcc version */ | ||
| #endif /* __predict_true */ | ||
|
|
||
| #if defined(HAVE_GLOB_H) && defined(GLOB_HAS_ALTDIRFUNC) && \ |
There was a problem hiding this comment.
what's this for? it looks like either an unrelated change or a bad merge.
audit.c
Outdated
| static const char unknownuser[] = "(unknown user)"; | ||
| static const char invaliduser[] = "(invalid user)"; | ||
|
|
||
| #ifdef DEBUG_AUDIT_EVENTS |
There was a problem hiding this comment.
why are you changing this?
configure.ac
Outdated
| AUDIT_MODULE=debug | ||
| AC_MSG_RESULT([debug]) | ||
| AC_DEFINE([SSH_AUDIT_EVENTS], [1], [Use audit debugging module]) | ||
| AC_DEFINE([DEBUG_AUDIT_EVENTS], [1], [Use audit debugging module]) |
There was a problem hiding this comment.
this part is also unnecessary. if SSH_AUDIT_EVENTS is enabled the hooks are enabled and so are the debug implementations. if SSH_AUDIT_EVENTS and CUSTOM_SSH_AUDIT_EVENTS is enabled your alternative implementations in audit-aix.c.
audit-aix.c
Outdated
|
|
||
| /* Maybe add the audit class to struct Authmethod? */ | ||
| ssh_audit_event_t | ||
| audit_classify_auth(const char *method) |
There was a problem hiding this comment.
this already exists in audit.c and does not need to be duplicated here
audit-aix.c
Outdated
|
|
||
| /* helper to return supplied username */ | ||
| const char * | ||
| audit_username(void) |
audit-aix.c
Outdated
| } | ||
| } | ||
|
|
||
| /* On other systems, use getpwnam or authctxt->pw */ |
There was a problem hiding this comment.
all of this code is inside AIX specific ifdefs so you do not need to handle other systems here,
|
@daztucker , I have reworked on the the review comments and have incorporated it as a part of the 2nd commit. Please let me know if anything more needs to be addressed. I also see that the CI checks w.r.t Ubuntu Kitchensink are failing , not sure why this is failing , any suggestions would be helpful. Thank you, |
1. Core Audit Implementation Changes
File: [audit.c]
Added AIX-Specific Audit Support:
AIX Headers: Added <sys/audit.h> and <usersec.h> for AIX audit subsystem
Enhanced [audit_username()]: Improved logic to handle NULL authctxt cases
AIX Event Names: Added AIX-compliant event names in [audit_event_lookup()]:
SSH_exceedmtrix, SSH_rootdned, SSH_authsuccess, etc.
Standard names for non-AIX: LOGIN_EXCEED_MAXTRIES, AUTH_SUCCESS, etc.
Enhanced [audit_event()] Function:
UID Tracking: Added auth_uid to track authenticating user's UID
AIX UID Retrieval: Uses getuserattr() on AIX, getpwnam() on other systems
Remote IP Handling: Safely handles NULL ssh pointer
Detailed Logging: Logs auth_uid, username, event type, and remote IP
AIX Audit Writing: Calls auditwrite() with proper result codes (0=success, 1=failure)
Error Handling: Proper buffer truncation checks and error logging
Enhanced [audit_session_open()] Function:
AIX-Specific Implementation: Complete audit trail for session opens
Detailed Context: Logs username, tty, hostname, PID, and UID
AIX Audit Integration: Writes to AIX audit subsystem with auditwrite()
Fallback for Non-AIX: Maintains simple debug logging for other platforms
2. Audit Header Changes
File: [audit.h]
Added New Event Types:
SSH_BAD_PCKT, // bad/invalid packet received
SSH_CIPHER_NO_MATCH, // cipher negotiation failed
SSH_SESSION_OPEN, // session opened
These events enable tracking
of security-relevant protocol events.
3. Client/Server Separation Solution
File: [audit-aix.c] (NEW FILE)
Purpose:
Provides AIX specific implementations of all audit functions for Server binaries.
Key Design:
Conditional Compilation: Uses USE_AIX_AUDIT
4. Testing:
Check if the audit records are getting captured post starting the audit system on AIX as different user and different use case: