Skip to content

RFC7250 (RPK) support (Fixes #6929) - #16620

Closed
tmshort wants to merge 1 commit into
openssl:masterfrom
akamai:master-rpk-new
Closed

RFC7250 (RPK) support (Fixes #6929)#16620
tmshort wants to merge 1 commit into
openssl:masterfrom
akamai:master-rpk-new

Conversation

@tmshort

@tmshort tmshort commented Sep 17, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor

Add support for the RFC7250 extensions, allows the use of only private keys for connection (i.e. certs not needed). Fixes #6929

Configurable with no-rpk
Add SSL_add1_expected_peer_rpk() to add a list of valid keys
Add SSL_get0_peer_rpk() to retrive peer's public key
Add RPK SSL options
Add unit tests
Add documentation
Add s_client/s_server support

Checklist
  • documentation is added or updated
  • tests are added or updated

@tmshort
tmshort force-pushed the master-rpk-new branch 3 times, most recently from 23f7f18 to a42e7b2 Compare September 17, 2021 15:57
@paulidale paulidale added approval: otc review pending branch: master Applies to master branch triaged: feature The issue/pr requests/adds a feature labels Sep 18, 2021
@paulidale

paulidale commented Sep 18, 2021

Copy link
Copy Markdown
Contributor

Adding the no-rpk option to the run checker builds is prudent. The CI one will check that it works here, I'm not sure where it should end up.

@tmshort

tmshort commented Sep 18, 2021

Copy link
Copy Markdown
Contributor Author

Adding the no-rpk option to the run checker builds is prudent. The CI one will check that it works here, I'm not sure where it should end up.

I can do that; also the no-tls builds cascade the no-rpk option.

FdaSilvaYY
FdaSilvaYY reviewed Sep 18, 2021
View reviewed changes
Comment thread include/openssl/tls1.h Outdated
Comment thread doc/man3/SSL_add1_expected_peer_rpk.pod Outdated
Comment thread doc/man3/SSL_add1_expected_peer_rpk.pod Outdated
@paulidale

paulidale commented Sep 20, 2021

Copy link
Copy Markdown
Contributor

Someone will want -no-rpk but not -no-tls, so we should test the former individually.

@tmshort

tmshort commented Sep 20, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@paulidale, added: Run-checker CI / run-checker (no-rpk) (pull_request)

@tmshort

tmshort commented Sep 20, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

No idea why run-checker (no-ui) is failing. Possibly unrelated. This does not modify code in that area and it had previously passed. A local no-ui build works...

@paulidale

paulidale commented Sep 21, 2021

Copy link
Copy Markdown
Contributor

It's a harmless failure.

@t8m t8m added this to the Post 3.0.0 milestone Sep 24, 2021
kaduk
kaduk reviewed Sep 24, 2021
View reviewed changes
Comment thread apps/s_server.c Outdated
kaduk
kaduk reviewed Sep 24, 2021
View reviewed changes
Comment thread apps/s_server.c Outdated
kaduk
kaduk reviewed Sep 27, 2021
View reviewed changes

@kaduk kaduk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the updates so far!

Comment thread test/build.info Outdated
@tmshort tmshort mentioned this pull request Oct 14, 2021
Closed
2 tasks
@tmshort

tmshort commented Nov 17, 2021

Copy link
Copy Markdown
Contributor Author

ping? OTC review?

@tmshort

tmshort commented Dec 9, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@mattcaswell @paulidale @t8m How does one get this out of the hold: substantial feature and approval: otc review pendiong state?

@paulidale

paulidale commented Dec 9, 2021

Copy link
Copy Markdown
Contributor

There will be a blog post about this. These things can, unfortunately, take a while.

@tmshort

tmshort commented Feb 11, 2022

Copy link
Copy Markdown
Contributor Author

There will be a blog post about this. These things can, unfortunately, take a while.

ping @paulidale ?

@openssl-machine

openssl-machine commented Mar 15, 2022

Copy link
Copy Markdown
Collaborator

This PR is in a state where it requires action by @openssl/omc but the last update was 31 days ago

FdaSilvaYY
FdaSilvaYY reviewed Mar 18, 2022
View reviewed changes

@FdaSilvaYY FdaSilvaYY left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just one more typo ;)

Comment thread doc/man3/SSL_add1_expected_peer_rpk.pod Outdated
FdaSilvaYY
FdaSilvaYY reviewed Mar 18, 2022
View reviewed changes

@FdaSilvaYY FdaSilvaYY left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There a few 2021 to replace by 2022.

Comment thread ssl/statem/statem_srvr.c Outdated
Comment thread util/libssl.num Outdated
Comment thread Configure Outdated

@FdaSilvaYY FdaSilvaYY Mar 18, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pre-condition is not checked inside s_client or s_server tools ?
No unwanted failure in case of forcing tls_1 with RPK ??

@tmshort tmshort Mar 21, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm torn here on this. We don't necessarily want to force a failure if, say, TLSv1 is negotiated. Sure RPK won't be used, but there won't be a failure (a certificate is still available). RPK is a negotiated option.

@tmshort
RFC7250 (RPK) support
c87bf2e 
Add support for the RFC7250 extensions, allows the use of only private
keys for connection (i.e. certs not needed).

Configurable with no-rpk
Add SSL_add1_expected_peer_rpk() to add a list of valid keys
Add SSL_get0_peer_rpk() to retrive peer's public key
Add RPK SSL options
Add unit tests
Add documentation
Add s_client/s_server support
@openssl-machine

openssl-machine commented Apr 18, 2022

Copy link
Copy Markdown
Collaborator

This PR is in a state where it requires action by @openssl/omc but the last update was 30 days ago

@tmshort tmshort mentioned this pull request Apr 26, 2022
Closed
2 tasks
@tmshort

tmshort commented Apr 26, 2022

Copy link
Copy Markdown
Contributor Author

Closing in favor of #18185

@tmshort tmshort closed this Apr 26, 2022
@mattcaswell mattcaswell mentioned this pull request Jun 27, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers
@kaduk kaduk kaduk left review comments
@FdaSilvaYY FdaSilvaYY FdaSilvaYY left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

branch: master Applies to master branch triaged: feature The issue/pr requests/adds a feature

Projects

None yet

Milestone

Post 3.0.0

Development

Successfully merging this pull request may close these issues.

Raw Public Key (RFC 7250) support

7 participants

@tmshort @paulidale @openssl-machine @kaduk @FdaSilvaYY @t8m @mattcaswell