Project

General

Profile

Bug #9345

Hide username/passwords for CHAP iSCSI in save debug output

Added by Josh Paetzel over 5 years ago. Updated about 4 years ago.

Status:
Resolved
Priority:
Expected
Assignee:
Josh Paetzel
Category:
OS
Target version:
Seen in:
Severity:
New
Reason for Closing:
Reason for Blocked:
Needs QA:
Yes
Needs Doc:
Yes
Needs Merging:
Yes
Needs Automation:
No
Support Suite Ticket:
n/a
Hardware Configuration:
ChangeLog Required:
No

Description

When doing a save debug ctl.conf is included. This file contains CHAP usernames and passwords used to authenticate to iSCSI. (The old iSCSI target had a separate file with CHAP users and passwords which wasn't included in the save debug output, now that everything is in clt.conf we can't just blindly copy the file into save debug output)

Associated revisions

Revision 2b25086c (diff)
Added by Josh Paetzel over 5 years ago

Add support for a shadow ctl.conf that has no CHAP username/passwords in it. This file is consumed by freenas-debug to keep usernames and passwords out of it's output. Ticket: #9345 Merge-FN93: y Merge-TN93: y

Revision 98dc9827 (diff)
Added by Josh Paetzel over 5 years ago

Add support for a shadow ctl.conf that has no CHAP username/passwords in it. This file is consumed by freenas-debug to keep usernames and passwords out of it's output. Ticket: #9345 Merge-FN93: y Merge-TN93: y (cherry picked from commit 2b25086cb11b49f3d1a7184387a13dc3e03890c3)

Revision 6e96991b (diff)
Added by Josh Paetzel over 5 years ago

Add support for a shadow ctl.conf that has no CHAP username/passwords in it. This file is consumed by freenas-debug to keep usernames and passwords out of it's output. Ticket: #9345 Merge-FN93: y Merge-TN93: y (cherry picked from commit 2b25086cb11b49f3d1a7184387a13dc3e03890c3)

History

#1 Updated by Xin Li over 5 years ago

I think this can be fixed with:

diff --git a/src/freenas/usr/local/libexec/freenas-debug/iscsi/iscsi.sh b/src/freenas/usr/local/libexec/freenas-debug/iscsi/iscsi.sh
index 0324982..7defd90 100755
--- a/src/freenas/usr/local/libexec/freenas-debug/iscsi/iscsi.sh
+++ b/src/freenas/usr/local/libexec/freenas-debug/iscsi/iscsi.sh
@@ -33,7 +33,7 @@ iscsi_directory() { echo "iSCSI"; }
 iscsi_func()
 {
        section_header "/etc/ctl.conf" 
-       sc "/etc/ctl.conf" 
+       sc "/etc/ctl.conf" | sed -e 's,^        chap .*$,       chap XXXX XXXXX' -e 's,^        chap-mutual .*$,        chap-mutual XXXX XXXXX'
        section_footer

        section_header "ctladm devlist -v" 

I wouldn't consider it a critical one, though, but rather a nice to have thing: if iSCSI is exposed to the Internet or even untrusted network, the user already have a much bigger problem.

#2 Updated by Jordan Hubbard over 5 years ago

Another option would be to break the authentication data into an optional 2nd file, /etc/ctl.auth - ctld could load /etc/ctl.conf and then, if it detects an /etc/ctl.auth file, load that as well. That way we get backwards compatibility with old ctl.conf files but allow users to break the authentication information into a separate file so that they can attach it to bug reports, share it with other people, etc without accidentally disclosing their passwords. A lot of similar services try to break that information into two places for this exact reason. Doing that would allow us to simply continue to export ctl.conf as we do now, we would simply have to also generate a ctl.auth file in the presence of authentication information in the config.

#3 Updated by Josh Paetzel over 5 years ago

  • Status changed from Investigation to 15
  • Assignee changed from Josh Paetzel to Jordan Hubbard

So are you going to do this jordan? You said it was a 20 minute fix.

#4 Updated by Jordan Hubbard over 5 years ago

  • Status changed from 15 to Investigation
  • Priority changed from Critical to Expected

I said it would probably take 20 minutes to do it, not unlike many of your 20 minute tasks that take weeks to complete, if ever, because that 20 minutes never quite becomes available in one segment. I will look into it as time permits. Critical, however, this is not.

#5 Updated by Charles Volgemann over 5 years ago

wow this is flying by the seat of our pants more like crop dusting under the power lines than barn storming. I think we may have some wire wrapped around our landing gear from the last pass. I am fladd I held back on my update as you suggested.

#6 Updated by Jordan Hubbard over 5 years ago

I think the flying analogy has hit the ground. In any case, just to be clear, this "issue" with CTL has been around since 9.2.1.x, so delaying or skipping an upgrade wouldn't have any bearing on this issue whatsoever.

I've also looked at the code and the way that ctld handles its preference information (and reloading of same) suggests that I was wrong - this won't be easy to break into another file. We need to go with the obfuscation of the save debug output, as Xin suggests.

#7 Updated by Josh Paetzel over 5 years ago

It wasn't an issue until a very recent SU as CTL.conf wasn't included in the save debug.

I'm more inclined to think generating a shadow file with no passwords and using that in the save debug is slightly more robust than trying to cook the output after the fact. Certainly the latter is more vulnerable to being broken by subtle changes.

Let me know which way you want to go and I'll do it for tomorrow's SU.

#8 Updated by Josh Paetzel over 5 years ago

  • Status changed from Investigation to 15

#9 Updated by Jordan Hubbard over 5 years ago

  • Status changed from 15 to Screened
  • Assignee changed from Jordan Hubbard to Josh Paetzel

I think the shadow password idea would be fine too - at this point, we've certainly spent more time in discussion about the matter than it even merits. :)

#10 Updated by Josh Paetzel over 5 years ago

  • Status changed from Screened to Ready For Release

#11 Updated by Jordan Hubbard over 5 years ago

  • Status changed from Ready For Release to Resolved

#12 Avatar?id=14398&size=24x24 Updated by Kris Moore about 4 years ago

  • Target version changed from Unspecified to N/A

Also available in: Atom PDF