Hi Paul,

I agree with you that a nesting level of more than 4-5 is a bad thing, but I 
believe that throughout the midas code, this level is not exceeded (my poor 
mind also does not hold more than 5 things (;-) ). An indent level of 8 columns 
alone does hot force you too much in not extending the nesting level. I have 
seen code which does that, so there are nesting levels of 8 and more, which 
ends up that the code is smashed to the right side of the screen, where each 
statement is broken into many line since each line only holds 10 or 20 
characters. All the nice real estate on the left side of the scree is lost.

So having said that, I don't feel a strong need of giving up a "-i2", since the 
midas code does not contain deep nesting levels and hopefully will never have. 
In my opinion, a small indent level makes more use of your screen space, since 
you do not have a large white area at the left. A typical nesting level is 3-4, 
which causes already 32 blank charactes at the left, or 1/3 of your screen, 
just for nothing. It will lead to more lines (even with -l90), so people have 
to scroll more.

What do others think (Pierre, Konstantin, Renee) ?

> I don't feel a strong need of giving up a "-i2"...

I am comfortable with the current MIDAS styling convention and I would rather not
have yet another private religious war over the right location for the curley braces.

If we are to consider changing the MIDAS coding convention, I urge all and sundry
to read the ROOT coding convention, as written by Rene Brun and Fons Rademakers at
http://root.cern.ch/root/Conventions.html. The ROOT people did their homework, they
did read the literature and they produced a well considered and well argumented style.

Also, while there, do read the Taligent documentation- by far, one of the most
coherent manuals to C++ programming style.

K.O.

Ok, taking all comments so far into account, I conclude adopting the ROOT 
coding style would be best for us. So I put

indent:
	find . -name "*.[hc]" -exec indent -kr -nut -i3 {} \;

Into the makefile. Hope everybody is happy now (;-)))

There is a number of problems with network security in midas. (as separate from web/http/https security).

1) too many network sockets are unnecessarily bound to the external network interface instead of localhost (UDP ports are already bound to localhost on MacOS).
2) by default the RPC ports of each midas program accept connections and RPC commands from anywhere in the world (an access control list is already implemented via /Experiment/Security/Rpc Hosts, but not active by default)
3) mserver also has an access control list but it is not integrated with the access control list for the RPC ports.
4) it is difficult to run midas in the presence of firewalls (midas programs listen on random network ports - cannot be easily added to firewall rules)

There is a new git branch "feature/rpcsecurity" where I am addressing some of these problems:

1) UDP sockets are only used for internal communications (hotlinks & etc) within one machine, so they should be bound to the localhost address and become invisible to external machines. This change breaks binary compatibility from old clients - they are have to be relinked with the new midas library or hotlinks & etc will stop working. If some clients cannot be rebuild (I have one like this), I am preserving the old way by checking for a special file in the experiment directory (same place as ODB.SHM). (done)

2) if one runs on a single machine, does not use the mserver and does not have clients running on other machines, then all the RPC ports can be bound to localhost. (this kills the MacOS popups about "odbedit wants to connect to the Internet"). (partially done)

This (2) will become the new default - out of the box, midas will not listen to any external network connections - making it very secure.

To use the mserver, one will have to change the ODB setting "/Experiment/Security/Enable external RPC connections" and restart all midas programs (I am looking for a better name for this odb setting).

3) the out-of-the-box default access control list for RPC connections will be set to "localhost", which will reject all external connections, even when they are enabled by (2). One will be required to enter the names of all machines that will run midas clients in "/Experiment/Security/Rpc hosts". (already implemented in main midas, but default access control list is empty meaning everybody is permitted)

4) the mserver will be required to attach to some experiment and will use this same access control list to restrict access to the main mserver listener port. Right now the mserver listens on this port without attaching to any experiment and accepts the access control list via command line arguments. I think after this change a single mserver will still be able to service multiple experiments (TBD).

5) I am adding an option to fix TCP port numbers for MIDAS programs via "/Experiment/Security/Rpc ports/fename = (int)5555". Once a remote frontend is bound to a fixed port, appropriate openings can be made in the firewall, etc. Default port number value will be 0 meaning "use random port", same as now.

One problem remains with initial connecting to the mserver. The client connects to the main mserver listener port (easy to firewall), but then the mserver connects back to the client - this reverse connection is difficult to firewall and this handshaking is difficult to fix in the midas sources. It will probably remain unresolved for now.

K.O.

There is a number of problems with network security in midas. (as separate from 
web/http/https security).

