Skip to content

Commit

Permalink
Fixes password leak of Auth plugins to Audit Logs (#57)
Browse files Browse the repository at this point in the history
* Auth plugin adds `(sensitive)` instead of plain passwords
to AuditLogs
* Added generic `isSensitive()` to identify Passwords before logging

Signed-off-by: Dinesh Prasanth M K <dmoluguw@redhat.com>
  • Loading branch information
SilleBille committed Oct 1, 2018
1 parent 19120d1 commit cc2b50f
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 80 deletions.
30 changes: 30 additions & 0 deletions base/common/src/com/netscape/certsrv/apps/CMS.java
Original file line number Diff line number Diff line change
Expand Up @@ -1671,6 +1671,36 @@ public static boolean isExcludedLdapAttr(String key) {
return _engine.isExcludedLdapAttr(key);
}

/**
* Check whether the string is contains password
*
* @param name key string
* @return whether key is a password or not
*/
public static boolean isSensitive(String name) {
return (name.startsWith("__") ||
name.endsWith("password") ||
name.endsWith("passwd") ||
name.endsWith("pwd") ||
name.equalsIgnoreCase("admin_password_again") ||
name.equalsIgnoreCase("directoryManagerPwd") ||
name.equalsIgnoreCase("bindpassword") ||
name.equalsIgnoreCase("bindpwd") ||
name.equalsIgnoreCase("passwd") ||
name.equalsIgnoreCase("password") ||
name.equalsIgnoreCase("pin") ||
name.equalsIgnoreCase("pwd") ||
name.equalsIgnoreCase("pwdagain") ||
name.equalsIgnoreCase("uPasswd") ||
name.equalsIgnoreCase("PASSWORD_CACHE_ADD") ||
name.startsWith("p12Password") ||
name.equalsIgnoreCase("host_challenge") ||
name.equalsIgnoreCase("card_challenge") ||
name.equalsIgnoreCase("card_cryptogram") ||
name.equalsIgnoreCase("drm_trans_desKey") ||
name.equalsIgnoreCase("cert_request"));
}

/**
* Main driver to start CMS.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,21 +203,7 @@ public void outputHttpParameters(HttpServletRequest httpReq) {
// __ (double underscores); however, in the event that
// a security parameter slips through, we perform multiple
// additional checks to insure that it is NOT displayed
if (pn.startsWith("__") ||
pn.endsWith("password") ||
pn.endsWith("passwd") ||
pn.endsWith("pwd") ||
pn.equalsIgnoreCase("admin_password_again") ||
pn.equalsIgnoreCase("directoryManagerPwd") ||
pn.equalsIgnoreCase("bindpassword") ||
pn.equalsIgnoreCase("bindpwd") ||
pn.equalsIgnoreCase("passwd") ||
pn.equalsIgnoreCase("password") ||
pn.equalsIgnoreCase("pin") ||
pn.equalsIgnoreCase("pwd") ||
pn.equalsIgnoreCase("pwdagain") ||
pn.equalsIgnoreCase("uPasswd") ||
pn.equalsIgnoreCase("PASSWORD_CACHE_ADD")) {
if (CMS.isSensitive(pn)) {
CMS.debug("AdminServlet::service() param name='" + pn +
"' value='(sensitive)'");
} else {
Expand Down Expand Up @@ -992,7 +978,7 @@ protected String auditParams(HttpServletRequest req) {
if (name.equals(Constants.RS_ID)) continue;

String value = null;
if (name.equalsIgnoreCase("PASSWORD_CACHE_ADD"))
if (CMS.isSensitive(name))
value = "(sensitive)";
else
value = req.getParameter(name);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -403,26 +403,7 @@ public void outputHttpParameters(HttpServletRequest httpReq) {
// __ (double underscores); however, in the event that
// a security parameter slips through, we perform multiple
// additional checks to insure that it is NOT displayed
if (pn.startsWith("__") ||
pn.endsWith("password") ||
pn.endsWith("passwd") ||
pn.endsWith("pwd") ||
pn.equalsIgnoreCase("admin_password_again") ||
pn.equalsIgnoreCase("directoryManagerPwd") ||
pn.equalsIgnoreCase("bindpassword") ||
pn.equalsIgnoreCase("bindpwd") ||
pn.equalsIgnoreCase("passwd") ||
pn.equalsIgnoreCase("password") ||
pn.equalsIgnoreCase("pin") ||
pn.equalsIgnoreCase("pwd") ||
pn.equalsIgnoreCase("pwdagain") ||
pn.startsWith("p12Password") ||
pn.equalsIgnoreCase("uPasswd") ||
pn.equalsIgnoreCase("host_challenge") ||
pn.equalsIgnoreCase("card_challenge") ||
pn.equalsIgnoreCase("card_cryptogram") ||
pn.equalsIgnoreCase("drm_trans_desKey") ||
pn.equalsIgnoreCase("cert_request")) {
if (CMS.isSensitive(pn)) {
CMS.debug("CMSServlet::service() param name='" + pn +
"' value='(sensitive)'");
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,20 +70,7 @@ public void outputHttpParameters(HttpServletRequest httpReq) {
// __ (double underscores); however, in the event that
// a security parameter slips through, we perform multiple
// additional checks to insure that it is NOT displayed
if (pn.startsWith("__") ||
pn.endsWith("password") ||
pn.endsWith("passwd") ||
pn.endsWith("pwd") ||
pn.equalsIgnoreCase("admin_password_again") ||
pn.equalsIgnoreCase("directoryManagerPwd") ||
pn.equalsIgnoreCase("bindpassword") ||
pn.equalsIgnoreCase("bindpwd") ||
pn.equalsIgnoreCase("passwd") ||
pn.equalsIgnoreCase("password") ||
pn.equalsIgnoreCase("pin") ||
pn.equalsIgnoreCase("pwd") ||
pn.equalsIgnoreCase("pwdagain") ||
pn.equalsIgnoreCase("uPasswd")) {
if (CMS.isSensitive(pn)) {
CMS.debug("BaseServlet::service() param name='" + pn +
"' value='(sensitive)'");
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -258,21 +258,7 @@ protected void printParameterValues(HashMap<String, String> data) {
// __ (double underscores); however, in the event that
// a security parameter slips through, we perform multiple
// additional checks to insure that it is NOT displayed
if (paramName.startsWith("__") ||
paramName.endsWith("password") ||
paramName.endsWith("passwd") ||
paramName.endsWith("pwd") ||
paramName.equalsIgnoreCase("admin_password_again") ||
paramName.equalsIgnoreCase("directoryManagerPwd") ||
paramName.equalsIgnoreCase("bindpassword") ||
paramName.equalsIgnoreCase("bindpwd") ||
paramName.equalsIgnoreCase("passwd") ||
paramName.equalsIgnoreCase("password") ||
paramName.equalsIgnoreCase("pin") ||
paramName.equalsIgnoreCase("pwd") ||
paramName.equalsIgnoreCase("pwdagain") ||
paramName.equalsIgnoreCase("uPasswd") ||
paramName.equalsIgnoreCase("cert_request")) {
if (CMS.isSensitive(paramName)) {
CMS.debug("CAProcessor: - " + paramName + ": (sensitive)");
} else {
CMS.debug("CAProcessor: - " + paramName + ": " + entry.getValue());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@
import com.netscape.certsrv.base.EBaseException;
import com.netscape.certsrv.base.SessionContext;
import com.netscape.certsrv.logging.AuditEvent;
import com.netscape.certsrv.logging.ILogger;
import com.netscape.certsrv.logging.event.AuthEvent;
import com.netscape.certsrv.logging.event.CertRequestProcessedEvent;
import com.netscape.certsrv.logging.ILogger;
import com.netscape.certsrv.profile.ECMCBadIdentityException;
import com.netscape.certsrv.profile.ECMCBadMessageCheckException;
import com.netscape.certsrv.profile.ECMCBadRequestException;
Expand Down Expand Up @@ -306,20 +306,7 @@ public void process(CMSRequest cmsReq) throws EBaseException {
// __ (double underscores); however, in the event that
// a security parameter slips through, we perform multiple
// additional checks to insure that it is NOT displayed
if (paramName.startsWith("__") ||
paramName.endsWith("password") ||
paramName.endsWith("passwd") ||
paramName.endsWith("pwd") ||
paramName.equalsIgnoreCase("admin_password_again") ||
paramName.equalsIgnoreCase("directoryManagerPwd") ||
paramName.equalsIgnoreCase("bindpassword") ||
paramName.equalsIgnoreCase("bindpwd") ||
paramName.equalsIgnoreCase("passwd") ||
paramName.equalsIgnoreCase("password") ||
paramName.equalsIgnoreCase("pin") ||
paramName.equalsIgnoreCase("pwd") ||
paramName.equalsIgnoreCase("pwdagain") ||
paramName.equalsIgnoreCase("uPasswd")) {
if (CMS.isSensitive(paramName)) {
CMS.debug("ProfileSubmitCMCServlet Input Parameter " +
paramName + "='(sensitive)'");
} else {
Expand Down

0 comments on commit cc2b50f

Please sign in to comment.