Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(677)

Issue 265700044: Bug 1215295 - Support RSA-PSS for digital signing (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 years, 5 months ago by ttaubert
Modified:
8 years, 4 months ago
Reviewers:
ekr, mt, ekr-rietveld
Visibility:
Public.

Description

Bug 1215295 - Support RSA-PSS for digital signing

Patch Set 1 #

Total comments: 8

Patch Set 2 : Bug 1215295 - Support RSA-PSS for digital signing #

Patch Set 3 : Bug 1215295 - Support RSA-PSS for digital signing #

Unified diffs Side-by-side diffs Delta from patch set Stats (+554 lines, -80 lines) Patch
M external_tests/ssl_gtest/ssl_loopback_unittest.cc View 1 2 4 chunks +272 lines, -1 line 0 comments Download
M external_tests/ssl_gtest/tls_connect.h View 1 2 2 chunks +10 lines, -0 lines 0 comments Download
M external_tests/ssl_gtest/tls_connect.cc View 1 2 3 chunks +26 lines, -0 lines 0 comments Download
M external_tests/ssl_gtest/tls_parser.h View 1 2 1 chunk +11 lines, -0 lines 0 comments Download
M lib/nss/nss.def View 2 chunks +3 lines, -0 lines 0 comments Download
M lib/pk11wrap/pk11mech.c View 1 2 2 chunks +55 lines, -0 lines 0 comments Download
M lib/pk11wrap/pk11obj.c View 4 chunks +33 lines, -2 lines 0 comments Download
M lib/pk11wrap/pk11pub.h View 1 2 chunks +8 lines, -0 lines 0 comments Download
M lib/ssl/ssl3con.c View 1 2 27 chunks +129 lines, -71 lines 0 comments Download
M lib/ssl/ssl3ecc.c View 1 2 chunks +2 lines, -2 lines 0 comments Download
M lib/ssl/sslimpl.h View 1 2 1 chunk +3 lines, -3 lines 0 comments Download
M lib/ssl/sslt.h View 1 2 1 chunk +2 lines, -1 line 0 comments Download

Messages

Total messages: 2
mt
This looks mostly good. https://codereview.appspot.com/265700044/diff/1/lib/pk11wrap/pk11pub.h File lib/pk11wrap/pk11pub.h (right): https://codereview.appspot.com/265700044/diff/1/lib/pk11wrap/pk11pub.h#newcode715 lib/pk11wrap/pk11pub.h:715: const SECItem *hash); nit: check ...
8 years, 5 months ago (2015-10-16 17:54:28 UTC) #1
ttaubert
8 years, 5 months ago (2015-10-16 19:00:00 UTC) #2
Will update the patch shortly and then work on tests.

https://codereview.appspot.com/265700044/diff/1/lib/pk11wrap/pk11pub.h
File lib/pk11wrap/pk11pub.h (right):

https://codereview.appspot.com/265700044/diff/1/lib/pk11wrap/pk11pub.h#newcod...
lib/pk11wrap/pk11pub.h:715: const SECItem *hash);
On 2015/10/16 17:54:27, mt wrote:
> nit: check alignment

Done.

https://codereview.appspot.com/265700044/diff/1/lib/ssl/ssl3con.c
File lib/ssl/ssl3con.c (right):

https://codereview.appspot.com/265700044/diff/1/lib/ssl/ssl3con.c#newcode964
lib/ssl/ssl3con.c:964: PRBool isTLS, PRBool isTLS13)
On 2015/10/16 17:54:28, mt wrote:
> I think that I'd prefer to see a version number passed here instead.  You
could
> reduce two args to one and avoid all the guesswork.

Done.

https://codereview.appspot.com/265700044/diff/1/lib/ssl/ssl3con.c#newcode982
lib/ssl/ssl3con.c:982: if (isTLS13) {
On 2015/10/16 17:54:27, mt wrote:
> nit: Indent is screwy.

Done.

https://codereview.appspot.com/265700044/diff/1/lib/ssl/ssl3con.c#newcode1075
lib/ssl/ssl3con.c:1075: SECItem *buf, PRBool isTLS, PRBool isTLS13, void *pwArg)
On 2015/10/16 17:54:27, mt wrote:
> As above.

Done.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b