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.

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

x/crypto/ssh: support encrypted private keys #4

Closed

Conversation

Unimarimos76
Copy link

Hi @tsurubee

Why

The sshr.crypto used in the sshr package does not support the keys generated by OpenSSH version 7.8.

Resolve

So I found the following issue on the original golang

golang/go#18692

This issue discusses support for private keys with passphrases.
So I ported the final merged version to sshr.crypto.
However, it did not work on its own, because it failed to call certs.go.
So I ported the original ssh/certs.go.

I tested it with the keys generated by OpenSSH version 7.8, and confirmed that it works.

How about improving sshr.crypto in this direction?

@tsurubee
Copy link
Owner

tsurubee commented Feb 1, 2021

@Unimarimos76
Thanks for your PR! Great job!
This PR is what you wrote as a future plan in section 7.1 of the paper below that you previously presented at IOT51?
http://id.nii.ac.jp/1001/00206563/
I thought the PR looked good, but let me confirm the following two points.

  1. Are the three files you changed (certs.go, bcrypt_pbkdf.go and keys.go) the same as the latest files in the ssh package below? (That is, you haven't made any changes to the file yourself at all?)
    https://github.com/golang/crypto/tree/master/ssh

  2. You said that it worked well with private keys generated by OpenSSH 7.8, but have you confirmed that it works well with keys generated by the versions earlier than OpenSSH 7.8?

I appreciate your contribution!
Please confirm the above points.

@Unimarimos76
Copy link
Author

Unimarimos76 commented Feb 18, 2021

Hi @tsurubee !
Thanks for the reply.
Let me answer your two points.

  1. The three modified files will be the same as the latest files of the latest ssh package. There is nothing I added.

  2. Confirmed that OpenSSH7.8 works before and after changing the key
    I was able to check it here, but I wanted tsurubee to check it too, so I forked sshr and added a container called fuga that
    uses openSSH7.8 for public key authentication.

https://github.com/Unimarimos76/sshr

To try the key for OpenSSH 7.8 or later, do the following

  1. docker-compose up -d
  2. ssh -i ./misc/testdata/client_keys/id_rsa_openssh [email protected] -p 2222

I'm sorry for the late reply, but I need a review.

@tsurubee
Copy link
Owner

tsurubee commented Mar 4, 2021

@Unimarimos76
Sorry for the delay in replying. I have checked the changes in the ssh package in detail.
I have a couple of questions, so I'll write them in Japanese.

取り込んでいただいたGoのSSHパッケージの変更を詳細に確認したところ,key.goのParseRawPrivateKey関数にあるcase文でパスフレーズ付きOpenSSH形式の鍵の場合の対応が入っていて,この変更を取り込むことは良いと思いました.
ただ, @Unimarimos76 さんの動作検証の方法がそれで良いのか疑問に思っています.
検証で, ./misc/testdata/client_keys/id_rsa_opensshで確認を行っていますが,今回の変更はクライアントの秘密鍵の形式の話でなく,sshrサーバが保持する秘密鍵(後続のSSHサーバへの接続に用いるもの)の読み込みの話ではないでしょうか?(そもそもsshrはクライアントの秘密鍵を読み込んだりしないので)

実際に私は変更前のsshr.cryptoを使ったsshrサーバでクライアントの秘密鍵をパスフレーズ付きのOpenSSH形式の鍵(-----BEGIN OPENSSH PRIVATE KEYで始まるもの)で問題なく動作することを確認しました.

もしかしたらそこの認識が大きくずれているかもしれないと思うので,

The sshr.crypto used in the sshr package does not support the keys generated by OpenSSH version 7.8.

ここで言っている対応していないということを再現する方法を教えていただけないでしょうか.どうことをやろうとして困っていたかを詳細に教えていただけると助かります.回答も日本語でかまいません!
不明点があれば質問していただければなるべく早く回答いたします.
確認よろしくお願いします.

@Unimarimos76
Copy link
Author

Unimarimos76 commented Mar 4, 2021

こんにちは @tsurubee さん
返信ありがとうございます.すぐに私もsshrの公式リポジトリで実際にできることを確認しました.
返信を見てもしかしたら私のsshrの利用方法がよくないのではないかと思いました.

今回sshrの改良に取り組んだリポジトリはこちらになります.
改良に取り組んでいた時FetchAuthorizedKeyHookを追加しました.

その時はクライアントの公開鍵をDBに格納しておりそこから公開鍵を取り出してくる形にしていました.
そのコードはこちらになります.
(DBのuser,passwordは変更してあり,プライベートなIPであるため問題ありません)

func FetchAuthorizedKeysMyHook(username, host string) ([]byte, error) {
	db, err := sqlx.Open("mysql", "hoge:hogehoge@tcp(10.0.0.58:3306)/sshr")
	if err != nil {
		log.Fatal(err)
	}

	var user User
	err =  db.Get(&user, "SELECT * FROM StudentInfo where StudentNum=? LIMIT 1", "ie-user")
	if err != nil {
		return []byte("0"), err
	} 
	log.Println(user);
	return []byte(user.KEY), nil
}

このようにしてFetchAuthorizedKeysMyHookを使いました.これを使用して公開鍵を用いたsshを行うと鍵認証ではなくパスワード認証になりました.ssh -vvv ie-sshr -i ~/.ssh/id_rsa とsshのデバッグメッセージを利用してでも鍵認証が利用されなかったため原因を探していたところgolangのsshパッケージのissueにたどり着きました.

これが原因だと考え今回の改善を行ないました.これが私が秘密鍵の読み込みをスルーしてしまうのではないかと至った流れとなります.
上のことを踏まえると私のFetchAuthorizedKeysHookの使い方が悪いように感じています.

行なったことや,公開できる情報を書きました.他にも足りないことがあると思いますがご教授していただけると幸いです.
よろしくお願いいたします.

追記

openSSH7.8以前で生成される鍵での検証も行いました.しかし,それでもsshの秘密鍵を使用しての接続を行うことができませんでした.
原因として考えられることがFetchAuthorizedKeysHookの私の使い方が悪いように思います.

@tsurubee
Copy link
Owner

tsurubee commented Mar 4, 2021

@Unimarimos76
詳細に状況を教えていただき,ありがとうございます!
おそらく状況から考えると,OpenSSH 7.8以降の鍵形式の問題ではなく,おっしゃる通りFetchAuthorizedKeysHookの問題と思われます.

まず,原因の切り分けとして,
https://github.com/tsurubee/sshr.crypto/blob/master/ssh/proxy.go#L53
ここの公開鍵のcase文の中のどこか途中でbreakreturnしてパスワード認証に切り替わっているはずなので,どこで case "publickey": を抜けているか特定すると良いと思います.
おそらく,
https://github.com/tsurubee/sshr.crypto/blob/master/ssh/proxy.go#L70
ここ以降のどこかの if err != nil に引っかかっていると思うので,そこの場所の特定とその時のエラーメッセージを教えていただけると解決できそうです!
それか,もしかしたら,どこもエラーではなく,DBから取り出したuser.KEYに改行文字などの不適切な文字列が混じっていて単に公開鍵が一致していないだけかもしれませんね.

この辺りの情報がわかりましたら,また一緒に考えましょう!お願いします!

@Unimarimos76
Copy link
Author

@tsurubee さん 返信ありがとうございます!

公開鍵が一致していない可能性もありそうですね.私も少しづつでバックしながら調査をしようと思います.
私も調査してわかったことがあり次第issueに書かせていただきます!

@tsurubee
Copy link
Owner

tsurubee commented Mar 4, 2021

はい!不明点あればいつでもお聞きください!

@tsurubee
Copy link
Owner

I'm going to close the PR once.
If necessary, you can make a new PR.
Thank you!

@tsurubee tsurubee closed this Nov 29, 2021
@Unimarimos76
Copy link
Author

I can't do PR, but I can't do it. Also, if we can improve it, we will create a PR again. Thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants