spring-projects / spring-integration

Spring Integration provides an extension of the Spring programming model to support the well-known Enterprise Integration Patterns (EIP)
http://projects.spring.io/spring-integration/
Apache License 2.0
1.54k stars 1.1k forks source link

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

Open smitsjelle opened 1 month ago

smitsjelle commented 1 month ago

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.

artembilan commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago
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 commented 4 weeks ago

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 commented 4 weeks ago

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?