pam: also set teleport-specific env vars via pam_putenv #3725
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Using `pam_putenv` from `libpam` exposes these env vars to `pam_exec.so` and possibly other built-in PAM modules. Keep setting them via `os.Setenv` too, for `pam_script.so` to use. Updates #3692
awly
requested review from
fspmarshall,
klizhentas,
russjones and
webvictim
as code owners
russjones
reviewed
May 16, 2020
|
|
||
| // Also set it via PAM-specific pam_putenv, which is respected by | ||
| // pam_exec (and possibly others), where parent env vars are not. | ||
| kv := C.CString(fmt.Sprintf("%s=%s", k, v)) |
There was a problem hiding this comment.
C.CString does a malloc under the hood. Will PAM free this memory? If not you'll need to call C.free yourself.
There was a problem hiding this comment.
Ah, nice catch, added C.free.
First time using cgo, appreciate all the advice!
| // Also set it via PAM-specific pam_putenv, which is respected by | ||
| // pam_exec (and possibly others), where parent env vars are not. | ||
| kv := C.CString(fmt.Sprintf("%s=%s", k, v)) | ||
| retval := C._pam_putenv(pamHandle, p.pamh, kv) |
There was a problem hiding this comment.
Put pam_putenv have any maximum size? If so you might want to cap the length here.
There was a problem hiding this comment.
http://www.linux-pam.org/Linux-PAM-html/adg-interface-by-app-expected.html#adg-pam_putenv doesn't mention any limits and their source code seems to allocate as much as needed.
fspmarshall
approved these changes
May 19, 2020
russjones
approved these changes
May 19, 2020
benarent
added a commit
that referenced
this pull request
Jun 18, 2020
* Base fork for 4.3 docs * [docs] external email identities and Kube Users (#3628) * Base fork for 4.3 docs * [docs] external email identities and Kube Users (#3628) * Remove trailing whitespace from docs files Some editors will do this automatically on save. This causes a lot of diffs when editing the docs in such an editor. Clean them up once now and we'll try to keep it tidy going forward. * Add make rules for docs whitespace and milv docs-test-whitespace: checks for trailing whitespace in all .md files under docs/. docs-fix-whitespace: removes trailing whitespace in all .md files under docs/. docs-test-links: runs milv in all docs/ subdirectories that have milv.config.yaml. docs-test: runs whitespace and links tests, used during `make docs` * Document the new `--use-local-ssh-agent` flag for tsh The flag is used to bypass the local SSH agent even when it's running. Specifically, this helps with agents that don't support certs. The flag was added in #3721 * Remove pam_script.so docs from SSH PAM page With #3725 we now populate teleport-specific env vars in a way that's accessible to `pam_exec.so`. There's no longer any reason to install pam_script.so separately and duplicate our docs. Updates #3692 * Using the correct --insecure-no-tls flag * Run docs-fix-whitespace make rule in a busybox container * Fixes #3414 Co-authored-by: Andrew Lytvynov <andrew@gravitational.com> Co-authored-by: Gus Luxton <gus@gravitational.com> Co-authored-by: Steven Martin <steven@gravitational.com> Co-authored-by: Gus Luxton <webvictim@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Using
pam_putenvfromlibpamexposes these env vars topam_exec.soand possibly other built-in PAM modules. Keep setting them via
os.Setenvtoo, forpam_script.soto use.Updates #3692