Skip to content

[RFC] Use ports-based config#224

Merged
nl5887 merged 5 commits intohoneytrap:masterfrom
CapacitorSet:config-ports
May 1, 2018
Merged

[RFC] Use ports-based config#224
nl5887 merged 5 commits intohoneytrap:masterfrom
CapacitorSet:config-ports

Conversation

@CapacitorSet
Copy link
Contributor

Fixes #219: the configuration is based around ports and not around services, as discussed in chat. Also fixes #215.

Commit e86fd10 has a more accurate description:

This is a minor redesign of the Honeytrap model, where the focus of the configuration moves from services (a list of services each with their ports) to ports (a list of ports with their services, and a list of port-agnostic services).

As a consequence of this redesign, it is now possible to define a priority model for services on the same port. Simply put, services that come first take a higher priority (i.e. are tested first for compatibility with the connection). An example application for this is running TR-069 and HTTP on the same port: TR-069 is HTTP-based but more specific, so it should be tested first, with HTTP being a fallback.

@CapacitorSet CapacitorSet changed the title Use ports-based config [RFC] Use ports-based config Apr 11, 2018
@CapacitorSet
Copy link
Contributor Author

Actually, do not merge this - I'll make a new PR after the plugins feature is implemented. However, do feel free to review it in the meanwhile.

@CapacitorSet CapacitorSet force-pushed the config-ports branch 2 times, most recently from 855710e to 155d97f Compare April 27, 2018 16:36
This is a minor redesign of the Honeytrap model, where the focus of the configuration moves from services (a list of services each with their
ports) to ports (a list of ports with their services, and a list of port-agnostic services).

As a consequence of this redesign, it is now possible to define a priority model for services on the same port. Simply put, services that come
first take a higher priority (i.e. are tested first for compatibility with the connection). An example application for this is running TR-069
and HTTP on the same port: TR-069 is HTTP-based but more specific, so it should be tested first, with HTTP being a fallback.
@CapacitorSet
Copy link
Contributor Author

CapacitorSet commented Apr 27, 2018

Nevermind, I rebased it successfully onto master, and it seems that it doesn't conflict with the plugins branch so it should work correctly with the feature. This PR is ready for merging, as far as I'm concerned.

} else if err != nil {
log.Errorf(color.RedString("Could not peek bytes: %s", err.Error()))
return nil
return nil, nil
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't it return an error or return the original conn?

n = _n // avoid silly "variable not used" warning
if err == io.EOF {
return nil
return nil, nil
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't it return an error or return the original conn?

}
// No service can handle the connection. Let the caller deal with it.
return nil
return nil, nil
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't it return the original conn?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What would be the point in returning the original connection?

@nl5887
Copy link
Contributor

nl5887 commented Apr 28, 2018

Should we give warnings for ports that don't have services configured?

@CapacitorSet
Copy link
Contributor Author

I added an error return value for findService, and a warning for empty ports.

@nl5887 nl5887 merged commit a356ea0 into honeytrap:master May 1, 2018
@CapacitorSet CapacitorSet deleted the config-ports branch May 1, 2018 17:51
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.

Use Ports to define Services Peeking corrupts the request data

2 participants