Orion should return error and refuse to start when port is negative

[profijoeln]

Hello,

Testing new Orion version 3.0.0, in Kubernetes environment, the container comes up but does not show service at port 1026.
Only after specifying -port 1026 argument, the component was reachable on this port, even from inside the container.

Tested with
https://hub.docker.com/layers/fiware/orion/3.0.0/images/sha256-8c82ef33e15f97fa5c5947adebe7f727b6003a91f81e3f09d074c85ffb3ac1c6?context=explore
and
https://hub.docker.com/layers/fiware/orion/latest/images/sha256-090f0b274d1100809c155a96c1013b5b1665fe4131dfe69101f876e6d0e4e1cd?context=explore

Official documentation refers it should default to 1026:
https://github.com/telefonicaid/fiware-orion/blob/3.0.0/doc/manuals/admin/cli.md

Logs when starting component without -port argument or ENV:

kubectl logs orion-three-6f84658555-c2s6p
time=2021-06-08T10:13:38.831Z | lvl=INFO | corr=N/A | trans=N/A | from=N/A | srv=N/A | subsrv=N/A | comp=Orion | op=contextBroker.cpp[1059]:main | msg=start command line </usr/bin/contextBroker -fg -multiservice -ngsiv1Autocast -disableFileLog -dbhost data-mongo-three -corsOrigin __ALL -logLevel DEBUG -reqMutexPolicy none -reqPoolSize 4>
time=2021-06-08T10:13:38.832Z | lvl=INFO | corr=N/A | trans=N/A | from=N/A | srv=N/A | subsrv=N/A | comp=Orion | op=contextBroker.cpp[933]:logEnvVars | msg=env var ORION_PORT (-port): -1
time=2021-06-08T10:13:38.833Z | lvl=INFO | corr=N/A | trans=N/A | from=N/A | srv=N/A | subsrv=N/A | comp=Orion | op=contextBroker.cpp[1133]:main | msg=Orion Context Broker is running
time=2021-06-08T10:13:38.931Z | lvl=INFO | corr=N/A | trans=N/A | from=N/A | srv=N/A | subsrv=N/A | comp=Orion | op=mongoConnectionPool.cpp[506]:mongoConnectionPoolInit | msg=Connected to mongodb://data-mongo-three/?connectTimeoutMS=10000 (dbName: orion, poolsize: 10)
time=2021-06-08T10:13:38.938Z | lvl=INFO | corr=N/A | trans=N/A | from=N/A | srv=N/A | subsrv=N/A | comp=Orion | op=contextBroker.cpp[1259]:main | msg=Startup completed

[fgalan]

Looking to:

time=2021-06-08T10:13:38.832Z | lvl=INFO | corr=N/A | trans=N/A | from=N/A | srv=N/A | subsrv=N/A | comp=Orion | op=contextBroker.cpp[933]:logEnvVars | msg=env var ORION_PORT (-port): -1

it seems that somebody (the kubernetes framework that start Orion?) is injecting a wrong variable value.

Although probably Orion should react to this giving an error and refusing to start.

[profijoeln]

Good point.

Kubernetes indeed creates such environment variables according to service-resource.

And one of those ENV will be called servicename_PORT . I had a Kubernetes service named orion in the environment, which coincidently creates ENV variable ORION_PORT.

Note; this Kubernetes created ENV is not only the port number, but whole tcp-address with port.

Solutions include;

1. Not creating Kubernetes service named orion
2. Using ENV in deployment to specify ORION_PORT (this overrides one created by Kubernetes service)
3. Using -port argument for Orion

Thank you for your time.

[fgalan]

In Orion there is room to a little improvement, so I have re-scoped the issue to:

Although probably Orion should react to this giving an error and refusing to start.

[kzangeli]

Well ...
Are we talking about a user setting the env var to -1 ?

export ORION_PORT=-1

Cause ... if the env var isn't set, then Orion will not use it.
Orion will instead look at the CLI param -port and if that is also not set, fall back to its default value, which is 1026.

Now, if this is malfunctioning (getenv returns -1 on error, such as env var not found), it would be a bug.

But, if a user has deliberately set the env var to -1 ... well, then the broker should fail - perhaps with a clear error message.
Not sure how bind reacts to a -1 port ...
Actually yes, the port is an unsigned short, to -1 is understood as to 65535 (0xFFFF).
Ans this is EXACTLY what the user asked for, definitely not a bug.

Theoretically ... would need to run a test to be 100% sure.

[fgalan]

