You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Attila Bukor (Code Review)" <ge...@cloudera.org> on 2021/03/09 14:47:25 UTC

[kudu-CR] KUDU-3207 Switch RSA private key format to PKCS#8

Hello Alexey Serbin, Andrew Wong, Grant Henke,

I'd like you to do a code review. Please visit

    http://gerrit.cloudera.org:8080/17163

to review the following change.


Change subject: KUDU-3207 Switch RSA private key format to PKCS#8
......................................................................

KUDU-3207 Switch RSA private key format to PKCS#8

When Kudu writes a private key in PEM format, it uses RSAPrivateKey
family, which doesn't specify the output format, meaning it's up to
OpenSSL to decide the format it uses. There are several private keys in
the tests stored in PKCS#1 format which are string matched against. If
OpenSSL uses PKCS#8, such as in case of the FIPS-approved environment I
used, these assertions fail.

This commit changes the private key format to PKCS#8, re-enables the
test that was skipped in FIPS-approved mode as a workaround, and changes
all test private keys that were stored in PKCS#1 to PKCS#8. The actual
keys weren't changed, only the storage format.

I ran all tests manually in a non-FIPS environment (CentOS 7.9.2009 with
OpenSSL 1.0.2k-fips, fips mode off) and a FIPS environment (CentOS
7.8.2003 with OpenSSL 1.0.2v-fips, fips mode on).

Change-Id: Ie46fd4f0b8bafcbe606a444e31c9af9e09291e64
---
M src/kudu/security/crypto-test.cc
M src/kudu/security/crypto.cc
M src/kudu/security/test/test_certs.cc
3 files changed, 189 insertions(+), 191 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/63/17163/1
-- 
To view, visit http://gerrit.cloudera.org:8080/17163
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie46fd4f0b8bafcbe606a444e31c9af9e09291e64
Gerrit-Change-Number: 17163
Gerrit-PatchSet: 1
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>

[kudu-CR] KUDU-3207 Switch RSA private key format to PKCS#8

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17163 )

Change subject: KUDU-3207 Switch RSA private key format to PKCS#8
......................................................................


Patch Set 3: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie46fd4f0b8bafcbe606a444e31c9af9e09291e64
Gerrit-Change-Number: 17163
Gerrit-PatchSet: 3
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 09 Mar 2021 20:02:15 +0000
Gerrit-HasComments: No

[kudu-CR] KUDU-3207 Switch RSA private key format to PKCS#8

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/17163 )

Change subject: KUDU-3207 Switch RSA private key format to PKCS#8
......................................................................


Patch Set 1: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie46fd4f0b8bafcbe606a444e31c9af9e09291e64
Gerrit-Change-Number: 17163
Gerrit-PatchSet: 1
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 09 Mar 2021 15:02:22 +0000
Gerrit-HasComments: No

[kudu-CR] KUDU-3207 Switch RSA private key format to PKCS#8

Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/17163 )

Change subject: KUDU-3207 Switch RSA private key format to PKCS#8
......................................................................

KUDU-3207 Switch RSA private key format to PKCS#8

When Kudu writes a private key in PEM format, it uses RSAPrivateKey
family, which doesn't specify the output format, meaning it's up to
OpenSSL to decide the format it uses. There are several private keys in
the tests stored in PKCS#1 format which are string matched against. If
OpenSSL uses PKCS#8, such as in case of the FIPS-approved environment I
used, these assertions fail.

This commit changes the private key format to PKCS#8, re-enables the
test that was skipped in FIPS-approved mode as a workaround, and changes
all test private keys that were stored in PKCS#1 to PKCS#8. The actual
keys weren't changed, only the storage format.

I ran all tests manually in a non-FIPS environment (CentOS 7.9.2009 with
OpenSSL 1.0.2k-fips, fips mode off) and a FIPS environment (CentOS
7.8.2003 with OpenSSL 1.0.2v-fips, fips mode on).

Change-Id: Ie46fd4f0b8bafcbe606a444e31c9af9e09291e64
Reviewed-on: http://gerrit.cloudera.org:8080/17163
Reviewed-by: Alexey Serbin <as...@cloudera.com>
Reviewed-by: Grant Henke <gr...@apache.org>
Tested-by: Attila Bukor <ab...@apache.org>
---
M src/kudu/security/crypto-test.cc
M src/kudu/security/crypto.cc
M src/kudu/security/test/test_certs.cc
3 files changed, 196 insertions(+), 202 deletions(-)

Approvals:
  Alexey Serbin: Looks good to me, approved
  Grant Henke: Looks good to me, approved
  Attila Bukor: Verified

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie46fd4f0b8bafcbe606a444e31c9af9e09291e64
Gerrit-Change-Number: 17163
Gerrit-PatchSet: 4
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] KUDU-3207 Switch RSA private key format to PKCS#8

Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong, Grant Henke, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/17163

