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

Add MongoDB scanner. #156

Merged
merged 11 commits into from
Aug 22, 2018
Merged

Add MongoDB scanner. #156

merged 11 commits into from
Aug 22, 2018

Conversation

parkanzky
Copy link
Contributor

Added mongodb support.

How to Test

Pass IP of mongodb host to zgrab.

Notes & Caveats

Still needs tests

Issue Tracking

#16

@parkanzky parkanzky added the WIP Work In Progress label Aug 20, 2018

// getCommandMsg returns a mongodb message containing the specified BSON-encoded command.
// metdata and commandArgs expected to be BSON byte arrays.
func getCommandMsg(database string, commandName string, metadata []byte, commandArgs []byte) ([]byte) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can make you life a lot easier by using a bytes.Buffer with a preallocated buffer here.

metaData, err := bson.Marshal(bson.M{ "buildInfo": 1 })
if err != nil {
// programmer error
panic("Invalid BSON")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure this shouldn't return an error?

panic("Invalid BSON")
}
commandArgs, err := bson.Marshal(bson.M{})
if err != nil {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above. A good pattern is log error at source, and return up.


// getBuildInfoMsg returns a mongodb message containing a command to retrieve MongoDB build info.
func getBuildInfoMsg() ([]byte) {
metaData, err := bson.Marshal(bson.M{ "buildInfo": 1 })

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is static data, do once at process start time in the init() function and save for later use.

return zgrab2.TryGetScanStatus(err), nil, err
}
if len(binfo) < MSGHEADER_LEN + 4 {
err = fmt.Errorf("Server truncated message - no metadata doc (%d bytes: %s)", len(binfo), hex.EncodeToString(binfo))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

%x will do the hex encoding for you

)

const (
OP_REPLY = 1

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

go prefers camel case for all things, including constants. Not a huge deal, but go vet will yell at you if you run it.

if err != nil {
return nil, err
}
msglen := int(binary.LittleEndian.Uint32(msglen_buf))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this ever need to be an int?


// ReadMsg reads a full MongoDB message from the connection.
func (conn *Connection) ReadMsg() ([]byte, error) {
msglen_buf := make([]byte, 4)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want this to be fast, use an array, and pass it is a slice view.

var msglen_buf [4]byte
io.ReadFull(conn.conn, msglen_buf[:])

This will save you a heap allocation.

Paul A. Parkanzky added 4 commits August 20, 2018 16:16
* Generate static messages in Scanner Init()
* s/panic/log.Fatal/
* Remove unnecessary casting
* Use stack var and pass slice to avoid unnecessary alloc
@parkanzky parkanzky removed the WIP Work In Progress label Aug 21, 2018
section_payload, err := bson.Marshal(bson.M{ "buildinfo": 1, "$db": "admin" })
if err != nil {
// programmer error
log.Fatal("Invalid BSON")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice if the specific error got logged too -- e.g. log.Fatalf("Invalid BSON: %v", err) or ...%s", err.String())

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call

Cxx string `bson:"cxx,omitempty"`
CxxFlags string `bson:"cxxflags,omitempty"`
LinkFlags string `bson:"linkflags,omitempty"`
TargetAarch string `bson:"target_arch,omitempty"`
Copy link
Contributor

@justinbastress justinbastress Aug 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo? Also, this may be from the spec itself, but do they really treat "distarch" and "target_arch" differently?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol this is a typo and I actually wrote the integration test to accommodate it. will fix

scan.conn.Write(query)
msg, err := scan.conn.ReadMsg()
if err != nil {
return zgrab2.TryGetScanStatus(err), nil, err
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the data returned from the first packet / the fact that getMaxWireVersion() didn't throw sufficient to say that this is a MongoDB server?

If so, returning the error is still correct, but instead of nil, it should return a partial response object.

If OTOH random data could get this far, then returning a nil result is the right choice.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, if getMaxWireVersion() succeeds, then the server is speaking valid mongodb protocol. Should return partial response.

return err
}
if n != len(data) {
return &zgrab2.ScanError{Status: zgrab2.SCAN_CONNECTION_CLOSED, Err: nil}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a built-in error for short writes -- I think it's io.ErrShortWrite?

In the long run, this probably should be built into the common socket wrapper code.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ack

}

// Result holds the data returned by the scan
type Result struct {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We typically use snake_case for our JSON output -- so you would need additional json tags, e.g. json:"git_version" or json:"build_environment,omitempty" (the omitempty used to prevent JSON fields with their default go values from being written; usually we use omitempty unless there is a specific reason not to -- like for int/bool values where 0/false are meaningful)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ack

Paul A. Parkanzky added 2 commits August 21, 2018 16:01
* Add error message to invalid BSON log msg
* Use snake case for json output
* Update affect integration tests
Copy link
Contributor

@justinbastress justinbastress left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

Paul A. Parkanzky added 2 commits August 21, 2018 17:08
* Separate out isMaster and buildInfo commands
* Return results of both in separate sub-structs
* Include isMaster results regardless of whether buildInfo succeeds
Copy link
Contributor

@justinbastress justinbastress left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

@parkanzky parkanzky merged commit 264b811 into master Aug 22, 2018
@parkanzky parkanzky deleted the paul/mongodb branch August 22, 2018 17:54
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.

3 participants