The user doesn't know anything about ours internals codifications ;). If he sets ORION_PORT to -1 and get Orion running in port 65535 that's weird. We should end with an error in that case.

Maybe a "layer" checking env vars values before injecting them to actual settings internally could be helpful.

Very minor issue, anyway.

[kzangeli]

Sure, we don't want to be nasty :)
The parsing library has limit checks ... perhaps only for the CLI params.
The values should be checked after getting the value from env var as well.
[ Just went into the source code and found out that the limits aren't set for '-port':

src/app/contextBroker/contextBroker.cpp:
 { "-port",  &port, "PORT", PaInt, PaOpt, 1026,  PaNL,   PaNL, ...

Try changing PaNL, PaNL to 1024, 65535 (ports under 1024 can't be used unless you are root, so, let's avoid that) and see what happens.
[ I'm sure you'll find some #define for 65535 (I know you love those), it should be called MAXUSHORT or something, from the std C lib. ]
If the limit check doesn't work for env-vars (we only recently started to use those) - there's a bug in the paParse library

[kzangeli]

/usr/include/limits.h: #define USHRT_MAX 65535

[kzangeli]

So, I modified orion-ld:

kz@xps:context.Orion-LD-5⭐ git diff src/app/orionld/orionld.cpp
diff --git a/src/app/orionld/orionld.cpp b/src/app/orionld/orionld.cpp
index 14528e5c6..b839ad27a 100644
--- a/src/app/orionld/orionld.cpp
+++ b/src/app/orionld/orionld.cpp
@@ -305,7 +305,7 @@ PaArgument paArgs[] =
   { "-noswap",                &noswap,                  "NOSWAP",                    PaBool,    PaHid,  false,           false,  true,             NOSWAP_DESC              },
   { "-fg",                    &fg,                      "FOREGROUND",                PaBool,    PaOpt,  false,           false,  true,             FG_DESC                  },
   { "-localIp",               bindAddress,              "LOCALIP",                   PaString,  PaOpt,  IP_ALL,          PaNL,   PaNL,             LOCALIP_DESC             },
-  { "-port",                  &port,                    "PORT",                      PaInt,     PaOpt,  1026,            PaNL,   PaNL,             PORT_DESC                },
+  { "-port",                  &port,                    "PORT",                      PaInt,     PaOpt,  1026,            1024,   65535,            PORT_DESC                },
   { "-pidpath",               pidPath,                  "PID_PATH",                  PaString,  PaOpt,  PIDPATH,         PaNL,   PaNL,             PIDPATH_DESC             },
   { "-dbhost",                dbHost,                   "MONGO_HOST",                PaString,  PaOpt,  LOCALHOST,       PaNL,   PaNL,             DBHOST_DESC              },
   { "-rplSet",                rplSet,                   "MONGO_REPLICA_SET",         PaString,  PaOpt,  _i "",           PaNL,   PaNL,             RPLSET_DESC              },
kz@xps:context.Orion-LD-5⭐ 

And I get this:

kz@xps:context.Orion-LD-5⭐ export ORIONLD_PORT=-1
kz@xps:context.Orion-LD-5⭐ orionld 
orionld: value -1 for option '-port' not within limits
         1024 <= -1 <= 65535 (environment variable)

The bug in not in the lib, but in the broker's main file (contextBroker.cpp for orion)

[fgalan]

I think it should be

1,   65535

instead of:

1024,   65535

as Orion shouldn't limit itself the range of ports in which it can listen. In other words, users with enough privileges should be allowed to run Orion in privileged ports (i.e. ports <=1024).

[kzangeli]

Sure, if you wish :)
If you do that, you should strongly recommend somewhere that no one runs the broker as root.
Quite unnecessary and dangerous.

[ArqamFarooqui110719]

Hi @fgalan sir,
I would like to work on this issue.
As per the above discussion, I understood that currently Orion can also run with negative port. And I have also verified the same.

As per my investigation, Currently Orion can run with any port number ( any -ve as well as +ve port number without any limit) but while querying with any API, Orion is only allowing the ports > 0 and <= 65535.

To fix the same I need to modify the src/app/contextBroker/contextBroker.cpp and limit the allowing ports from Lower Limit 1 to Upper Limit 65535.
I have confirmed that after modification, Orion is only running between ports 1 to 65535 otherwise it is giving below error:

contextBroker: value 65536 for option '-port' not within limits
               1 <= 65536 <= 65535 (command line argument)

[ArqamFarooqui110719]

Hi @fgalan sir,
PR #4287
Kindly review and merge the PR if it is OK.

[fgalan]

Fixed by PR #4287