Status

JSS
Library
RESOLVED FIXED
8 months ago
8 months ago

People

(Reporter: Endi S. Dewata, Assigned: Endi S. Dewata)

Tracking

4.4.1

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

8 months 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

8 months ago
Depends on: 956866

Updated

8 months ago
Assignee: glenbeasley → edewata
Target Milestone: --- → 4.4.1
(Assignee)

Comment 1

8 months 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

8 months 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

8 months 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

8 months 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

8 months 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

8 months 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

8 months 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

8 months ago
Attachment #8849677 - Flags: review?(emaldona) → review+

Comment 8

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