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

Add configuration to disable signing (Fixes #817) #848

Merged
merged 1 commit into from
Dec 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions src/it/java/com/hierynomus/smbj/IntegrationTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<FileIdBothDirectoryInformation> 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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ public static Stream<Arguments> validConfigs() {
public static Stream<Arguments> defaultTestingConfig() {
return Stream.of(Arguments.of(config(SMB2Dialect.SMB_3_1_1, true, true)));
}

public static Stream<Arguments> dfsConfig() {
return Stream.of(Arguments.of(config(SMB2Dialect.SMB_2_1, false, true)));
}
Expand Down
31 changes: 29 additions & 2 deletions src/main/java/com/hierynomus/smbj/SmbConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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)
Expand Down Expand Up @@ -131,9 +134,9 @@ private static List<Factory.Named<Authenticator>> 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<Authenticator>)spnegoFactory);
} catch (InstantiationException | IllegalAccessException | ClassNotFoundException | ClassCastException e) {
} catch (InstantiationException | IllegalAccessException | ClassNotFoundException | ClassCastException | NoSuchMethodException | InvocationTargetException e) {
throw new SMBRuntimeException(e);
}
}
Expand All @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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");
}
Expand Down
10 changes: 7 additions & 3 deletions src/main/java/com/hierynomus/smbj/connection/Connection.java
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ public class Connection extends Pooled<Connection> implements Closeable, PacketR
private final SMBClient client;
final ServerList serverList;

private PacketSignatory signatory;
private Signatory signatory;
private PacketEncryptor encryptor;

public SMBClient getClient() {
Expand All @@ -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(
Expand Down Expand Up @@ -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);
Expand Down
36 changes: 36 additions & 0 deletions src/main/java/com/hierynomus/smbj/connection/NoSignatory.java
Original file line number Diff line number Diff line change
@@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -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();
Expand Down
27 changes: 27 additions & 0 deletions src/main/java/com/hierynomus/smbj/connection/Signatory.java
Original file line number Diff line number Diff line change
@@ -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);
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down
6 changes: 3 additions & 3 deletions src/main/java/com/hierynomus/smbj/session/Session.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -61,15 +61,15 @@ 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<String, Session> nestedSessionsByHost = new HashMap<>();
private ReentrantReadWriteLock nestedSessionsRwLock = new ReentrantReadWriteLock();
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;
Expand Down
42 changes: 42 additions & 0 deletions src/test/java/com/hierynomus/smbj/SmbConfigTest.java
Original file line number Diff line number Diff line change
@@ -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());
}
}
6 changes: 5 additions & 1 deletion src/test/java/com/hierynomus/smbj/testing/Utils.java
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
}
Loading