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

Fix insecure connections #1160

Open
wants to merge 2 commits into
base: edge
Choose a base branch
from
Open
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
Original file line number Diff line number Diff line change
Expand Up @@ -739,21 +739,21 @@ private void loadSettings() {
editor.putBoolean(Constants.PREFERENCES_KEY_OFFLINE, false);

editor.putString(Constants.PREFERENCES_KEY_SERVER_NAME + 1, "Demo Server");
editor.putString(Constants.PREFERENCES_KEY_SERVER_URL + 1, "http://demo.subsonic.org");
editor.putString(Constants.PREFERENCES_KEY_USERNAME + 1, "guest");
editor.putString(Constants.PREFERENCES_KEY_SERVER_URL + 1, "https://demo.navidrome.org");
editor.putString(Constants.PREFERENCES_KEY_USERNAME + 1, "demo");
if (Build.VERSION.SDK_INT < 23) {
editor.putString(Constants.PREFERENCES_KEY_PASSWORD + 1, "guest");
editor.putString(Constants.PREFERENCES_KEY_PASSWORD + 1, "demo");
} else {
// Attempt to encrypt password
String encryptedDefaultPassword = KeyStoreUtil.encrypt("guest");
String encryptedDefaultPassword = KeyStoreUtil.encrypt("demo");

if (encryptedDefaultPassword != null) {
// If encryption succeeds, store encrypted password and flag password as encrypted
editor.putString(Constants.PREFERENCES_KEY_PASSWORD + 1, encryptedDefaultPassword);
editor.putBoolean(Constants.PREFERENCES_KEY_ENCRYPTED_PASSWORD + 1, true);
} else {
// Fall back to plaintext if Keystore is having issue
editor = editor.putString(Constants.PREFERENCES_KEY_PASSWORD + 1, "guest");
editor = editor.putString(Constants.PREFERENCES_KEY_PASSWORD + 1, "demo");
editor.putBoolean(Constants.PREFERENCES_KEY_ENCRYPTED_PASSWORD + 1, false);
}
}
Expand Down
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 @@ -470,6 +470,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>