Joe McDonnell has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/22910 )

Change subject: PROTOTYPE: Support certificates signed with RSASSA-PSS for 
channel bindings
......................................................................


Patch Set 2:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/22910/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/22910/2//COMMIT_MSG@7
PS2, Line 7: PROTOTYPE
> I guess Joe was going to file a Kudu upstream JIRA for this as he mentioned
Filed KUDU-3663 and updated this message


http://gerrit.cloudera.org:8080/#/c/22910/2/src/kudu/security/cert.h
File src/kudu/security/cert.h:

http://gerrit.cloudera.org:8080/#/c/22910/2/src/kudu/security/cert.h@79
PS2, Line 79: WARN_UNUSED_RESULT
> nit: in Kudu this is no longer needed in 1.18 and newer codebase since the
If you don't mind, I might leave this here to make it easier to backport.


http://gerrit.cloudera.org:8080/#/c/22910/2/src/kudu/security/cert.cc
File src/kudu/security/cert.cc:

http://gerrit.cloudera.org:8080/#/c/22910/2/src/kudu/security/cert.cc@28
PS2, Line 28: #include <cstddef>
> I wonder what triggered this. Maybe NULL? Let's see what happens when I swi
Switching NULL to nullptr eliminates the need for this, so this is gone.


http://gerrit.cloudera.org:8080/#/c/22910/2/src/kudu/security/cert.cc@198
PS2, Line 198: NULL
> nit for here and below: consider changing to 'nullptr' to match the rest of
Done


http://gerrit.cloudera.org:8080/#/c/22910/2/src/kudu/security/cert.cc@200
PS2, Line 200:   OBJ_find_sigid_algs(signature_nid, &digest_nid, NULL);
> Yeah, I think it is possible to implement similar support for OpenSSL 1.0.2
Added a better error message for RSASSA-PSS for this case.


http://gerrit.cloudera.org:8080/#/c/22910/2/src/kudu/security/cert.cc@210
PS2, Line 210:   if (digest_nid == NID_undef) {
> Since we are making changed here, we should wrap this if condition with UNL
Added PREDICT_FALSE to this condition (Kudu calls this PREDICT_FALSE rather 
than UNLIKELY).


http://gerrit.cloudera.org:8080/#/c/22910/2/src/kudu/security/cert.cc@213
PS2, Line 213:         "server certificate using $0 has no signature digest 
(hash) algorithm",
> Nit: this error would be clearer if the string said "server certificate usi
Done



--
To view, visit http://gerrit.cloudera.org:8080/22910
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I26a25a43d778fd2f2fcf293ecb199133c675212b
Gerrit-Change-Number: 22910
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Jason Fehr <[email protected]>
Gerrit-Reviewer: Joe McDonnell <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 19 May 2025 19:52:59 +0000
Gerrit-HasComments: Yes

Reply via email to