The sensitive information of uri may leak

[YLChen-007]

ISSUE TYPE

• Bug Report

COMPONENT NAME

cloudstack-service-secondary-storage 

CLOUDSTACK VERSION

commit ID: 45d267ccbf2749c547cbbbac4a2cb1f3351dcaf2 on main branch.

CONFIGURATION

OS / ENVIRONMENT

SUMMARY

The sensitive information of URI may leak through "logger.error"

STEPS TO REPRODUCE

EXPECTED RESULTS

ACTUAL RESULTS

error code location

protected String parseCifsMountOptions(URI uri) {
    List<NameValuePair> args = URLEncodedUtils.parse(uri, "UTF-8");
    boolean foundUser = false;
    boolean foundPswd = false;
    StringBuilder extraOpts = new StringBuilder();
    for (NameValuePair nvp : args) {
        String name = nvp.getName();
        if (name.equals("user")) {
            foundUser = true;
            logger.debug("foundUser is" + foundUser);
        } else if (name.equals("password")) {
            foundPswd = true;
            logger.debug("password is present in uri");
        }

        extraOpts.append(name + "=" + nvp.getValue() + ",");
    }

    if (logger.isDebugEnabled()) {
        logger.error("extraOpts now " + extraOpts); //output 
    }

    if (!foundUser || !foundPswd) {
        String errMsg = "Missing user and password from URI. Make sure they" + "are in the query string and separated by '&'.  E.g. "
                + "cifs://example.com/some_share?user=foo&password=bar";
        logger.error(errMsg);
        throw new CloudRuntimeException(errMsg);
    }
    return extraOpts.toString();
}

[DaanHoogland]

@1561316811 , looking at the logs above I do now see an issue. Please explain?

[YLChen-007]

In the following code, in line 3096, we can infer that "nvp" variable may contain password by lines 3101 and 3106. But the "extraOpts" appends the information of "nvp.getValue()" and then was logged out in line 3110.

cloudstack/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java

Lines 3091 to 3120 in 2959cc6

	protected String parseCifsMountOptions(URI uri) {
	List<NameValuePair> args = URLEncodedUtils.parse(uri, "UTF-8");
	boolean foundUser = false;
	boolean foundPswd = false;
	StringBuilder extraOpts = new StringBuilder();
	for (NameValuePair nvp : args) {
	String name = nvp.getName();
	if (name.equals("user")) {
	foundUser = true;
	logger.debug("foundUser is" + foundUser);
	} else if (name.equals("password")) {
	foundPswd = true;
	logger.debug("password is present in uri");
	}
	extraOpts.append(name + "=" + nvp.getValue() + ",");
	}
	if (logger.isDebugEnabled()) {
	logger.error("extraOpts now " + extraOpts);
	}
	if (!foundUser || !foundPswd) {
	String errMsg = "Missing user and password from URI. Make sure they" + "are in the query string and separated by '&'. E.g. "
	+ "cifs://example.com/some_share?user=foo&password=bar";
	logger.error(errMsg);
	throw new CloudRuntimeException(errMsg);
	}
	return extraOpts.toString();
	}

[DaanHoogland]

You are right, I will create a PR to add a call to StringUtils.cleanString() here.