Status

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: edewata, Assigned: edewata)

Tracking

4.4.1

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

2 years ago
User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Firefox/52.0
Build ID: 20170302120751

Steps to reproduce:

JSS should provide a callback mechanism which allows application to be notified on SSL alerts (e.g. to generate logs). This depends on SSL alert callback mechanism in NSS (see bug #956866).
(Assignee)

Updated

2 years ago
Depends on: 956866

Updated

2 years ago
Assignee: glenbeasley → edewata
Target Milestone: --- → 4.4.1
(Assignee)

Comment 1

2 years ago
Created attachment 8849622 [details] [diff] [review]
0001-Added-SSLSocketListener.patch

The SSLSocket has been modified to support SSLSocketListener
which will be invoked when an SSL alert has been sent or
received, also when an SSL handshake has been completed.
Attachment #8849622 - Flags: review?(emaldona)

Comment 2

2 years ago
Comment on attachment 8849622 [details] [diff] [review]
0001-Added-SSLSocketListener.patch

Review of attachment 8849622 [details] [diff] [review]:
-----------------------------------------------------------------

Partial review until next version.

::: org/mozilla/jss/ssl/callbacks.c
@@ +278,5 @@
> +
> +    jint rc;
> +    JNIEnv *env;
> +    jclass socketClass, eventClass;
> +    jmethodID eventConstructor, eventSetCode, eventSetLevel, eventSetDescription;

callbacks.c:282:33: warning: unused variable ‘eventSetCode’ [-Wunused-variable]
As we discussed remove eventCode in the next version of the patch

@@ +338,5 @@
> +
> +    jint rc;
> +    JNIEnv *env;
> +    jclass socketClass, eventClass;
> +    jmethodID eventConstructor, eventSetCode, eventSetLevel, eventSetDescription;

callbacks.c:342:33: warning: unused variable ‘eventSetCode’ [-Wunused-variable]
As we discussed remove 'eventSetcode' in the next version of this patch
(Assignee)

Comment 3

2 years ago
Created attachment 8849648 [details] [diff] [review]
0001-Added-SSLCipher-enumeration.patch

The new patch removes warnings caused by unused variable and also replaces SSLAlertLevel and SSLAlertDescription classes with enumerations.
Attachment #8849622 - Attachment is obsolete: true
Attachment #8849622 - Flags: review?(emaldona)
Attachment #8849648 - Flags: review?(emaldona)
(Assignee)

Comment 4

2 years ago
Created attachment 8849651 [details] [diff] [review]
0001-Added-SSLSocketListener.patch

Sorry, wrong patch.

The new patch removes warnings caused by unused variable and also replaces SSLAlertLevel and SSLAlertDescription classes with enumerations.
Attachment #8849648 - Attachment is obsolete: true
Attachment #8849648 - Flags: review?(emaldona)
Attachment #8849651 - Flags: review?(emaldona)

Comment 5

2 years ago
Comment on attachment 8849651 [details] [diff] [review]
0001-Added-SSLSocketListener.patch

Review of attachment 8849651 [details] [diff] [review]:
-----------------------------------------------------------------

Please see the changes needed on account of the license. It build cleanly without extra warnings and the tests are passing.

::: org/mozilla/jss/ssl/SSLAlertDescription.java
@@ +31,5 @@
> + * and other provisions required by the GPL or the LGPL. If you do not delete
> + * the provisions above, a recipient may use your version of this file under
> + * the terms of any one of the MPL, the GPL or the LGPL.
> + *
> + * ***** END LICENSE BLOCK ***** */

Please change the License to be
/* This Source Code Form is subject to the terms of the Mozilla Public
 * License, v. 2.0. If a copy of the MPL was not distributed with this
 * file, You can obtain one at http://mozilla.org/MPL/2.0/. */

Otherwise it looks good.

::: org/mozilla/jss/ssl/SSLAlertEvent.java
@@ +32,5 @@
> + * the provisions above, a recipient may use your version of this file under
> + * the terms of any one of the MPL, the GPL or the LGPL.
> + *
> + * ***** END LICENSE BLOCK ***** */
> +

Please change the License to be
/* This Source Code Form is subject to the terms of the Mozilla Public
 * License, v. 2.0. If a copy of the MPL was not distributed with this
 * file, You can obtain one at http://mozilla.org/MPL/2.0/. */

Otherwise looks good to me.

::: org/mozilla/jss/ssl/SSLAlertLevel.java
@@ +32,5 @@
> + * the provisions above, a recipient may use your version of this file under
> + * the terms of any one of the MPL, the GPL or the LGPL.
> + *
> + * ***** END LICENSE BLOCK ***** */
> +

Please change the License to be
/* This Source Code Form is subject to the terms of the Mozilla Public
 * License, v. 2.0. If a copy of the MPL was not distributed with this
 * file, You can obtain one at http://mozilla.org/MPL/2.0/. */

Otherwise looks good.

::: org/mozilla/jss/ssl/SSLSocketListener.java
@@ +32,5 @@
> + * the provisions above, a recipient may use your version of this file under
> + * the terms of any one of the MPL, the GPL or the LGPL.
> + *
> + * ***** END LICENSE BLOCK ***** */
> +

Please change the License to be
/* This Source Code Form is subject to the terms of the Mozilla Public
 * License, v. 2.0. If a copy of the MPL was not distributed with this
 * file, You can obtain one at http://mozilla.org/MPL/2.0/. */

Otherwise looks good to me.

Comment 6

2 years ago
Comment on attachment 8849651 [details] [diff] [review]
0001-Added-SSLSocketListener.patch

Review of attachment 8849651 [details] [diff] [review]:
-----------------------------------------------------------------

r- but only due to the license issues mentioned.
(Assignee)

Comment 7

2 years ago
Created attachment 8849677 [details] [diff] [review]
0001-Added-SSLSocketListener.patch

This patch fixes the license notices in the above files.
Attachment #8849651 - Attachment is obsolete: true
Attachment #8849651 - Flags: review?(emaldona)
Attachment #8849677 - Flags: review?(emaldona)

Updated

2 years ago
Attachment #8849677 - Flags: review?(emaldona) → review+

Comment 8

2 years ago
Pushed: https://hg.mozilla.org/projects/jss/rev/e84cc2684fb04a2e5acf62b8e7bba87bfaef02ab
Status: UNCONFIRMED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.