Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

providers/darwin - kern.procargs2 guard against runtime panic #172

Merged
merged 3 commits into from
May 18, 2023

Conversation

andrewkroh
Copy link
Member

@andrewkroh andrewkroh commented May 15, 2023

Prior to this change kern_procargs iterated over the data based
on the argc value without checking if the underlying slice held
enough args.

To prevent a runtime error this adds a check to verify there is more
data before trying to index another argument.

Add a fuzz test to check for panics in the parsing code for kern.procargs2.

Relates #173

@andrewkroh andrewkroh force-pushed the bugfix/darwin-procargs-panic branch from 4cb446c to 8ca8557 Compare May 15, 2023 20:36
@andrewkroh andrewkroh added the bug Something isn't working label May 15, 2023
@elasticmachine
Copy link
Collaborator

elasticmachine commented May 15, 2023

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2023-05-18T04:23:47.688+0000

  • Duration: 10 min 7 sec

Test stats 🧪

Test Results
Failed 0
Passed 346
Skipped 4
Total 350

Prior to this change kern_procargs iterated over the data based
on the argc value without checking if the underlying slice held
enough args.

To prevent a runtime error this adds a check to verify there is more
data before trying to index another argument.

Add a fuzz test to check for panics in the parsing code for kern.procargs2.
@andrewkroh andrewkroh force-pushed the bugfix/darwin-procargs-panic branch from 93207e8 to fa26e88 Compare May 15, 2023 20:55
@andrewkroh
Copy link
Member Author

/test

@andrewkroh andrewkroh enabled auto-merge (squash) May 15, 2023 21:42
@andrewkroh andrewkroh requested a review from a team May 17, 2023 12:46
One change in external behavior is that instead of returning io.EOF it now returns errInvalidProcargs2Data.
@andrewkroh andrewkroh requested a review from efd6 May 18, 2023 02:58
@efd6
Copy link
Contributor

efd6 commented May 18, 2023

Failure looks to be unrelated (the second TLS timeout I've seen today).

@andrewkroh
Copy link
Member Author

/test

@andrewkroh andrewkroh disabled auto-merge May 18, 2023 03:14
@efd6
Copy link
Contributor

efd6 commented May 18, 2023

/test

@andrewkroh andrewkroh merged commit c0d4d10 into elastic:main May 18, 2023
jaysoffian pushed a commit to jaysoffian/pam-ysshca that referenced this pull request Aug 31, 2023
Use raw syscalls to retrieve the command line under Darwin, since macOS
does not provide a `/proc` filesystem.

The code to do this is from https://github.com/elastic/go-sysinfo which
can be sanity checked against:

https://github.com/apple-oss-distributions/adv_cmds/blob/adv_cmds-205/ps/print.c#L115

I've verified with these changes that `pam_sshca.so` works as expected
under macOS 13 (Ventura) on an arm64 host.

Issues:

1. The Linux `pam.d/sudo` configuration line:

       "auth   [success=done default=die]   pam_sshca.so"

   Does not work on Darwin. Instead use one of the following:

       "auth   requisite   /path/to/pam_sshca.so"

   Or:

       "auth   required   /path/to/pam_sshca.so"

    Neither is identical to "[success=done default=die]" whose
    semantics are impossible under Darwin. See the `pam.conf` man pages
    on Linux and macOS for details.

2. The "log/syslog" module does not work under macOS >= 12 (Monterey).
   Log messages are silently dropped:

   golang/go#59229

3. The `kern.procargs2` syscall returns incorrect data under macOS
   10.15 (Catalina) due to a bug in that OS version. The code won't
   panic under that OS version but it won't return a command line:

   - elastic/go-sysinfo#172
   - elastic/go-sysinfo#173
jaysoffian pushed a commit to jaysoffian/pam-ysshca that referenced this pull request Aug 31, 2023
Use raw syscalls to retrieve the command line under Darwin, since macOS
does not provide a `/proc` filesystem.

The code to do this is from https://github.com/elastic/go-sysinfo which
can be sanity checked against:

https://github.com/apple-oss-distributions/adv_cmds/blob/adv_cmds-205/ps/print.c#L115

I've verified with these changes that `pam_sshca.so` works as expected
under macOS 13 (Ventura) on an arm64 host.

Issues:

1. The Linux `pam.d/sudo` configuration line:

       "auth   [success=done default=die]   pam_sshca.so"

   Does not work on Darwin. Instead use one of the following:

       "auth   requisite   /path/to/pam_sshca.so"

   Or:

       "auth   required   /path/to/pam_sshca.so"

    Neither is identical to `[success=done default=die]` whose
    semantics are impossible under Darwin. See the `pam.conf` man pages
    on Linux and macOS for details.

2. The `log/syslog` module does not work under macOS >= 12 (Monterey).
   Log messages are silently dropped:

   golang/go#59229

3. The `kern.procargs2` syscall returns incorrect data under macOS
   10.15 (Catalina) due to a bug in that OS version. The code won't
   panic under that OS version but it won't return a command line:

   elastic/go-sysinfo#172
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants