Skip to content

adding heloserver name to the options for email #214

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

Merged
merged 4 commits into from
Sep 29, 2014

Conversation

jrossi
Copy link
Member

@jrossi jrossi commented May 27, 2014

This allow the manual setting of the server name during HELO message. This is to solve #213

@jrossi
Copy link
Member Author

jrossi commented May 27, 2014

@Mycron Can you test this as I have can not. This pull request addes the option:

<ossec_config>
    <global>
        <helo_server>ossechost01.example.net</helo_server>
    </global>
</ossec_config>

This still needs to be tested and docs completed, but i will do that once things have been tested.

@Mycron
Copy link

Mycron commented May 28, 2014

Hi, jrossi.
I committed your code, but with testing i get follow output on e-mail server:
May 28 12:02:47 whale postfix/smtpd[32728]: NOQUEUE: reject: RCPT from : 504 5.5.2 <%s>: Helo command rejected: need fully-qualified hostname; from=ossecm@whale to=[email protected] proto=SMTP helo=<%s>

As you can see, code not substitute value in variable heloserver.

@Mycron
Copy link

Mycron commented May 28, 2014

My previous comment not working :( With form "Helo <%s>\r\n" i get follow log:
May 28 12:41:27 whale postfix/smtpd[4696]: NOQUEUE: reject: RCPT from ...: 504 5.5.2 : Helo command rejected: need fully-qualified hostname; from=ossecm@whale to=[email protected] proto=SMTP helo=

@ddpbsd
Copy link
Member

ddpbsd commented May 28, 2014

The snprintf keeps the literal %s. I haven't checked the rest of the code, but it seems like it should be simple enough to skip the define, and snprintf the "Helo " directly.

@Mycron
Copy link

Mycron commented May 29, 2014

AFAIK, this function use format as 'printf'.
http://www.cplusplus.com/reference/cstdio/snprintf/
Therefore i expect substitution of "%s" and moreover the rest of the code use it in "mail from", "rcpt to" and so on.

@jrossi
Copy link
Member Author

jrossi commented May 30, 2014

@Mycron What error are you getting. Please see this full pull request as it does the correct thing.

@jrossi jrossi added this to the ossec-hids-2.9 milestone May 30, 2014
@Mycron
Copy link

Mycron commented May 30, 2014

Hi.
I get an error in helo command, which sent by ossec to mail server:
May 28 12:02:47 whale postfix/smtpd[32728]: NOQUEUE: reject: RCPT from : 504 5.5.2 <%s>: Helo command rejected: need fully-qualified hostname; from=ossecm@whale to=[email protected] proto=SMTP helo=<%s>

@ddpbsd
Copy link
Member

ddpbsd commented May 30, 2014

@jrossi It's possible that my testing was faulty, but I wasn't able to get a test program (made a very simple test case) to work. The %s in the #define never gets replaced in the snprintf.

It's definitely not as pretty

@ddpbsd
Copy link
Member

ddpbsd commented Jul 11, 2014

@Mycron @jrossi Please take a look at my commit here: ddpbsd@ad1ece9

I think this fixes the issue (for non-custom email at least).
WARNING: My apparmor decoder/rules are included in that branch.

@ddpbsd
Copy link
Member

ddpbsd commented Jul 21, 2014

@jrossi Is this something you're still interested in? How do we proceed with this?

@jrossi
Copy link
Member Author

jrossi commented Jul 22, 2014

I need to make this hack work :) I will look again this week. And test I have vagrant setup to test mail notifications now so that should help.

@Mycron
Copy link

Mycron commented Jul 22, 2014

Hi, folks
I was compile commit from ddpbsd, but withot success :(
Src from message show that substitution didn't work:

Received: from %s (unknown [192.168.14.233]) <---- %s is still there

On Tue, Jul 22, 2014 at 4:16 PM, Jeremy Rossi [email protected]
wrote:

I need to make this hack work :) I will look again this week. And test I
have vagrant setup to test mail notifications now so that should help.


Reply to this email directly or view it on GitHub
#214 (comment).

@cgzones
Copy link
Contributor

cgzones commented Jul 22, 2014

@Mycron what compiler and system do you use?

@ddpbsd
Copy link
Member

ddpbsd commented Jul 22, 2014

I finally got around to setting up a testing environment, and I see it too.
Back to the drawing board. :)

On Tue, Jul 22, 2014 at 11:37 AM, Vasiliy Shpanskiy <
[email protected]> wrote:

Hi, folks
I was compile commit from ddpbsd, but withot success :(
Src from message show that substitution didn't work:

Received: from %s (unknown [192.168.14.233]) <---- %s is still there

On Tue, Jul 22, 2014 at 4:16 PM, Jeremy Rossi [email protected]
wrote:

I need to make this hack work :) I will look again this week. And test I
have vagrant setup to test mail notifications now so that should help.

