1348856 - SSL alert callback

Status: RESOLVED FIXED

(JSS Graveyard :: Library, defect)

Description

[Endi S. Dewata]

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).

Comment 1

[Endi S. Dewata]

Attachment: 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.

Comment 2

[Elio Maldonado]

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

Comment 3

[Endi S. Dewata]

Attachment: 0001-Added-SSLCipher-enumeration.patch

The new patch removes warnings caused by unused variable and also replaces SSLAlertLevel and SSLAlertDescription classes with enumerations.

Comment 4

[Endi S. Dewata]

Attachment: 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.

Comment 5

[Elio Maldonado]

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

[Elio Maldonado]

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.

Comment 7

[Endi S. Dewata]

Attachment: 0001-Added-SSLSocketListener.patch

This patch fixes the license notices in the above files.

Comment 8

[Elio Maldonado]

Pushed: https://hg.mozilla.org/projects/jss/rev/e84cc2684fb04a2e5acf62b8e7bba87bfaef02ab