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

Make shadow more robust in hostile environments #4

Merged
merged 5 commits into from
Nov 13, 2015

Conversation

xnox
Copy link
Contributor

@xnox xnox commented Mar 20, 2015

I'm patching shadow to be more robust when operating in a-typical environments, but these improvements are general enough, that I believe warrant inclusion upstream by default.

Specifically there are deployments that use nss-altfiles / nss-extrausers and thus ship alternative group/passwd/shadow/gshadow files elsewhere on the filesystem (e.g. /var/lib/passwd). In such configurations admin modifiable files /etc/passwd, /etc/group and so on may not exist. Furthermore if one is bootstrapping a new distribution from scratch, it would be nice to point shadow utilities at an empty /etc and start creating default system accounts with useradd/usermod/groupadd/etc utilities without writing initial files by hand. Hence these changes:

  • create all databases, with correct (?! used typical permissions here, please correct if different defaults are desired) permissions, even if they are missing / are empty.
  • next /etc/shadow file existence is used as a flag file, whether or not shadow passwords should be used. I think this is very odd, hence I add FORCE_SHADOW option to make sure shadow/gshadow are used, even if those files are non-existent.
  • during testing I have noticed that when shadow is compiled with PAM support, settings that are not-applicable any more, but present in the stock /etc/login.defs are being complained about. So I made a change for shadow to not complain about well-known settings, which are not in effect when compiled with PAM support. If this is undesirable, maybe instead we would want to pre-process login.defs at compile time to make sure they do not contain any unknown settings in a given configuration (with/without pam, with/without subuids, etc.)
  • Lastly, I noticed that login command would bail out and exit, if /etc/login.defs file is not present on disk. I find that very odd - given that there are compiled in defaults for every single value for all shadow utils to operate normally. Thus I made it non fatal for login to operate without /etc/login.defs.

Overall my goal is to have fully usable system with empty /etc and with these initial patches this is achievable. At the moment I'm also working on adding full usermod support, when operating with nss-altfiles.

Please review and consider including these patches.

Dimitri John Ledkov added 5 commits February 27, 2015 17:01
For most operations tools have compiled-in defaults, and thus can
operate without login.defs present.
When compiled with PAM certain settings are not used, however they are
still defined in the stock login.defs file. Thus every command reports
them as "unknown setting contact administrator".

Alternative would be to parse stock login.defs and comment out/remove
settings that are not applied, when compiled with PAM.
passwd, shadow, group, gshadow etc. can be managed via nss -
e.g. system default accounts can be specified using nss_altfiles,
rather than in /etc/. Thus despite having default accounts, these
files can be missing on disk and thus should be opened with O_CREATE
whenever they are attempted to be opened in O_RDWR modes.
@hallyn
Copy link
Member

hallyn commented Aug 10, 2015

Sorry, I lost track of this, I thought you were discussing it with someone else. Should we merge this at this point?

hallyn added a commit that referenced this pull request Nov 13, 2015
Make shadow more robust in hostile environments
@hallyn hallyn merged commit e01bad7 into shadow-maint:master Nov 13, 2015
cgzones added a commit to cgzones/shadow that referenced this pull request Apr 1, 2023
Free the actual struct of the removed entry.

Example userdel report:

    Direct leak of 40 byte(s) in 1 object(s) allocated from:
        #0 0x55b230efe857 in reallocarray (./src/userdel+0xda857)
        shadow-maint#1 0x55b230f6041f in mallocarray ./lib/./alloc.h:97:9
        shadow-maint#2 0x55b230f6041f in commonio_open ./lib/commonio.c:563:7
        shadow-maint#3 0x55b230f39098 in open_files ./src/userdel.c:555:6
        shadow-maint#4 0x55b230f39098 in main ./src/userdel.c:1189:2
        shadow-maint#5 0x7f9b48c64189 in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
hallyn pushed a commit that referenced this pull request Apr 26, 2023
Free the actual struct of the removed entry.

Example userdel report:

    Direct leak of 40 byte(s) in 1 object(s) allocated from:
        #0 0x55b230efe857 in reallocarray (./src/userdel+0xda857)
        #1 0x55b230f6041f in mallocarray ./lib/./alloc.h:97:9
        #2 0x55b230f6041f in commonio_open ./lib/commonio.c:563:7
        #3 0x55b230f39098 in open_files ./src/userdel.c:555:6
        #4 0x55b230f39098 in main ./src/userdel.c:1189:2
        #5 0x7f9b48c64189 in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants