Skip to content
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

SMB outbound gateway may expose sensitive data in headers or payload #9399

Open
smitsjelle opened this issue Aug 19, 2024 · 5 comments
Open
Labels
status: waiting-for-reporter Needs a feedback from the reporter type: bug

Comments

@smitsjelle
Copy link

In what version(s) of Spring Integration are you seeing this issue?
Spring integration 6.2.1

Describe the bug

While playing around with a SMB outbound gateway, I encountered it may log the username and password of the SMB share, whereas this would normally be properly masked (when using JCIF's toString method on an SMBFile).

During some tests where I used 2 outbound gateways that first listed the files, and subsequently removed some of them, I found the full path to the SMB share (i.e. smb://[username]:[password]:[host]:[port]/path)) to be put in the file_remoteDirectory header and in an INFO level log entry.

To Reproduce
Configure the session factory to any SMB share with files on it, and publish any message on the request channel to trigger

I used the following (XML) configuration:

<smb:outbound-gateway reply-channel="listedFiles"
					  request-channel="listFiles"
					  session-factory="smbSessionfactory"
					  auto-create-local-directory="true" 
					  command="ls" 
					  expression="'generated/'"
					  local-directory="./tmp"/>

<int:channel id=listedFiles/>

<int:splitter input-channel="listedFiles" output-channel="splitFiles"/>

<int:channel id=splitFiles/>

<smb:outbound-gateway session-factory="smbSessionfactory" 
					  request-channel="splitFiles" 
					  reply-channel="logRemove"
					  command="rm" 
					  expression="payload.getFileInfo().getPath()"/> <!-- Note that getPath gives the entire SMB URL in this case -->

<int:logging-channel-adapter channel="logRemove" log-full-message="true"/>

The SMB outbound gateway executing the 'remove' command above will subsequently log the 'path' (that includes the entire SMB URL and thus the username and password) as per SMBSession#113 Additionally, the logger will have the URL including username and password in its file_remoteDirectory header

Expected behavior
I'd expect to not see my username and password of the SMB share in any logging, even if that logging originated from a mistake I made. This was already covered by the JCIFs library in the toString() call on the SMBFile#2071

I'm not sure whether this is an issue with how the list command creates the SMBFile objects, or if it's something else. I tried to reproduce the same for SFTP and FTP but couldn't reproduce the same case there, in those cases the errors and logs will not log sensitive information about my connection.

@smitsjelle smitsjelle added status: waiting-for-triage The issue need to be evaluated and its future decided type: bug labels Aug 19, 2024
@artembilan
Copy link
Member

Thank you for the report.
We can fix an SmbSession similar way as it is with an SftpSession: no logging at all.
There is no way to be sure where that path argument for session methods comes from.
However I agree that file_remoteDirectory must not contain that sensitive data.
Will investigate...

@artembilan
Copy link
Member

Are you sure that command="ls" on that gateway places the whole URL into that file_remoteDirectory?
I don't see how that may happen:

	private Object doLs(Message<?> requestMessage) {
		String dir = obtainRemoteDir(requestMessage);
		return this.remoteFileTemplate.execute(session -> {
			List<?> payload = ls(requestMessage, session, dir);
			return getMessageBuilderFactory()
					.withPayload(payload)
					.setHeader(FileHeaders.REMOTE_DIRECTORY, dir)
					.setHeader(FileHeaders.REMOTE_HOST_PORT, session.getHostPort());
		});

	}

	private String obtainRemoteDir(Message<?> requestMessage) {
		String dir = this.fileNameProcessor != null
				? this.fileNameProcessor.processMessage(requestMessage)
				: null;
		if (dir != null && !dir.endsWith(this.remoteFileTemplate.getRemoteFileSeparator())) {
			dir += this.remoteFileTemplate.getRemoteFileSeparator();
		}
		return dir;
	}

@artembilan
Copy link
Member

expression="payload.getFileInfo().getPath()"/> <!-- Note that getPath gives the entire SMB URL in this case -->

You did that yourself, so that expected that you may see sensitive data somewhere in logs.
See if toString() instead helps you.
However I don't think that SmbSession.remove() would work with that obfuscated URL.
Perhaps payload.fileInfo.url.path would be better to satisfy contract expectations...

@smitsjelle
Copy link
Author

I am a bit conflicted on this. I also mentioned that this is indeed due to a mistake that I made, yet even in this case I would not expect to get credentials into my logging. The reason I decided to open this issue was primarily that the behavior seems to be different for the other file operations; I tried with FTP and SFTP but I do not get similar logging that exposes my credentials to the logging.

I do agree with you that this will not happen during normal, properly configured, situations and have only observed operations being called on the SMBFile that mask the credentials. If due to the difference in nature between (S)FTP and SMB there is no way to align SMB to also have no way of accidentially exposing something in your logging, feel free to close this item.

@artembilan
Copy link
Member

Probably that's because of SmbFile extends URLConnection. Where the last one expose that full URL info.
The FtpSession also comes with a logging, but we don't see credentials in the logs because an FTPFile does not expose the whole url.
The SftpSession comes fully without any logging, although its SftpClient.DirEntry does not expose any credentials.

So, in general that's really the problem of the jcifs.smb.SmbFile where it exposes the whole connection info in its getPath().
However that still might be OK, because that's how a URL works:

    /**
     * {@inheritDoc}
     *
     * @see jcifs.SmbResourceLocator#getPath()
     */

    @Override
    public String getPath () {
        return this.url.toString();
    }

So, apparently whenever you log the URL object, you may see those credentials and that's now does not matter if you deal with SMB or Spring Integration at all.

Do I understand that right?
Does it really matter if we do something with logging in the SmbSession or not?

@artembilan artembilan added status: waiting-for-reporter Needs a feedback from the reporter and removed status: waiting-for-triage The issue need to be evaluated and its future decided labels Aug 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: waiting-for-reporter Needs a feedback from the reporter type: bug
Projects
None yet
Development

No branches or pull requests

2 participants