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 support for dst/src addr/port pathspecs #74

Closed
wants to merge 1 commit into from

Conversation

AdamJacobMuller
Copy link
Contributor

add support for:

%f - dest address
%h - dest port
%t - source address
%v - source port

format specifiers to pathspec

The goal of this is to allow pathspecs like:

/var/log/sslsplit/%t/%f/%h/%T

This improves upon the existing %s/%d for two big reasons
#1 using %s/%d results in unusable directory structures, because of the inclusion of the randomized source port address
#2 the inclusion of the brackets inside the %s/%d specifiers is sub-optimal in filesystems

I would point out, I am not a C programmer by trade and barely know C, this patch does work for me but is perhaps sub-optimal. Comments/improvements are definitely welcome.

%f - dest address
%h - dest port
%t - source address
%v - source port

format specifiers to pathspec
@droe droe self-assigned this Feb 19, 2015
@droe droe added the feature label Feb 19, 2015
@droe droe added this to the 0.5.0 milestone Feb 19, 2015
@droe
Copy link
Owner

droe commented Feb 26, 2015

Your solution causes getnameinfo() to be called twice for every endpoint (once for the host and once for the port). Can you change that into a single call of getnameinfo() by writing a derivative of sys_sockaddr_str() that returns host and port into separate buffers but only calls getnameinfo() once, instead of having separate helpers for host and port?

@AdamJacobMuller
Copy link
Contributor Author

I can try but no promises :) I'm really not a c programmer.

Aside from that, does the patch look ok?

@droe
Copy link
Owner

droe commented Mar 2, 2015

Thanks! I try to thoroughly test patches before merging them so no worries :)
On first glance the patch looks ok, yes, but I did not test it yet.

@droe
Copy link
Owner

droe commented Mar 14, 2015

Are you still working on this?

@droe
Copy link
Owner

droe commented Mar 15, 2015

FYI, I'm now working on separating host and port into distinct members of the state ctx structure and rewriting sys_sockaddr_str() to facilitate that.

droe added a commit that referenced this pull request Mar 15, 2015
Store host and port in separate strings internally and get rid of the
[host]:port representation where separate host and port would be
cleaner.  This includes the following user-visible changes:

-   Generated filenames that contain host and port, such as by -S and
    -F %d and %s, now use a host,port format instead of [host]:port.

-   Connect log now uses separate fields for host and port.

Issue:		#69 #74
Reported by:	Adam Jacob Muller
droe added a commit that referenced this pull request Mar 15, 2015
Issue:		#74
Submitted by:	Adam Jacob Muller
@droe
Copy link
Owner

droe commented Mar 15, 2015

Merged to develop with some modifications. Thanks for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants