Skip to content

Commit

Permalink
Make connections secure by default
Browse files Browse the repository at this point in the history
Every server configuration has its own setting that enables the use of
insecure connections. This is disabled by default. Only verified https
connections are allowed. Error messages with a note about the setting
have been added.

CVE-2018-1000664

Discussed in daneren2005#60
  • Loading branch information
flyingOwl authored and paroj committed Apr 1, 2024
1 parent a83e215 commit 29e6ba7
Show file tree
Hide file tree
Showing 8 changed files with 46 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -585,6 +585,12 @@ public void onClick(View v) {
serverAuthHeaderPreference.setSummary(R.string.settings_server_authheaders_summary);
serverAuthHeaderPreference.setTitle(R.string.settings_server_authheaders);

final CheckBoxPreference serverAllowInsecurePreference = new CheckBoxPreference(context);
serverAllowInsecurePreference.setKey(Constants.PREFERENCES_KEY_SERVER_ALLOW_INSECURE + instance);
serverAllowInsecurePreference.setChecked(Util.isAllowInsecureEnabled(context, instance));
serverAllowInsecurePreference.setSummary(R.string.settings_server_allowinsecure_summary);
serverAllowInsecurePreference.setTitle(R.string.settings_server_allowinsecure);

final Preference serverOpenBrowser = new Preference(context);
serverOpenBrowser.setKey(Constants.PREFERENCES_KEY_OPEN_BROWSER);
serverOpenBrowser.setPersistent(false);
Expand Down Expand Up @@ -660,6 +666,7 @@ public boolean onPreferenceClick(Preference preference) {
screen.addPreference(serverTagPreference);
screen.addPreference(serverSyncPreference);
screen.addPreference(serverAuthHeaderPreference);
screen.addPreference(serverAllowInsecurePreference);
screen.addPreference(serverTestConnectionPreference);
screen.addPreference(serverOpenBrowser);
screen.addPreference(serverRemoveServerPreference);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@
import javax.net.ssl.HostnameVerifier;
import javax.net.ssl.HttpsURLConnection;
import javax.net.ssl.SSLContext;
import javax.net.ssl.SSLException;
import javax.net.ssl.SSLSession;
import javax.net.ssl.SSLSocketFactory;
import javax.net.ssl.TrustManager;
Expand All @@ -108,7 +109,7 @@ public class RESTMusicService implements MusicService {
private static final int HTTP_REQUEST_MAX_ATTEMPTS = 5;
private static final long REDIRECTION_CHECK_INTERVAL_MILLIS = 60L * 60L * 1000L;

private SSLSocketFactory sslSocketFactory;
private SSLSocketFactory insecureSslSocketFactory;
private HostnameVerifier selfSignedHostnameVerifier;
private long redirectionLastChecked;
private int redirectionNetworkType = -1;
Expand All @@ -132,9 +133,9 @@ public void checkServerTrusted(
}
};
try {
SSLContext sslContext = SSLContext.getInstance("TLS");
sslContext.init(null, trustAllCerts, new java.security.SecureRandom());
sslSocketFactory = sslContext.getSocketFactory();
SSLContext insecureSslContext = SSLContext.getInstance("TLS");
insecureSslContext.init(null, trustAllCerts, new java.security.SecureRandom());
insecureSslSocketFactory = insecureSslContext.getSocketFactory();
} catch (Exception e) {
}

Expand Down Expand Up @@ -1884,6 +1885,10 @@ private HttpURLConnection getConnectionDirect(Context context, String url, Map<S
hasInstalledGoogleSSL = true;
}

if(!url.startsWith("https://") && !Util.isAllowInsecureEnabled(context, getInstance(context))) {
throw new SSLException("Only https connections are allowed!");
}

// Connect and add headers
URL urlObj = new URL(url);
HttpURLConnection connection = (HttpURLConnection) urlObj.openConnection();
Expand All @@ -1903,9 +1908,10 @@ private HttpURLConnection getConnectionDirect(Context context, String url, Map<S
}
}

if(connection instanceof HttpsURLConnection) {
if(Util.isAllowInsecureEnabled(context, getInstance(context)) && (connection instanceof HttpsURLConnection)) {
// if we allow insecure connections, disable ssl checks
HttpsURLConnection sslConnection = (HttpsURLConnection) connection;
sslConnection.setSSLSocketFactory(sslSocketFactory);
sslConnection.setSSLSocketFactory(insecureSslSocketFactory);
sslConnection.setHostnameVerifier(selfSignedHostnameVerifier);
}

Expand Down Expand Up @@ -2019,10 +2025,10 @@ public String getRestUrl(Context context, String method, boolean allowAltAddress
}
}

public SSLSocketFactory getSSLSocketFactory() {
return sslSocketFactory;
public SSLSocketFactory getInsecureSSLSocketFactory() {
return insecureSslSocketFactory;
}
public HostnameVerifier getHostNameVerifier() {
public HostnameVerifier getInsecureHostNameVerifier() {
return selfSignedHostnameVerifier;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,13 @@ public abstract class RemoteController {
protected boolean nextSupported = false;
protected ServerProxy proxy;
protected String rootLocation = "";
protected final boolean allowInsecure;

public RemoteController(DownloadService downloadService) {
this.downloadService = downloadService;
SharedPreferences prefs = Util.getPreferences(downloadService);
rootLocation = prefs.getString(Constants.PREFERENCES_KEY_CACHE_LOCATION, null);
allowInsecure = Util.isAllowInsecureEnabled(downloadService, Util.getActiveServer(downloadService));
}

public abstract void create(boolean playing, int seconds);
Expand Down Expand Up @@ -116,9 +118,10 @@ void clear() {

protected WebProxy createWebProxy() {
MusicService musicService = MusicServiceFactory.getMusicService(downloadService);
if(musicService instanceof CachedMusicService) {
// if we allow insecure connections, create WebProxy() with insecure ssl context
if(allowInsecure && (musicService instanceof CachedMusicService)) {
RESTMusicService restMusicService = ((CachedMusicService)musicService).getMusicService();
return new WebProxy(downloadService, restMusicService.getSSLSocketFactory(), restMusicService.getHostNameVerifier());
return new WebProxy(downloadService, restMusicService.getInsecureSSLSocketFactory(), restMusicService.getInsecureHostNameVerifier());
} else {
return new WebProxy(downloadService);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@
import android.os.Handler;
import android.os.Looper;
import android.util.Log;

import javax.net.ssl.SSLException;

import github.daneren2005.dsub.R;
import github.daneren2005.dsub.view.ErrorDialog;

Expand Down Expand Up @@ -131,6 +134,10 @@ protected String getErrorMessage(Throwable error) {
return context.getResources().getString(R.string.background_task_not_found);
}

if (error instanceof SSLException) {
return context.getResources().getString(R.string.background_task_network_insecure_error);
}

if (error instanceof IOException) {
return context.getResources().getString(R.string.background_task_network_error);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ public final class Constants {
public static final String PREFERENCES_KEY_PASSWORD = "password";
public static final String PREFERENCES_KEY_SERVER_AUTHHEADER = "authHeader";
public static final String PREFERENCES_KEY_ENCRYPTED_PASSWORD = "encryptedPassword";
public static final String PREFERENCES_KEY_SERVER_ALLOW_INSECURE = "allowInsecure";
public static final String PREFERENCES_KEY_INSTALL_TIME = "installTime";
public static final String PREFERENCES_KEY_THEME = "theme";
public static final String PREFERENCES_KEY_FULL_SCREEN = "fullScreen";
Expand Down
5 changes: 5 additions & 0 deletions app/src/main/java/github/daneren2005/dsub/util/Util.java
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,11 @@ public static boolean isAuthHeaderEnabled(Context context, int instance) {
return prefs.getBoolean(Constants.PREFERENCES_KEY_SERVER_AUTHHEADER + instance, true);
}

public static boolean isAllowInsecureEnabled(Context context, int instance) {
SharedPreferences prefs = getPreferences(context);
return prefs.getBoolean(Constants.PREFERENCES_KEY_SERVER_ALLOW_INSECURE + instance, false);
}

public static String getParentFromEntry(Context context, MusicDirectory.Entry entry) {
if(Util.isTagBrowsing(context)) {
if(!entry.isDirectory()) {
Expand Down
3 changes: 3 additions & 0 deletions app/src/main/res/values-de/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -649,5 +649,8 @@
<string name="menu.similar_artists.missing">Fehlende Künstler</string>
<string name="settings.casting_stream_original">Original streamen</string>
<string name="settings.casting_stream_original_summary">Zum streamen möglichst das Originalformat verwenden.</string>
<string name="settings.server_allowinsecure">Erlaube unsichere Verbindungen</string>
<string name="settings.server_allowinsecure_summary">Erlaube HTTP-Verbindungen und ignoriere Warnungen und Fehler bei HTTPS-Verbindungen (nicht empfohlen!)</string>
<string name="background_task.network_insecure_error">Die Verbindung zum Server ist unsicher. Unsichere Verbindungen sind in den Einstellungen nicht erlaubt worden.</string>

</resources>
3 changes: 3 additions & 0 deletions app/src/main/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -690,5 +690,8 @@
<item quantity="one">One day left of trial period</item>
<item quantity="other">%d days left of trial period</item>
</plurals>
<string name="settings.server_allowinsecure">Allow insecure connections</string>
<string name="settings.server_allowinsecure_summary">Allow http traffic and ignore warnings and errors with https connections (not recommended!)</string>
<string name="background_task.network_insecure_error">The connection to the server is insecure. Insecure connections have not been enabled in the settings.</string>

</resources>

0 comments on commit 29e6ba7

Please sign in to comment.