Reply to this email directly or view it on GitHub
#214 (comment).

Reply to this email directly or view it on GitHub
#214 (comment).

@ddpbsd
Copy link
Member

ddpbsd commented Jul 22, 2014

I see the problem with my code, it was changing it for the stupid sms
format. I added the bits to the regular email section too. It's not quite
identifying itself properly yet, but it's closer.

On Tue, Jul 22, 2014 at 12:03 PM, dan (ddp) [email protected] wrote:

I finally got around to setting up a testing environment, and I see it
too. Back to the drawing board. :)

On Tue, Jul 22, 2014 at 11:37 AM, Vasiliy Shpanskiy <
[email protected]> wrote:

Hi, folks
I was compile commit from ddpbsd, but withot success :(
Src from message show that substitution didn't work:

Received: from %s (unknown [192.168.14.233]) <---- %s is still there

On Tue, Jul 22, 2014 at 4:16 PM, Jeremy Rossi [email protected]
wrote:

I need to make this hack work :) I will look again this week. And test
I
have vagrant setup to test mail notifications now so that should help.

Reply to this email directly or view it on GitHub
#214 (comment).

Reply to this email directly or view it on GitHub
#214 (comment).

@ddpbsd
Copy link
Member

ddpbsd commented Jul 22, 2014

Ok, I think I got it that time. The server reported as the right hostname
(according to tcpdump), so give it a shot!

On Tue, Jul 22, 2014 at 12:12 PM, dan (ddp) [email protected] wrote:

I see the problem with my code, it was changing it for the stupid sms
format. I added the bits to the regular email section too. It's not quite
identifying itself properly yet, but it's closer.

On Tue, Jul 22, 2014 at 12:03 PM, dan (ddp) [email protected] wrote:

I finally got around to setting up a testing environment, and I see it
too. Back to the drawing board. :)

On Tue, Jul 22, 2014 at 11:37 AM, Vasiliy Shpanskiy <
[email protected]> wrote:

Hi, folks
I was compile commit from ddpbsd, but withot success :(
Src from message show that substitution didn't work:

Received: from %s (unknown [192.168.14.233]) <---- %s is still there

On Tue, Jul 22, 2014 at 4:16 PM, Jeremy Rossi [email protected]

wrote:

I need to make this hack work :) I will look again this week. And test
I
have vagrant setup to test mail notifications now so that should help.

Reply to this email directly or view it on GitHub
#214 (comment).

Reply to this email directly or view it on GitHub
#214 (comment).

@jrossi jrossi mentioned this pull request Sep 14, 2014
@jrossi
Copy link
Member Author

jrossi commented Sep 22, 2014

@ossec I updated this pull request and I think it should work now. Can someone look see ?

@cgzones
Copy link
Contributor

cgzones commented Sep 22, 2014

I would like to merge it, but github prevents merging cause to some conflicts to #274 .
Can you rebase it?

@jrossi
Copy link
Member Author

jrossi commented Sep 22, 2014

Yeapers. Will later today. Thanks

@@ -167,6 +167,7 @@ int Read_Global(XML_NODE node, void *configp, void *mailp)
char *xml_emailfrom = "email_from";
char *xml_emailidsname = "email_idsname";
char *xml_smtpserver = "smtp_server";
char *xml_heloserver = "helo_server";
Copy link
Contributor

Choose a reason for hiding this comment

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

can you declare this as const char * ?

@jrossi
Copy link
Member Author

jrossi commented Sep 29, 2014

@ossec anyone able to take a look see?

@ddpbsd
Copy link
Member

ddpbsd commented Sep 29, 2014

Haven't had a chance to test it. Building now.

@jrossi
Copy link
Member Author

jrossi commented Sep 29, 2014

@ddpbsd thank you

ddpbsd added a commit that referenced this pull request Sep 29, 2014
adding heloserver name to the options for email

Seems to work for me.
@ddpbsd ddpbsd merged commit bf66517 into ossec:master Sep 29, 2014
@aquerubin
Copy link
Contributor

I was looking at this change and it occurs to me wouldn't it make more sense to use the hostname rather than hardcoding 'notify.ossec.net' in the helo banner when mail->heloserver is not explicitly configured?

@jrossi
Copy link
Member Author

jrossi commented Sep 30, 2014

That makes sense. I am off in the make file weeds and some other areas. Pull request?

@aquerubin
Copy link
Contributor

On Tue, 30 Sep 2014, Jeremy Rossi wrote:

That makes sense. I am off in the make file weeds and some other areas.
Pull request?

Sure but before I go working on some of that other stuff, I'd like to get
closure on the IPv6 pull request which is almost a year old and appears to
have fallen completely off the priority list.

Antonio Querubin
e-mail: [email protected]
xmpp: [email protected]

@jrossi jrossi deleted the fixed-ehlo-name-sendmail-213 branch October 20, 2014 01:02
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.

5 participants