1) too many network sockets are unnecessarily bound to the external network interface 
instead of localhost (UDP ports are already bound to localhost on MacOS).
2) by default the RPC ports of each midas program accept connections and RPC commands 
from anywhere in the world (an access control list is already implemented via 
/Experiment/Security/Rpc Hosts, but not active by default)
3) mserver also has an access control list but it is not integrated with the access control list 
for the RPC ports.
4) it is difficult to run midas in the presence of firewalls (midas programs listen on random 
network ports - cannot be easily added to firewall rules)

There is a new git branch "feature/rpcsecurity" where I am addressing some of these 
problems:

1) UDP sockets are only used for internal communications (hotlinks & etc) within one 
machine, so they should be bound to the localhost address and become invisible to external 
machines. This change breaks binary compatibility from old clients - they are have to be 
relinked with the new midas library or hotlinks & etc will stop working. If some clients cannot 
be rebuild (I have one like this), I am preserving the old way by checking for a special file in 
the experiment directory (same place as ODB.SHM). (done)

2) if one runs on a single machine, does not use the mserver and does not have clients 
running on other machines, then all the RPC ports can be bound to localhost. (this kills the 
MacOS popups about "odbedit wants to connect to the Internet"). (partially done)

This (2) will become the new default - out of the box, midas will not listen to any external 
network connections - making it very secure.

To use the mserver, one will have to change the ODB setting "/Experiment/Security/Enable 
external RPC connections" and restart all midas programs (I am looking for a better name for 
this odb setting).

3) the out-of-the-box default access control list for RPC connections will be set to 
"localhost", which will reject all external connections, even when they are enabled by (2). One 
will be required to enter the names of all machines that will run midas clients in 
"/Experiment/Security/Rpc hosts". (already implemented in main midas, but default access 
control list is empty meaning everybody is permitted)

4) the mserver will be required to attach to some experiment and will use this same access 
control list to restrict access to the main mserver listener port. Right now the mserver listens 
on this port without attaching to any experiment and accepts the access control list via 
command line arguments. I think after this change a single mserver will still be able to service 
multiple experiments (TBD).

5) I am adding an option to fix TCP port numbers for MIDAS programs via 
"/Experiment/Security/Rpc ports/fename = (int)5555". Once a remote frontend is bound to a 
fixed port, appropriate openings can be made in the firewall, etc. Default port number value 
will be 0 meaning "use random port", same as now.

One problem remains with initial connecting to the mserver. The client connects to the main 
mserver listener port (easy to firewall), but then the mserver connects back to the client - this 
reverse connection is difficult to firewall and this handshaking is difficult to fix in the midas 
sources. It will probably remain unresolved for now.

K.O.

New git branch "feature/rpcsecurity" implements  these security features:

- all UDP ports are bound to the localhost interface - connections from outside are not possible
- by default out of the box MIDAS RPC TCP ports are bound to the localhost interface - connections from the outside are not possible.

This configuration is suitable for testing MIDAS on a laptop and for running a simple experiment where all programs run on one machine.

This configuration is secure (connections from the outside are not possible).

This configuration makes corporate security people happy - MIDAS ports do not show up on network port scans (nmap & etc). (except for the mhttpd 
web ports).

The change in binding UDP ports is incompatible with previous versions of MIDAS (except on MacOS, where UDP ports were always bound to localhost). 
All MIDAS programs should be rebuild and restarted, otherwise ODB hotlinks and some other stuff will not work. If rebuilding all MIDAS programs is 
impossible (for example I have one magic MIDAS frontend that cannot be rebuilt), one can force the old (insecure) behavior by creating a file 
.UDP_BIND_HOSTNAME in the experiment directory (next to .ODB.SHM).

The mserver will still work in this localhost-restricted configuration - one should use "odbedit -h localhost" to connect. Multiple mserver instances on 
the same machine - using different TCP ports via "-p" and ODB "/Experiment/midas server port" - are still supported.

To run MIDAS programs on remote machines, one should change the ODB setting "/Experiment/Security/Enable non-localhost RPC" to "yes" and 
add the hostnames of all remote machines that will run MIDAS programs to the MIDAS RPC access control list in ODB "/Experiment/Security/RPC hosts".

To avoid "guessing" the host names expected by MIDAS, do this: set "enable non-localhost rpc" to "yes" and restart the mserver. Then go to the remote 
machine and try to start the MIDAS program, i.e. "odbedit -h daq06". This will bomb and there will be a message in the midas log file saying - rejecting 
connection from unallowed host 'ladd21.triumf.ca'. Add this host to "/Experiment/Security/RPC hosts". After you add this hostname to "RPC hosts" and 
restart the mserver, the connection should be successful. When "RPC hosts" is fully populated, one should restart all midas programs - the access 
control list is only loaded at program startup.