to look at the new patch set (#3).

Change subject: KUDU-3207 Switch RSA private key format to PKCS#8
......................................................................

KUDU-3207 Switch RSA private key format to PKCS#8

When Kudu writes a private key in PEM format, it uses RSAPrivateKey
family, which doesn't specify the output format, meaning it's up to
OpenSSL to decide the format it uses. There are several private keys in
the tests stored in PKCS#1 format which are string matched against. If
OpenSSL uses PKCS#8, such as in case of the FIPS-approved environment I
used, these assertions fail.

This commit changes the private key format to PKCS#8, re-enables the
test that was skipped in FIPS-approved mode as a workaround, and changes
all test private keys that were stored in PKCS#1 to PKCS#8. The actual
keys weren't changed, only the storage format.

I ran all tests manually in a non-FIPS environment (CentOS 7.9.2009 with
OpenSSL 1.0.2k-fips, fips mode off) and a FIPS environment (CentOS
7.8.2003 with OpenSSL 1.0.2v-fips, fips mode on).

Change-Id: Ie46fd4f0b8bafcbe606a444e31c9af9e09291e64
---
M src/kudu/security/crypto-test.cc
M src/kudu/security/crypto.cc
M src/kudu/security/test/test_certs.cc
3 files changed, 196 insertions(+), 202 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/63/17163/3
-- 
To view, visit http://gerrit.cloudera.org:8080/17163
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie46fd4f0b8bafcbe606a444e31c9af9e09291e64
Gerrit-Change-Number: 17163
Gerrit-PatchSet: 3
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] KUDU-3207 Switch RSA private key format to PKCS#8

Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong, Grant Henke, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/17163

to look at the new patch set (#2).

Change subject: KUDU-3207 Switch RSA private key format to PKCS#8
......................................................................

KUDU-3207 Switch RSA private key format to PKCS#8

When Kudu writes a private key in PEM format, it uses RSAPrivateKey
family, which doesn't specify the output format, meaning it's up to
OpenSSL to decide the format it uses. There are several private keys in
the tests stored in PKCS#1 format which are string matched against. If
OpenSSL uses PKCS#8, such as in case of the FIPS-approved environment I
used, these assertions fail.

This commit changes the private key format to PKCS#8, re-enables the
test that was skipped in FIPS-approved mode as a workaround, and changes
all test private keys that were stored in PKCS#1 to PKCS#8. The actual
keys weren't changed, only the storage format.

I ran all tests manually in a non-FIPS environment (CentOS 7.9.2009 with
OpenSSL 1.0.2k-fips, fips mode off) and a FIPS environment (CentOS
7.8.2003 with OpenSSL 1.0.2v-fips, fips mode on).

Change-Id: Ie46fd4f0b8bafcbe606a444e31c9af9e09291e64
---
M src/kudu/security/crypto-test.cc
M src/kudu/security/crypto.cc
M src/kudu/security/test/test_certs.cc
3 files changed, 189 insertions(+), 195 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/63/17163/2
-- 
To view, visit http://gerrit.cloudera.org:8080/17163
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie46fd4f0b8bafcbe606a444e31c9af9e09291e64
Gerrit-Change-Number: 17163
Gerrit-PatchSet: 2
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] KUDU-3207 Switch RSA private key format to PKCS#8

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/17163 )

Change subject: KUDU-3207 Switch RSA private key format to PKCS#8
......................................................................


Patch Set 3: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie46fd4f0b8bafcbe606a444e31c9af9e09291e64
Gerrit-Change-Number: 17163
Gerrit-PatchSet: 3
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 09 Mar 2021 20:02:45 +0000
Gerrit-HasComments: No

[kudu-CR] KUDU-3207 Switch RSA private key format to PKCS#8

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17163 )

Change subject: KUDU-3207 Switch RSA private key format to PKCS#8
......................................................................


Patch Set 2: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/17163/2/src/kudu/security/test/test_certs.cc
File src/kudu/security/test/test_certs.cc:

PS2: 
If anything changed w.r.t. how to get these keys with their current format?  If yes, could you add corresponding blurb in the comment for kCaCert or around?  That might help when adding new keys in future.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie46fd4f0b8bafcbe606a444e31c9af9e09291e64
Gerrit-Change-Number: 17163
Gerrit-PatchSet: 2
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 09 Mar 2021 16:36:19 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3207 Switch RSA private key format to PKCS#8

Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/17163 )

Change subject: KUDU-3207 Switch RSA private key format to PKCS#8
......................................................................


Patch Set 3: Verified+1

unrelated flaky test failure


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie46fd4f0b8bafcbe606a444e31c9af9e09291e64
Gerrit-Change-Number: 17163
Gerrit-PatchSet: 3
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 09 Mar 2021 21:27:05 +0000
Gerrit-HasComments: No

[kudu-CR] KUDU-3207 Switch RSA private key format to PKCS#8

Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/17163 )

Change subject: KUDU-3207 Switch RSA private key format to PKCS#8
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/17163/2/src/kudu/security/test/test_certs.cc
File src/kudu/security/test/test_certs.cc:

PS2: 
> If anything changed w.r.t. how to get these keys with their current format?
Good point, thank you. Updated.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie46fd4f0b8bafcbe606a444e31c9af9e09291e64
Gerrit-Change-Number: 17163
Gerrit-PatchSet: 3
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 09 Mar 2021 19:52:55 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3207 Switch RSA private key format to PKCS#8

Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has removed a vote on this change.

Change subject: KUDU-3207 Switch RSA private key format to PKCS#8
......................................................................


Removed Verified-1 by Kudu Jenkins (120)
-- 
To view, visit http://gerrit.cloudera.org:8080/17163
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Ie46fd4f0b8bafcbe606a444e31c9af9e09291e64
Gerrit-Change-Number: 17163
Gerrit-PatchSet: 3
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)