Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ssh: add support for extension negotiation (rfc 8308) #211

Closed
wants to merge 8 commits into from
Closed

ssh: add support for extension negotiation (rfc 8308) #211

wants to merge 8 commits into from

Conversation

drakkan
Copy link
Member

@drakkan drakkan commented Mar 30, 2022

This is a rebase of the following PR

#197

with some changes and improvements:

  • added support for client certificate authentication
  • removed read loop from server handshake
  • adapted extInfoMsg to upstream changes

Signed-off-by: Nicola Murino [email protected]

@drakkan
ssh: add support for extension negotiation (rfc 8308)
7fe3443 
This is a rebase of the following PR

golang#197

with some changes and improvements:

- added support for client certificate authentication
- removed read loop from server handshake
- adapted extInfoMsg to upstream changes

Signed-off-by: Nicola Murino <[email protected]>
@gopherbot
Copy link
Contributor

This PR (HEAD: 7fe3443) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/crypto/+/396714 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@drakkan
remove the Extension setting
0196e38 
always add ext-info-s to KEX and send the SSH_MSG_EXT_INFO message
if we received ext-info-c from the client

Signed-off-by: Nicola Murino <[email protected]>
@gopherbot
Copy link
Contributor

This PR (HEAD: 0196e38) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/crypto/+/396714 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

This PR (HEAD: 2b41359) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/crypto/+/396714 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@drakkan
remove extInfoSent field
4929d4a 
we already know if this is the first key exchange

Signed-off-by: Nicola Murino <[email protected]>
@gopherbot
Copy link
Contributor

This PR (HEAD: 4929d4a) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/crypto/+/396714 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@drakkan
sendKexInit: evaluate first key exchange only once
76c9400 
we need it for both client and server

Signed-off-by: Nicola Murino <[email protected]>
@gopherbot
Copy link
Contributor

This PR (HEAD: 76c9400) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/crypto/+/396714 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@drakkan
improve code comments
5835f04 
Signed-off-by: Nicola Murino <[email protected]>
@gopherbot
Copy link
Contributor

This PR (HEAD: 5835f04) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/crypto/+/396714 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Nicola Murino:

Patch Set 6:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/396714.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Stan Hu:

Patch Set 6:

(2 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/396714.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Stan Hu:

Patch Set 6:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/396714.
After addressing review feedback, remember to publish your drafts!

@drakkan
added a shared method to parse extInfoMsgs
92ea34e 
both client and server side need to parse this message

Signed-off-by: Nicola Murino <[email protected]>
@gopherbot
Copy link
Contributor

This PR (HEAD: 92ea34e) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/crypto/+/396714 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Nicola Murino:

Patch Set 7:

(2 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/396714.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Stan Hu:

Patch Set 7:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/396714.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Stan Hu:

Patch Set 7:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/396714.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

This PR (HEAD: 8cff989) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/crypto/+/396714 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Nicola Murino:

Patch Set 7:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/396714.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Stan Hu:

Patch Set 8:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/396714.
After addressing review feedback, remember to publish your drafts!

@r0mant
Copy link

r0mant commented Aug 2, 2022

Hello, is there any ETA on when this PR will merge? Thanks.

@ianlancetaylor
Copy link
Member

@r0mant Please don’t reply on this GitHub thread. Visit golang.org/cl/396714. Thanks.

@russjones russjones mentioned this pull request Aug 3, 2022
Closed
@iQQBot iQQBot mentioned this pull request Aug 5, 2022
Closed
This was referenced Aug 22, 2022
Merged
Closed
@gopherbot
Copy link
Contributor

Message from Michał Trojnara:

Patch Set 8:

(3 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/396714.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Nicola Murino:

Patch Set 8:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/396714.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Michał Trojnara:

Patch Set 8:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/396714.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Nicola Murino:

Patch Set 8:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/396714.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Filippo Valsorda:

Patch Set 8:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/396714.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Nicola Murino:

Patch Set 8:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/396714.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Filippo Valsorda:

Patch Set 8:

(2 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/396714.
After addressing review feedback, remember to publish your drafts!

@FiloSottile FiloSottile mentioned this pull request Nov 3, 2022
Open
7 tasks
This was referenced Dec 30, 2022
Open
Open
@drakkan drakkan closed this Jan 23, 2023
@drakkan drakkan deleted the rfc8308 branch January 23, 2023 15:27
@reedloden
Copy link

@drakkan Did this get addressed upstream?

@brodock
Copy link

brodock commented Mar 1, 2023 •
edited
Loading

@reedloden doesn't seem like it was.

@stanhu
Copy link
Contributor

stanhu commented May 3, 2023

Actually, it did. https://go.dev/cl/447757 was merged in November 2022 and pared down this pull request: golang/go#49269 (comment).

I don't think we need to support ext-info-s either since this causes the client to send SSH_MSG_EXT_INFO, and this pull request just read and threw away the message.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@drakkan @gopherbot @r0mant @ianlancetaylor @reedloden @brodock @stanhu