If MIDAS clients have to connect from random hosts (i.e. dynamically assigned random DHCP addresses), one can disable the host name checks by 
setting ODB "/experiment/security/Disable RPC hosts check" to "yes". This configuration is insecure and should only be done on a private network 
behind a firewall.

After some more testing this branch will be merged into the main midas.

K.O.

Sometimes (maybe after a client uncleanly exits?), I see phantom "open
records", for example:
[local:twist:Running]Gas>sor
/Equipment/Gas/Common open 2 times by fe1hp 
/Equipment/Gas/Variables open 1 times by Logger 
/Equipment/Gas/Variables/Flow1 open 2 times by uBeamTcl1 uBeamTcl 
/Equipment/Gas/Settings/Command open 2 times by fe1hp 
/Equipment/Gas/Statistics open 1 times by 

Note the blank client name in the "/Equipment/Gas/Statistics" line.

This causes these warnings from mfe.c:
Cannot init equipment record, probably other FE is using it
Cannot delete statistics record, error 320
Cannot create statistics record, error 320
Cannot open statistics record, error 318. Probably other FE is using it

Then the number of generated events for this front end is never incremented.

Also attempts to delete this "open" record fail:
[local:twist:Running]Gas>del /Equipment/Gas/Statistics
Are you sure to delete the key
"/Equipment/Gas/Statistics"
and all its subkeys? (y/[n]) y
key is open by other client

How do I go about writing the db_validate_xxx() code to cleanup this
bogosity? I am not too familiar with the implementation of "open record"...

K.O.

I have seen the same behaviour and it annoys me, too. What I did in the past 
is a "cleanup" in ODBEdit which removes these open records. I have soem code 
in cm_watchdog(), which should take care of that. If a client is dead, it 
gets removed from the ODB, and its open records should get its notify_count 
decremented. So obviously this code has some bug. I plan to do in the 
following week (now I got some spare time) the following:

- exchange most db_create_record() by something better. Maybe 
db_check_record(..., correct_flag), which creates the record only if it does 
not exist at all, otherwise checks the structure. If correct_flag is TRUE, it 
corrects the strucure (by calling db_create_record()), if it's false it just 
returns an error code. This way one can decide from case to case which option 
is better. Like for the /Runinfo, the flag would be FALSE, maybe with a 
notification that the /Runinfo is different from the compiled-in structure, 
and one hast to recompile the application.

- revisit the open record issue from dying frontends. I remember vaguely that 
I tried to kill a frontend (kill -9), wait until the watchdog cleans up its 
entries, and it worked fine. So it's more the problem to reproduce the issue 
described in the previous elog entry. 

I tried to reproduce the problem, but without success. So in case this happens 
again, one should debug the code im cm_watchdog() next to the line

/* decrement notify_count for open records and clear exclusive mode */
...

So if a killed client is removed from the ODB via the watchdog (or a "cleanup" 
is done in ODBEdit), the notify_count should be decreased and thus the "open 
records" should be closed.

> [system.c:308:ss_shm_open,ERROR] Shared memory segment with key 0x4d008002 already exists, 
please remove it manually: ipcrm -M 0x4d008002
> [midas.c:1950:cm_connect_experiment1,ERROR] cannot open database
> Unexpected error #304

For the record, the SYSV shared memory with it's keys and segments has always been brittle and hard to 
debug with problems such as you describe.

Also SYSV shared memory suffers from key aliasing - shared memory segments created with different 
names all map into the same key, collide and nothing works. You may not see this if all the files are 
located on a local disk, but if the .SHM files are located on an NFS disk, it can happen (and did happen in 
T2K).

For this reason, since around August 2010, MIDAS also implements the POSIX shared memory and for new 
MIDAS installations, POSIX shared memory is the default. (On MacOS, POSIX shared memory was always 
the default because MacOS has very small maximum SYSV shared memory size).

The type of shared memory is set by the contents of .SHM_TYPE.TXT and it is possible to switch between 
SYSV and POSIX shared memory at will. (Ask me).

MIDAS still uses SYSV semaphores because they have a built-in feature to automatically unlock the 
semaphore if the program that locked it dies for any reason. POSIX semaphores do not have this built-in 
feature and we would have to implement some kind of detection and recovery for the case when a 
semaphore is locked by a program that died (and will never unlock it back).

