Inconsitent labeling of POP3 connections in connection logs

[matthias]

POP3 connections are unfortunately labeled inconsistently in conn.log: both the pop3 and pop-3 exist. I don't know if the following patch makes sense because it would probably break backwards compatibility with existing post-analysis scripts.

Index: port-name.bro
===================================================================
--- port-name.bro	(revision 6680)
+++ port-name.bro	(working copy)
@@ -15,8 +15,8 @@
 	[53/tcp] = "dns",
 	[79/tcp] = "finger",
 	[80/tcp] = "http",
-	[109/tcp] = "pop-2",
-	[110/tcp] = "pop-3",
+	[109/tcp] = "pop2",
+	[110/tcp] = "pop3",
 	[111/tcp] = "portmap",
 	[113/tcp] = "ident",
 	[119/tcp] = "nntp",

The only other location I where pop3 rather than pop-3 is mentioned was in sigs/dpd.sig:

signature dpd_pop3_server {
  ip-proto == tcp
  payload /^\+OK/
  requires-reverse-signature dpd_pop3_client
  enable "pop3"
  tcp-state responder
}

However, I am not sure if the enable "pop3" line is causing a pop3 service tag in conn.log.

[robin]

I think the culprit is the POP3 analyzer name in Analyzer.cc. The name from there can now show in the new-style DPD connection log and is also the label used by the signature "enable" keyword. I suggest to change the name in Analyzer.cc so that we are consistent with how Bro has been calling pop-3 in the past.

[matthias]

So would that be a matter of changing the AnalyzerTag? in Analyzer.cc to:

    AnalyzerTag::POP3, "POP-3", POP3_Analyzer::InstantiateAnalyzer,
        POP3_Analyzer::Available, 0, false },

I believe it is a pretty important issue because the trace-summaries in broctl currently have two entries, pop3 and pop-3. It is very confusing and annoying if one has to look at two different labels.

[robin]

Yes. Can you try that change and see if it fixes the problem?

[matthias]

It's unfortunately not working that way:

[..]/share/bro/pop3.bro, line 21: error: unknown identifier ANALYZER_POP, at or near "ANALYZER_POP"

[robin]

Ah, those constants are automatically generated, and now that needs to be ANALYZER_POP-3 (hope that works though ... Would be nicer, if it were ANALYZER_POP_3 )

[matthias]

If I used ANALYZER_POP_3 the connection logs would probably show pop_3, right? This will not fix the inconsistency though. Do you have another idea how to approach this issue? It seems that the easiest fix will be adapting port-names.bro.

[robin]

No, the string in the analyzer table specifies what will show up in the logs. That string is also used to generate the constants, but for that (and only for that) we could add a canocalization of s/-/_/; that would go into DPM.cc