diff --git a/src/it/java/com/hierynomus/smbj/IntegrationTest.java b/src/it/java/com/hierynomus/smbj/IntegrationTest.java index c49d019b..48599978 100644 --- a/src/it/java/com/hierynomus/smbj/IntegrationTest.java +++ b/src/it/java/com/hierynomus/smbj/IntegrationTest.java @@ -30,6 +30,7 @@ import org.testcontainers.junit.jupiter.Testcontainers; import com.hierynomus.msfscc.fileinformation.FileIdBothDirectoryInformation; +import com.hierynomus.mssmb2.SMB2Dialect; import com.hierynomus.smbj.share.DiskShare; import com.hierynomus.smbj.share.Share; import com.hierynomus.smbj.testcontainers.SambaContainer; @@ -89,6 +90,17 @@ public void shouldListContentsOfShareWithNullPath() throws Exception { }); } + @Test + public void shouldConnectAndListContentsOfShareWithDisabledSigning() throws Exception { + SmbConfig c = SmbConfig.builder().withDialects(SMB2Dialect.SMB_2_0_2).withMultiProtocolNegotiate(true).withSigningEnabled(false).withEncryptData(false).build(); + samba.withAuthenticatedClient(c, DEFAULT_AUTHENTICATION_CONTEXT, session -> { + try (DiskShare share = (DiskShare) session.connectShare("user")) { + List items = share.list(""); + assertThat(items).hasSize(2).map(FileIdBothDirectoryInformation::getFileName).contains(".", ".."); + } + }); + } + @ParameterizedTest(name = "should not fail closing connection twice") @MethodSource("com.hierynomus.smbj.testing.TestingUtils#defaultTestingConfig") public void shouldNotFailClosingConnectionTwice(SmbConfig c) throws Exception { diff --git a/src/it/java/com/hierynomus/smbj/testing/TestingUtils.java b/src/it/java/com/hierynomus/smbj/testing/TestingUtils.java index 2f047bde..0234af4e 100644 --- a/src/it/java/com/hierynomus/smbj/testing/TestingUtils.java +++ b/src/it/java/com/hierynomus/smbj/testing/TestingUtils.java @@ -66,6 +66,7 @@ public static Stream validConfigs() { public static Stream defaultTestingConfig() { return Stream.of(Arguments.of(config(SMB2Dialect.SMB_3_1_1, true, true))); } + public static Stream dfsConfig() { return Stream.of(Arguments.of(config(SMB2Dialect.SMB_2_1, false, true))); } diff --git a/src/main/java/com/hierynomus/smbj/SmbConfig.java b/src/main/java/com/hierynomus/smbj/SmbConfig.java index 2347ae29..ef52d9ae 100644 --- a/src/main/java/com/hierynomus/smbj/SmbConfig.java +++ b/src/main/java/com/hierynomus/smbj/SmbConfig.java @@ -21,6 +21,7 @@ import static com.hierynomus.mssmb2.SMB2Dialect.SMB_3_0_2; import static com.hierynomus.mssmb2.SMB2Dialect.SMB_3_1_1; +import java.lang.reflect.InvocationTargetException; import java.security.SecureRandom; import java.util.ArrayList; import java.util.Arrays; @@ -77,6 +78,7 @@ public final class SmbConfig { private Random random; private UUID clientGuid; private boolean signingRequired; + private boolean signingEnabled; private boolean dfsEnabled; private boolean useMultiProtocolNegotiate; private SecurityProvider securityProvider; @@ -103,6 +105,7 @@ public static Builder builder() { .withSecurityProvider(getDefaultSecurityProvider()) .withSocketFactory(new ProxySocketFactory()) .withSigningRequired(false) + .withSigningEnabled(true) .withDfsEnabled(false) .withMultiProtocolNegotiate(false) .withBufferSize(DEFAULT_BUFFER_SIZE) @@ -131,9 +134,9 @@ private static List> getDefaultAuthenticators() { if (!ANDROID) { try { - Object spnegoFactory = Class.forName("com.hierynomus.smbj.auth.SpnegoAuthenticator$Factory").newInstance(); + Object spnegoFactory = Class.forName("com.hierynomus.smbj.auth.SpnegoAuthenticator$Factory").getDeclaredConstructor().newInstance(); authenticators.add((Factory.Named)spnegoFactory); - } catch (InstantiationException | IllegalAccessException | ClassNotFoundException | ClassCastException e) { + } catch (InstantiationException | IllegalAccessException | ClassNotFoundException | ClassCastException | NoSuchMethodException | InvocationTargetException e) { throw new SMBRuntimeException(e); } } @@ -156,6 +159,7 @@ private SmbConfig(SmbConfig other) { random = other.random; clientGuid = other.clientGuid; signingRequired = other.signingRequired; + signingEnabled = other.signingEnabled; dfsEnabled = other.dfsEnabled; securityProvider = other.securityProvider; readBufferSize = other.readBufferSize; @@ -200,6 +204,16 @@ public boolean isSigningRequired() { return signingRequired; } + /** + * Whether the client should sign messages to the server. When message signing is enabled the client will sign messages to the server. + */ + public boolean isSigningEnabled() { + return signingEnabled; + } + + /** + * Whether the client should use the DFS protocol. + */ public boolean isDfsEnabled() { return dfsEnabled; } @@ -369,6 +383,11 @@ public Builder withSigningRequired(boolean signingRequired) { return this; } + public Builder withSigningEnabled(boolean signingEnabled) { + config.signingEnabled = signingEnabled; + return this; + } + public Builder withReadBufferSize(int readBufferSize) { if (readBufferSize <= 0) { throw new IllegalArgumentException("Read buffer size must be greater than zero"); @@ -453,6 +472,14 @@ public SmbConfig build() { throw new IllegalStateException("At least one SMB dialect should be specified"); } + if (config.signingRequired && !config.signingEnabled) { + throw new IllegalStateException("If signing is required, it should also be enabled"); + } + + if (!config.signingEnabled && SMB2Dialect.supportsSmb3x(config.dialects)) { + throw new IllegalStateException("Signing cannot be disabled when using SMB3.x dialects"); + } + if (config.encryptData && !SMB2Dialect.supportsSmb3x(config.dialects)) { throw new IllegalStateException("If encryption is enabled, at least one dialect should be SMB3.x compatible"); } diff --git a/src/main/java/com/hierynomus/smbj/connection/Connection.java b/src/main/java/com/hierynomus/smbj/connection/Connection.java index 60235f9a..f82bdeeb 100644 --- a/src/main/java/com/hierynomus/smbj/connection/Connection.java +++ b/src/main/java/com/hierynomus/smbj/connection/Connection.java @@ -86,7 +86,7 @@ public class Connection extends Pooled implements Closeable, PacketR private final SMBClient client; final ServerList serverList; - private PacketSignatory signatory; + private Signatory signatory; private PacketEncryptor encryptor; public SMBClient getClient() { @@ -110,7 +110,12 @@ public Connection(SmbConfig config, SMBClient client, SMBEventBus bus, ServerLis private void init() { bus.subscribe(this); this.sequenceWindow = new SequenceWindow(); - this.signatory = new PacketSignatory(config.getSecurityProvider()); + if (config.isSigningEnabled()) { + this.signatory = new PacketSignatory(config.getSecurityProvider()); + } else { + logger.warn("Signing is disabled for this connection."); + this.signatory = new NoSignatory(); + } this.encryptor = new PacketEncryptor(config.getSecurityProvider()); this.packetHandlerChain = new SMB3DecryptingPacketHandler(sessionTable, encryptor).setNext( @@ -139,7 +144,6 @@ public void connect(String hostname, int port) throws IOException { transport.connect(new InetSocketAddress(hostname, port)); this.connectionContext = new ConnectionContext(config.getClientGuid(), hostname, port, config); new SMBProtocolNegotiator(this, config, connectionContext).negotiateDialect(); - this.signatory.init(); this.encryptor.init(connectionContext); this.pathResolver = new SymlinkPathResolver(PathResolver.LOCAL); diff --git a/src/main/java/com/hierynomus/smbj/connection/NoSignatory.java b/src/main/java/com/hierynomus/smbj/connection/NoSignatory.java new file mode 100644 index 00000000..5d61e566 --- /dev/null +++ b/src/main/java/com/hierynomus/smbj/connection/NoSignatory.java @@ -0,0 +1,36 @@ +/* + * Copyright (C)2016 - SMBJ Contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.hierynomus.smbj.connection; + +import javax.crypto.SecretKey; + +import com.hierynomus.mssmb2.SMB2Packet; +import com.hierynomus.mssmb2.SMB2PacketData; + +/** + * When signing is disabled, this class is used to sign and verify packets so that the code does not need to take the lack of signing into account. + */ +public class NoSignatory implements Signatory { + @Override + public SMB2Packet sign(SMB2Packet packet, SecretKey secretKey) { + return packet; + } + + @Override + public boolean verify(SMB2PacketData packet, SecretKey secretKey) { + return true; + } +} diff --git a/src/main/java/com/hierynomus/smbj/connection/PacketSignatory.java b/src/main/java/com/hierynomus/smbj/connection/PacketSignatory.java index f95ac67a..72b35442 100644 --- a/src/main/java/com/hierynomus/smbj/connection/PacketSignatory.java +++ b/src/main/java/com/hierynomus/smbj/connection/PacketSignatory.java @@ -37,7 +37,7 @@ import com.hierynomus.security.SecurityProvider; import com.hierynomus.smb.SMBBuffer; -public class PacketSignatory { +public class PacketSignatory implements Signatory { private static final Logger logger = LoggerFactory.getLogger(PacketSignatory.class); private SecurityProvider securityProvider; @@ -46,9 +46,7 @@ public class PacketSignatory { this.securityProvider = securityProvider; } - void init() { - } - + @Override public SMB2Packet sign(SMB2Packet packet, SecretKey secretKey) { if (secretKey != null) { return new SignedPacketWrapper(packet, secretKey); @@ -59,6 +57,7 @@ public SMB2Packet sign(SMB2Packet packet, SecretKey secretKey) { } // TODO make session a packet handler which wraps the incoming packets + @Override public boolean verify(SMB2PacketData packet, SecretKey secretKey) { try { SMBBuffer buffer = packet.getDataBuffer(); diff --git a/src/main/java/com/hierynomus/smbj/connection/Signatory.java b/src/main/java/com/hierynomus/smbj/connection/Signatory.java new file mode 100644 index 00000000..4c8234fc --- /dev/null +++ b/src/main/java/com/hierynomus/smbj/connection/Signatory.java @@ -0,0 +1,27 @@ +/* + * Copyright (C)2016 - SMBJ Contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.hierynomus.smbj.connection; + +import javax.crypto.SecretKey; + +import com.hierynomus.mssmb2.SMB2Packet; +import com.hierynomus.mssmb2.SMB2PacketData; + +public interface Signatory { + public SMB2Packet sign(SMB2Packet packet, SecretKey secretKey); + + public boolean verify(SMB2PacketData packet, SecretKey secretKey); +} diff --git a/src/main/java/com/hierynomus/smbj/connection/packet/SMB2SignatureVerificationPacketHandler.java b/src/main/java/com/hierynomus/smbj/connection/packet/SMB2SignatureVerificationPacketHandler.java index 5ae74d1f..0a91e8d7 100644 --- a/src/main/java/com/hierynomus/smbj/connection/packet/SMB2SignatureVerificationPacketHandler.java +++ b/src/main/java/com/hierynomus/smbj/connection/packet/SMB2SignatureVerificationPacketHandler.java @@ -18,8 +18,8 @@ import com.hierynomus.mssmb2.DeadLetterPacketData; import com.hierynomus.mssmb2.SMB2PacketData; import com.hierynomus.protocol.transport.TransportException; -import com.hierynomus.smbj.connection.PacketSignatory; import com.hierynomus.smbj.connection.SessionTable; +import com.hierynomus.smbj.connection.Signatory; import com.hierynomus.smbj.session.Session; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -67,9 +67,9 @@ public class SMB2SignatureVerificationPacketHandler extends SMB2PacketHandler { private static final Logger logger = LoggerFactory.getLogger(SMB2SignatureVerificationPacketHandler.class); private SessionTable sessionTable; - private PacketSignatory signatory; + private Signatory signatory; - public SMB2SignatureVerificationPacketHandler(SessionTable sessionTable, PacketSignatory signatory) { + public SMB2SignatureVerificationPacketHandler(SessionTable sessionTable, Signatory signatory) { this.sessionTable = sessionTable; this.signatory = signatory; } diff --git a/src/main/java/com/hierynomus/smbj/session/Session.java b/src/main/java/com/hierynomus/smbj/session/Session.java index 4f451a90..2c4b8087 100644 --- a/src/main/java/com/hierynomus/smbj/session/Session.java +++ b/src/main/java/com/hierynomus/smbj/session/Session.java @@ -29,7 +29,7 @@ import com.hierynomus.smbj.common.SmbPath; import com.hierynomus.smbj.connection.Connection; import com.hierynomus.smbj.connection.PacketEncryptor; -import com.hierynomus.smbj.connection.PacketSignatory; +import com.hierynomus.smbj.connection.Signatory; import com.hierynomus.smbj.event.SMBEventBus; import com.hierynomus.smbj.event.SessionLoggedOff; import com.hierynomus.smbj.event.TreeDisconnected; @@ -61,7 +61,7 @@ public class Session implements AutoCloseable { private final SmbConfig config; private SMBEventBus bus; private final PathResolver pathResolver; - private PacketSignatory signatory; + private Signatory signatory; private PacketEncryptor encryptor; private TreeConnectTable treeConnectTable = new TreeConnectTable(); private Map nestedSessionsByHost = new HashMap<>(); @@ -69,7 +69,7 @@ public class Session implements AutoCloseable { private AuthenticationContext userCredentials; private SessionContext sessionContext; - public Session(Connection connection, SmbConfig config, AuthenticationContext userCredentials, SMBEventBus bus, PathResolver pathResolver, PacketSignatory signatory, PacketEncryptor encryptor) { + public Session(Connection connection, SmbConfig config, AuthenticationContext userCredentials, SMBEventBus bus, PathResolver pathResolver, Signatory signatory, PacketEncryptor encryptor) { this.connection = connection; this.config = config; this.userCredentials = userCredentials; diff --git a/src/test/java/com/hierynomus/smbj/SmbConfigTest.java b/src/test/java/com/hierynomus/smbj/SmbConfigTest.java new file mode 100644 index 00000000..0bd358a7 --- /dev/null +++ b/src/test/java/com/hierynomus/smbj/SmbConfigTest.java @@ -0,0 +1,42 @@ +/* + * Copyright (C)2016 - SMBJ Contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.hierynomus.smbj; + +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; + +import org.junit.jupiter.api.Test; + +import com.hierynomus.mssmb2.SMB2Dialect; + +public class SmbConfigTest { + @Test + public void testCreateDefaultConfig() { + assertDoesNotThrow(() -> SmbConfig.createDefaultConfig()); + } + + @Test + public void shouldNotBuildConfigWithRequiredAndDisabledSigning() { + assertThrows(IllegalStateException.class, + () -> SmbConfig.builder().withDialects(SMB2Dialect.SMB_2_0_2).withSigningRequired(true).withSigningEnabled(false).build()); + } + + @Test + public void shouldNotBuildConfigWithDisabledSigningAndSmb3xDialect() { + assertThrows(IllegalStateException.class, + () -> SmbConfig.builder().withDialects(SMB2Dialect.SMB_3_0).withSigningEnabled(false).build()); + } +} diff --git a/src/test/java/com/hierynomus/smbj/testing/Utils.java b/src/test/java/com/hierynomus/smbj/testing/Utils.java index 58fc96d1..013f8b5e 100644 --- a/src/test/java/com/hierynomus/smbj/testing/Utils.java +++ b/src/test/java/com/hierynomus/smbj/testing/Utils.java @@ -20,8 +20,12 @@ public class Utils { public static SmbConfig config(PacketProcessor processor) { + return configBuilder(processor).build(); + } + + public static SmbConfig.Builder configBuilder(PacketProcessor processor) { return SmbConfig.builder() .withTransportLayerFactory(new StubTransportLayerFactory<>(new DefaultPacketProcessor().wrap(processor))) - .withAuthenticators(new StubAuthenticator.Factory()).build(); + .withAuthenticators(new StubAuthenticator.Factory()); } }