K.O.

P.S. I will address the rest of Prof. Thorsten's question in a private email.

P.P.S. Please post elog messages in the "plain" format. NOT HTML or ELCODE.

[quote="Stefan Ritt"][quote="Thorsten Lux"]In addition now I cannot start
anymore the mlogger from the web interface but only manually. However, I can
stop it from the web interface.[/quote]

At least that one can be fixed easily. Each program has a certain command with
which one can start it. This has to be put into the ODB under
/Programs/<program>. In your case you probably need

/Programs/Logger/Start command = mlogger -D

to start the logger from the Web page. To debug your run stop problems, I would
recommend to start all programs in a terminal window and look which one crashes
on the run end.

/Stefan[/quote]


Hi Stefan,

under /Programs/Logger/Start command I have
/home/next/MIDAS/midas/linux/bin/mlogger -D . This command does not work if I
press the "Start Logger" button on the mhttpd webpage but when I copy and paste
this command to a terminal window, it does the job. 

Well, thanks to you both for the fast response. I wrote Konstantin an email with
the results of the tests he suggested me to do.

Ciao

> Hi Stefan,
> 
> under /Programs/Logger/Start command I have
> /home/next/MIDAS/midas/linux/bin/mlogger -D . This command does not work if I
> press the "Start Logger" button on the mhttpd webpage but when I copy and paste
> this command to a terminal window, it does the job. 
> 
> Well, thanks to you both for the fast response. I wrote Konstantin an email with
> the results of the tests he suggested me to do.
> 
> Ciao

Let me guess: mhttpd is started under root (to be able to connect to port 80), and for root the mlogger program 
is not in the path. Try to put into the odb the full path:

/Programs/Logger Start command = /usr/local/bin/mlogger -D

> 
> > Hi Stefan,
> > 
> > under /Programs/Logger/Start command I have
> > /home/next/MIDAS/midas/linux/bin/mlogger -D . This command does not work if I
> > press the "Start Logger" button on the mhttpd webpage but when I copy and paste
> > this command to a terminal window, it does the job. 
> > 
> > Well, thanks to you both for the fast response. I wrote Konstantin an email with
> > the results of the tests he suggested me to do.
> > 
> > Ciao
> 
> Let me guess: mhttpd is started under root (to be able to connect to port 80), and for root the mlogger program 
> is not in the path. Try to put into the odb the full path:
> 
> /Programs/Logger Start command = /usr/local/bin/mlogger -D

Yes, mhttpd is started as sudo, but I have the full path in the start command. And every user has the right to
execute mlogger. But okay, I will concentrate first to get the rest working again and then I will fight this problem.

Thanks!

Hi,

it seems that I solved the problem in a quite brutal way.
I opened the database with odbedit and saved first the whole database as a ASCII
file and then I did the same for each section separately. Then I closed odbedit.
Afterwards I deleted all .*.SHM files including .ODB.SHM and rebooted the system.
After the restart I opened odbedit and started mhttpd. With this blank system
the problem had disappeared. Afterwards I loaded section by section from the
previous created ASCII files. After each section I tested if I can start and
stop runs and it worked without problems. At the end I also loaded the ASCII
file which contained the whole database. In this case I got the following error
message:
[odb.c:6038:db_paste,ERROR] found string exceeding MAX_STRING_LENGTH

However, after a reboot everything worked fine. I can start and stop runs, with
and without frontend, without any error message. Only the mlogger resisted to
work again. 

But also this problem we solved. It seems it was related to a missing library
path. It is strange since while in the mhttpd web page the command does not work
and is not giving any error message, copying the same command to a terminal and
to start it manually does the job. We solved it by putting the start of mlogger
in a simple shell script and to execute it then from the mhttpd web page.
Probably not an elegant solution but it does the job.

Well, with this I can enjoy my weekend to start over with data taking next week!

Thanks a lot!

Thorsten

> [odb.c:6038:db_paste,ERROR] found string exceeding MAX_STRING_LENGTH

Ok, so here is what probably happened. Some user program wrote a long string into the ODB and somehow corrupted it. This corruption persists as long as you work with 
binary data. Indeed "rebuilding" the ODB helps in that case. What we do actually is at the beginning of every run, the ODB contents is dumped into the data file via

/Logger/Channels/0/Setting/ODB dump

in case we get ODB corruption, we clear all *.shm files as well as the shared memory segments, create a fresh ODB, extract the ODB from the last successful run via

odbhist -e runxxx.mid

