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 SSLHandshake Exception by using bypass #7657

Merged
merged 6 commits into from
Apr 22, 2021
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
- We added two new fields to track the creation and most recent modification date and time for each entry. [koppor#130](https://github.com/koppor/jabref/issues/130)
- We added a feature that allows the user to copy highlighted text in the preview window. [#6962](https://github.com/JabRef/jabref/issues/6962)
- We added a feature that allows you to create new BibEntry via paste arxivId [#2292](https://github.com/JabRef/jabref/issues/2292)
- We added a feature that allows the user to choose whether to trust the target site when unable to find a valid certification path from the file download site. [#7616](https://github.com/JabRef/jabref/issues/7616)

### Changed

Expand Down
29 changes: 29 additions & 0 deletions src/main/java/org/jabref/gui/fieldeditors/LinkedFileViewModel.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@
import java.util.Optional;
import java.util.function.BiPredicate;

import javax.net.ssl.HostnameVerifier;
import javax.net.ssl.HttpsURLConnection;
import javax.net.ssl.SSLSocketFactory;
import javax.xml.transform.TransformerException;

import javafx.beans.Observable;
Expand Down Expand Up @@ -425,6 +428,10 @@ public void download() {
}

URLDownload urlDownload = new URLDownload(linkedFile.getLink());
if (!checkSSLHandshake(urlDownload)) {
return;
}

BackgroundTask<Path> downloadTask = prepareDownloadTask(targetDirectory.get(), urlDownload);
downloadTask.onSuccess(destination -> {
LinkedFile newLinkedFile = LinkedFilesEditorViewModel.fromFile(destination, databaseContext.getFileDirectories(filePreferences), externalFileTypes);
Expand Down Expand Up @@ -463,7 +470,28 @@ public void download() {
}
}

public boolean checkSSLHandshake(URLDownload urlDownload) {
try {
urlDownload.canBeReached();
} catch (kong.unirest.UnirestException ex) {
if (ex.getCause() instanceof javax.net.ssl.SSLHandshakeException) {
if (dialogService.showConfirmationDialogAndWait(Localization.lang("Download file"),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need to ask the user, or should we just bypass ssl verification? For example, firefox downloads the linked pdf without any prompt.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that bypassing SSL verification should let users know.
We can add an option in the preferences to allow users to select all downloads to bypass the verification. The advantage of this feature that the user does not need to click on the trusted site every time. The disadvantage is that the code changes may be a bit big.

But I found that JabRef uses URLDownload.bypassSSLVerification() directly in some places, such as org.jabref.logic.importer.fetcher.IEEE#getURLForQuery, line 253.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's okay to show a dialog here. Should not happen that often.

Localization.lang("Unable to find valid certification path to requested target(%0), download anyway?",
urlDownload.getSource().toString()))) {
URLDownload.bypassSSLVerification();
return true;
} else {
dialogService.notify(Localization.lang("Download operation canceled."));
}
}
return false;
}
return true;
}

public BackgroundTask<Path> prepareDownloadTask(Path targetDirectory, URLDownload urlDownload) {
SSLSocketFactory defaultSSLSocketFactory = HttpsURLConnection.getDefaultSSLSocketFactory();
HostnameVerifier defaultHostnameVerifier = HttpsURLConnection.getDefaultHostnameVerifier();
BackgroundTask<Path> downloadTask = BackgroundTask
.wrap(() -> {
Optional<ExternalFileType> suggestedType = inferFileType(urlDownload);
Expand All @@ -476,6 +504,7 @@ public BackgroundTask<Path> prepareDownloadTask(Path targetDirectory, URLDownloa
return targetDirectory.resolve(fulltextDir).resolve(suggestedName);
})
.then(destination -> new FileDownloadTask(urlDownload.getSource(), destination))
.onFinished(() -> URLDownload.setSSLVerification(defaultSSLSocketFactory, defaultHostnameVerifier))
.onFailure(exception -> dialogService.showErrorDialogAndWait("Download failed", exception));
return downloadTask;
}
Expand Down
15 changes: 15 additions & 0 deletions src/main/java/org/jabref/logic/net/URLDownload.java
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import javax.net.ssl.HostnameVerifier;
import javax.net.ssl.HttpsURLConnection;
import javax.net.ssl.SSLContext;
import javax.net.ssl.SSLSocketFactory;
import javax.net.ssl.TrustManager;
import javax.net.ssl.X509TrustManager;

Expand Down Expand Up @@ -132,6 +133,20 @@ public X509Certificate[] getAcceptedIssuers() {
}
}

/**
*
* @param socketFactory trust manager
* @param verifier host verifier
*/
public static void setSSLVerification(SSLSocketFactory socketFactory, HostnameVerifier verifier) {
try {
HttpsURLConnection.setDefaultSSLSocketFactory(socketFactory);
HttpsURLConnection.setDefaultHostnameVerifier(verifier);
} catch (Exception e) {
LOGGER.error("A problem occurred when reset SSL verification", e);
}
}

public URL getSource() {
return source;
}
Expand Down
3 changes: 3 additions & 0 deletions src/main/resources/l10n/JabRef_en.properties
Original file line number Diff line number Diff line change
Expand Up @@ -2292,3 +2292,6 @@ Import\ settings=Import settings
Custom\ DOI\ URI=Custom DOI URI
Customization=Customization
Use\ custom\ DOI\ base\ URI\ for\ article\ access=Use custom DOI base URI for article access

Unable\ to\ find\ valid\ certification\ path\ to\ requested\ target(%0),\ download\ anyway?=Unable to find valid certification path to requested target(%0), download anyway?
Download\ operation\ canceled.=Download operation canceled.