ClientOriginIPAddress is 0.0.0.0 or ""

[Ed Tanous]

On Fri, Apr 16, 2021 at 6:34 AM Ivan Mikhaylov <i.mikhaylov at yadro.com> wrote:
>
> On Fri, 2021-04-09 at 07:58 -0700, Ed Tanous wrote:
> > On Thu, Apr 8, 2021 at 10:51 PM Sunitha Harish
> > <sunithaharish04 at gmail.com> wrote:
> > >
> > > On 06-04-2021 22:21, Ed Tanous wrote:
> > > > On Tue, Mar 23, 2021 at 3:28 PM Jun-Lin Chen (陳俊霖)
> > > > <Jun-Lin.Chen at quantatw.com> wrote:
> > > > > Hi,
> > > > >
> > > > > If continously send POST sessions Redfish request via Postman. I observe
> > > > > bmcweb debug logs, it will established connection at first request then
> > > > > not fetch user ip after the second request.
> > > > >
> > > > >
> > > > >
> > > > > That's why ClientOriginIPAddress in session after the second created is
> > > > > always 0.0.0.0 (not be assigned).
> > > > >
> > > > >
> > > > >
> > > > > In HTTP 1.1 It uses keep-alive Header default. so BMC does not fetch
> > > > > user IP when its connection is established.
> > > > >
> > > > >
> > > > >
> > > > > Is this behavior we expect?
> > > > >
> > > > >
> > > > >
> > > > > Best regards,
> > > > >
> > > > > Jun-Lin Chen
> > > >
> > > > Sunitha,
> > > > Considering this is code you wrote, would you mind replying to Jun-Lin
> > > > about his experience and what the expected behavior is?  For what it's
> > > > worth, I've seen it report 0.0.0.0 before on my system, I just haven't
> > > > had a chance to chase it down and debug it.
> > > >
> > > > -Ed
> > >
> > > Hi Jun-Lin,
> > >
> > > The ClientOriginIPAddress is expected to get populated when the session
> > > is established to the BMC ( the first request). This is a one time
> > > fetch, and it will be persisted for that session.
> >
> > Can you think of a reason it would report all zeros?  Can you verify
> > this functionality works properly on your system?
> > Part of me wonders if this has something to do with http keepalive.
> >
> > > Regards,
> > > Sunitha
> > > > >
> > > > > From: Jun-Lin Chen (陳俊霖)
> > > > > Sent: Friday, March 5, 2021 4:05 PM
> > > > > To: sunharis at in.ibm.com
> > > > > Cc: openbmc at lists.ozlabs.org
> > > > > Subject: ClientOriginIPAddress is 0.0.0.0 or ""
> > > > >
> > > > >
> > > > >
> > > > > Hi Sunitha,
> > > > >
> > > > >
> > > > >
> > > > > I found some problem when I create session.
> > > > >
> > > > > The property “ClientOriginIPAddress” is “” after POST
> > > > > https://${bmc}/login -d '{"username": <>,"password": <>}
> > > > >
> > > > > And it is 0.0.0.0 after POST
> > > > > https://${bmc}/redfish/v1/SessionService/Sessions -d '{"username":
> > > > > <>,"password": <>}'
> > > > >
> > > > > Both of them looked like something wrong. And I also look for source
> > > > > code in bmcweb repository.
> > > > >
> > > > > There only defined the default value: “” in generateUserSession() and
> > > > > not found fetch user ip in elsewhere.
> > > > >
> > > > > I think it is why ClientOriginIPAddress is strange.
> > > > >
> > > > >
> > > > >
> > > > > May I ask what step or something I lack in creation session so this
> > > > > property is abnormal?
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > > > Best regards,
> > > > >
> > > > > Jun-Lin Chen
> > > > >
> > > > >
>
> Hello Ed, I made some research at this point, ipAddress always sets to 0.0.0.0
> after first Request, it is done by req.emplace(parser->get()) in doWrite
> function in http/http_connection.h. fetchClientIp sets ipAddress only on
> connection start. It is possible to fix it with additional fetchClientIp inside
> handle function which seems overhead or
>
> ipAddress = req->ipAddress;
> req.emplace(parser->get());
> req->ipAddress = ipAddress;

Yep, this makes sense.  I would expect parsing the ip address to be
almost zero overhead (it should just be a memcpy), so it should be ok
to do on every request.  This is what I think we should do.

Sunitha,
Can you put together a patchset to fix the above and test it works as
expected on your system?

>
> I don't like both variants but possible that ipAddress shouldn't be a part of
> Request struct.

It's part of the request struct so we can separate any handler from
having to know anything about the connection semantics.  This makes it
so we at least have a hope of unit testing things in the future.

>
> Any ideas?
>