-
Notifications
You must be signed in to change notification settings - Fork 323
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
-vv shows password in clear text #407
Comments
Hi Uwe, well, that's what one would expect from a complete traffic log. It is correct that we have added the http traffic in 1.8.0 to the debug output, because we had several cases with authentication problems which were difficult to debug, especially when the end users don't control the vpn service. When we ask for the full traffic log we usually instruct the users to anonymize any sensitive data. On the contrary, the regular debug output line
is rather questionable, because it is hard-coded like this and printed always no matter if a password is stored in the configuration or supplied via command line or entered interactively. I'm not sure how we should proceed here. We could replace any occurrences of the clear text password by a starred string in the traffic log, but when trying to debug encoding issues of special characters, this might be a new obstacle during debugging. I'm inclined to say that we should rather remove the stared password line from the regular debug log, or print it depending on some condition. |
@UweSauter I tend to agree with @mrbaseman: the password in clear is by construction, for debugging purposes. I would like to close this issue, would that be OK? |
I still disagree that debug log files should contain clear text passwords. If a clear text password would be needed to debug encoding issues this should be a separate option. But if you want to keep current behavior, go on. This issue was meant to express my concern, not to force anything. |
well, maybe -v -v -v for the password would be an option that is relatively easy to implement |
Ah, sorry. It is not easy to implement at all, because the password is part of the packet content that is shown. So, we would have to match the content of the packets and replace everything that matches the password that has been entered before printing the packet content to the screen. |
PR #443 addresses this issue. The "easiest" implementation actually was to search for occurrences of the password in the debug output buffer and wipe them out. The implementation literally overwrites the characters, so that some password problems may still be spotted at the number of stars that are shown. I think the exact length of the password is a piece of information which is acceptable to be shown in the detailed debug output. |
I have merged #443 on the current master. The solution to this issue will be in the next release. |
the fix went into the 1.10.0 Release. |
Looks to me #443 does not work when the password contains special chars that get urlencoded. |
Ah right. We store the raw password in |
Therefore we also need to encode the password before searching it in the log stream: |
#535 is a fix for the case in which the password contains characters that need to be url_endoded. When there are no special characters, the result of the encoding routine is the same as the input. |
Fixed in openfortivpn 1.12.0. |
When running openfortivpn with "-v -v" it will show the password in clear text inside the POST request.
With this config file
test.vpn
:I get (important part)
username=testuser&credential=testpw&realm=&ajax=1&redir=%2Fremote%2Findex&just_logged_in=1
complete output:
The text was updated successfully, but these errors were encountered: