Hello,
First of all, I am very new to bro, excuse me if I am missing something
obvious!
Anyway: I have been playing around with bro analyzing a pcap which among
other things includes an FTP transaction. I noticed that the FTP
password field was set to <hidden>.
I mananged to find the very nice documentation over at
http://www.bro-ids.org/documentation/scripts/base/protocols/ftp/main.html
which made me tweak the default_capture_password variable to "T". This
however did not seem to change the password field.
I then noticed that there was a specific check for known anonymous users
which would make sense to not store a password for, but the user in my case,
"ftpuser", was not in the list. Looking at the script it seemed to me the
test case was reversed, actually changing the password to <hidden> if
the user was _not_ in the anonymous list so i simply changed it. This
made the password visible.
I then tested changing the default_capture_password variable back and
forth but it didnt seem to make a difference: the password was shown
either way. Based on this i grepped around somewhat in the other
scripts and found that the HTTP script did a similiar thing with a
default_capture_password variable.
The actual use of the variable seemed to be missing from the FTP script,
so i added that as well based on the HTTP example.
Since i guess code says more than words, I created a git patch just to
show what was done, it can be fetched here:
http://dump.komsi.se/bro/0001-Fix-FTP-script-password-saving.patch
Finally, i might have missed it in the docs, but what would be the
preferable way to enable password capture? I'm guessing it belongs in
bro/site/local.bro but i'm not sure about the syntax to describe (in
this case) if the setting relates to HTTP or FTP etc.
Thanks for this great framework, it sure looks very interesting!
Regards,
Patrik Lundin
I then noticed that there was a specific check for known anonymous users
which would make sense to not store a password for, but the user in my case,
"ftpuser", was not in the list. Looking at the script it seemed to me the
test case was reversed, actually changing the password to <hidden> if
the user was _not_ in the anonymous list so i simply changed it. This
made the password visible.
That line of code actually works backwards from what you are thinking. The password is always captured into that field if it's seen. That line just overwrites the password before logging it if you decide that you actually don't want the password (you can inspect at runtime, but it's not logged).
I then tested changing the default_capture_password variable back and
forth but it didnt seem to make a difference: the password was shown
either way.
The actual use of the variable seemed to be missing from the FTP script,
so i added that as well based on the HTTP example.
Good catch! I totally missed that.
Finally, i might have missed it in the docs, but what would be the
preferable way to enable password capture? I'm guessing it belongs in
bro/site/local.bro but i'm not sure about the syntax to describe (in
this case) if the setting relates to HTTP or FTP etc.
redef FTP::default_capture_password = T;
Doing it in local.bro should be fine.
Thanks for this great framework, it sure looks very interesting!
Thanks for reporting the bug. I committed a slightly different fix to our fastpath branch and added "ftpuser" as another anonymous username. The fix will show up in the 2.1 release. You seem to have made the changes for yourself now to make this work at least, right?
.Seth
That line of code actually works backwards from what you are thinking.
The password is always captured into that field if it's seen. That
line just overwrites the password before logging it if you decide that
you actually don't want the password (you can inspect at runtime, but
it's not logged).
Ah, i guess i expected that "capture_password" included logging it,
but i realize it makes sense to have it available at runtime yet keep
it out of the logs.
Have i grasped it correctly that the general thinking is that the
"capture_password" knob is only intended to control if the password
is available at runtime for analysis, but that you usually don't want to
log it except for a few select users? Why have you decided that users
"probably" want to log the password for anonymous/guest users?
redef FTP::default_capture_password = T;
Doing it in local.bro should be fine.
Not sure if i'm doing it wrong, but i just added that to the end of
local.bro and it didnt't seem to do anything.
Thanks for reporting the bug. I committed a slightly different fix to
our fastpath branch and added "ftpuser" as another anonymous username.
The fix will show up in the 2.1 release. You seem to have made the
changes for yourself now to make this work at least, right?
Thanks for looking into it and explaining stuff, i actually dont have a
burning need to have "ftpuser" added, it just happened to be the user
that was used in this specific pcap. Based on my misconception that
capturing the password was the same as wanting to log it i thought
an errenous negation had snuck in :).
Lets say i wanted to actually log passwords for all users, what would
be the proper way to accomplish that?
Thanks for your time,
Patrik Lundin
Why have you decided that users
"probably" want to log the password for anonymous/guest users?
The passwords used for those accounts are considered informational. They aren't considered a secret value so we log it. 
redef FTP::default_capture_password = T;
Not sure if i'm doing it wrong, but i just added that to the end of
local.bro and it didnt't seem to do anything.
Sorry, I forgot to be explicit that what you reported was a real bug which prevented this from working. It's fixed in our fastpath branch (for small fixed like this) and will be incorporated into our 2.1 release.
Thanks for looking into it and explaining stuff, i actually dont have a
burning need to have "ftpuser" added, it just happened to be the user
that was used in this specific pcap.
That's exactly the reason I added it. We try to stick to what's actually seen in the real world and "ftpuser" seems like a reasonable name to add.
Lets say i wanted to actually log passwords for all users, what would
be the proper way to accomplish that?
Considering that this feature is broken in 2.0 you can apply the changes that I did in fastpath to your repository clone:
diff --git a/scripts/base/protocols/ftp/main.bro b/scripts/base/protocols/ftp/main.bro
index e6c0131..aa7d824 100644
--- a/scripts/base/protocols/ftp/main.bro
+++ b/scripts/base/protocols/ftp/main.bro
@@ -22,7 +22,7 @@ export {
const default_capture_password = F &redef;
## User IDs that can be considered "anonymous".
- const guest_ids = { "anonymous", "ftp", "guest" } &redef;
+ const guest_ids = { "anonymous", "ftp", "ftpuser", "guest" } &redef;
type Info: record {
## Time when the command was sent.
@@ -160,8 +160,12 @@ function ftp_message(s: Info)
# or it's a deliberately logged command.
if ( |s$tags| > 0 || (s?$cmdarg && s$cmdarg$cmd in logged_commands) )
{
- if ( s?$password && to_lower(s$user) !in guest_ids )
+ if ( s?$password &&
+ !s$capture_password &&
+ to_lower(s$user) !in guest_ids )
+ {
s$password = "<hidden>";
+ }
local arg = s$cmdarg$arg;
if ( s$cmdarg$cmd in file_cmds )
.Seth
That's exactly the reason I added it. We try to stick to what's
actually seen in the real world and "ftpuser" seems like a reasonable
name to add.
Just to be explicit, this pcap is a "dig around these bits and find out
what is bad" training/testing example. I'm not sure it is actually
based on traffic caught in the wild.
- if ( s?$password && to_lower(s$user) !in guest_ids )
+ if ( s?$password &&
+ !s$capture_password &&
+ to_lower(s$user) !in guest_ids )
+ {
s$password = "<hidden>";
+ }
I'm not sure i'm mentally parsing this right... Wouldn't this change
actually make the code log all passwords (as i expected in the first
place) if capture_password is true? Wasn't your intention to always keep
the passwords out of the logs unless specifically anonymous/guest?
It's getting very late/early here, hope im not being extraordinarily
slow!
Regards,
Patrik Lundin
You've got it right. Anonymous users always have their passwords logged. You can also specify that any arbitrary FTP session should have it's password logged by setting the $capture_password field to T.
.Seth