and load it via odbedit. I put some additional code in most midas functions to prevent this corruption (and thus your saw the above error "found string exceeding 
MAX_STRING_LENGTH"), but since the ODB is physically in the address space of each midas program, they can theoretically bypass the midas functions and write accidentally 
into the ODB with an uninitialized pointer or so.

Best regards,
Stefan

The current h1_book() macros (and the previous example analyzer code) have an
odd persistency problem: for example, the user wants to change some histogram
limits, edits the h1_book() calls, rebuilds and restarts the analyzer, starts a
new run, and observes that all histograms are filled using the old limits, his
changes "did not take". The user panics, I get paged during the Holy Lunch Hour,
everybody is unhappy.

This is what I think happens:

1) analyzer starts
2) LoadRootHistgrams() loads old histograms from file
3) user code calls h1_book()
4) h1_book template in midas.h does this (roughly):
      hist = (TH1X *) gManaHistosFolder->FindObjectAny(name);
      if (hist == NULL) {
         hist = new TH1X(name, title, bins, min, max);
5) since the histogram already exists (loaded from the file, with the old
limits), the TH1X constructor is not called at all, new histogram limits are
utterly ignored.

A possible solution is to unconditionally create the ROOT objects, like I do in
the example code posted at http://dasdevpc.triumf.ca:9080/Midas/191. That code
produces an annoying warning from ROOT about possible memory leaks. This could
be fixed by adding a two liner to "find and delete" the object before it is
created, trippling the number of user code lines per histogram (find & delete,
then create). Highly ugly.

midas.h macros (h1_book & co) can be fixed by adding checks for histogram limits
and such, but I would much prefer a generic solution/convention that would work
for arbitrary ROOT objects without MIDAS-specific wrappers (think TProfile,
TGraph, etc...).

Any suggestions?

K.O.

> The current h1_book() macros (and the previous example analyzer code) have an
> odd persistency problem: for example, the user wants to change some histogram
> limits, edits the h1_book() calls, rebuilds and restarts the analyzer, starts a
> new run, and observes that all histograms are filled using the old limits, his
> changes "did not take". The user panics, I get paged during the Holy Lunch Hour,
> everybody is unhappy.
> 
> This is what I think happens:
> 
> 1) analyzer starts
> 2) LoadRootHistgrams() loads old histograms from file

I can't get onto cvs@midas.psi.ch right now
(cvs update
cvs@midas.psi.ch's password: 
Permission denied, please try again.)

but when I changed LoadRootHistograms a few days ago I left it as:

    } else if (obj->InheritsFrom( "TH1")) {

      // still don't know how to do TH1s

so h1_book() is creating the first and only copy of the histograms.
I am able to create new histogram limits.
I don't get the memory leak problems.

However I have seen the memory leak problems before, and they are real.
They must be dealt with either by (1) first deleteing the old histogram
or (2) ensuring that histogram names are unique in the whole application
(different modules/folders can not use the same histogram names).

I will return to this once I can do a cvs update for midas.

John.

> 3) user code calls h1_book()
> 4) h1_book template in midas.h does this (roughly):
>       hist = (TH1X *) gManaHistosFolder->FindObjectAny(name);
>       if (hist == NULL) {
>          hist = new TH1X(name, title, bins, min, max);
> 5) since the histogram already exists (loaded from the file, with the old
> limits), the TH1X constructor is not called at all, new histogram limits are
> utterly ignored.
> 
> A possible solution is to unconditionally create the ROOT objects, like I do in
> the example code posted at <a
href="http://dasdevpc.triumf.ca:9080/Midas/191">http://dasdevpc.triumf.ca:9080/Midas/191</a>.
That code
> produces an annoying warning from ROOT about possible memory leaks. This could
> be fixed by adding a two liner to "find and delete" the object before it is
> created, trippling the number of user code lines per histogram (find & delete,
> then create). Highly ugly.
> 
> midas.h macros (h1_book & co) can be fixed by adding checks for histogram limits
> and such, but I would much prefer a generic solution/convention that would work
> for arbitrary ROOT objects without MIDAS-specific wrappers (think TProfile,
> TGraph, etc...).
> 
> Any suggestions?
> 
> K.O.

> I can't get onto cvs@midas.psi.ch right now
> (cvs update
> cvs@midas.psi.ch's password: 
> Permission denied, please try again.)

I had to upgrade midas.psi.ch today with Scientific Linux 3.03. Most things are back to work, but
 I failed to do the anonymous CVS account. I have to wait for next week when the experts are
there. I will let you know when it's working again.

- Stefan