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

First draft of passwd discovery agent. #29

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

First draft of passwd discovery agent. #29

wants to merge 8 commits into from

Conversation

LeamHall
Copy link

discovery_agents/passwd

This is my first contribution so feel free to critically review. :) Especially the spaces and tabs used for indentation. It was written with my old vimrc:

set smartindent
set tabstop=2
set shiftwidth=2
set expandtab
set paste
set number

@Alan-R
Copy link
Member

Alan-R commented Mar 20, 2016

Here are a few issues that came up when I looked at the script:

  • It doesn't do what you intended when run under /bin/sh (use printf rather than echo with flags). Strangely, echo is quite different from platform to platform, but printf is the same everywhere. This is especially important for the discovery agent since should be usable on any POSIX platform.
  • It doesn't produce JSON no matter what shell it is invoked under (use the jsonlint command to check the output - that's what the tests do).
  • As written, it doesn't support regression testing.
  • It does not include regression tests and correct test output.
  • If it is unable to read /etc/passwd, it will fail in a way that gives no indication of what the problem is.
  • The author information is incorrect.
  • You should include the vim comments from the boilerplate file to ensure correct formatting at least for vim users.

For more information about providing a discovery agent (particularly about testing) please see the article writing an assimilation discovery agent. It was written precisely to address these issues.

If you incorporate it with tests, and it passes tests, then it will be producing legal JSON, and it will give the proper failure return if it is unable to read the password file. The test infrastructure ensures both of those.

However, it doesn't ensure that the JSON you produced is sensible, meaningful or has the correct data. Humans have to ensure these things.

I would suggest these names for the JSON fields:
login, pw, uid, gid, comment, home, shell. If you want to use gecos for comment you can, but I would prefer comment, since few people would have any idea what gecos meant, or its historical significance. If you want to break the comment field down into csv fields, you could - but I would make it an array ["Alan Robertson", "", "", "", ""] since the fields have different meanings in different operating systems. I think a number of them use commas for the various fields. You have more recent experience in other OSes (AIX, Solaris, etc) than I do.

Here's my password entry from my desktop:
"alanr":"x":"1000":"1000":"Alan Robertson,,,":"/home/alanr":"/bin/bash",

This would translate into:
{"login": "alanr", "pw": "x", "uid", 1000, "gid", 1000, "comment": ["Alan Robertson", "", "", ""], "shell": "/bin/bash"}
Note that the uid and gid values are not quoted - they are numbers, not strings.

@LeamHall
Copy link
Author

Alan, thanks! Let me go work on that.

discovery_agents/passwd
testcode/discovery_input/passwd
Not formatting JSON right, and the discovery agent needs to test
and resolve empty fields that JSON doesn't like.

Per Alan also need to ensure alpha fields are in fact alpha.
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.

None yet

2 participants