Skip to content

Commit

Permalink
Replace deprecation warning (#140)
Browse files Browse the repository at this point in the history
  • Loading branch information
pablo-ruth authored Aug 26, 2022
1 parent d2a2c9c commit 0e30ff4
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 30 deletions.
26 changes: 18 additions & 8 deletions client/files.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,15 +78,15 @@ func CertStillValid(path string) bool {
}

// CertInfo extract principals and expiration from SSH certificate
func CertInfo(cert string) (principals []string, before uint64, algo string, err error) {
algo = strings.Split(cert, " ")[0]
func CertInfo(cert string) (principals []string, before uint64, keyType string, err error) {

parsedKey, _, _, _, err := ssh.ParseAuthorizedKey([]byte(cert))
if err != nil {
return principals, before, algo, err
return principals, before, keyType, err
}
parsedCert := parsedKey.(*ssh.Certificate)

return parsedCert.ValidPrincipals, parsedCert.ValidBefore, algo, nil
return parsedCert.ValidPrincipals, parsedCert.ValidBefore, parsedCert.Key.Type(), nil
}

// WriteUserSignedKey writes user certificate on disk.
Expand Down Expand Up @@ -174,14 +174,24 @@ func chooseSSHKeyType(key string) (string, bool) {
}
}

// CertAlgoIsDeprecated returns true if certificate algorithm is deprecated by openssh
func CertAlgoIsDeprecated(s string) bool {
// CertKeyTypeIsDeprecated returns true if certificate key type is deprecated by openssh
func CertKeyTypeIsDeprecated(s string) bool {
switch {
case s == ssh.CertAlgoRSAv01:
return true
case s == ssh.CertAlgoDSAv01:
return true
default:
return false
}
}

// CertKeyTypeIsBuggy returns true if certificate key type is buggy with
// some versions of openssh client/server combination, see discussion for
// more details : https://github.com/signmykeyio/signmykey/pull/138
func CertKeyTypeIsBuggy(s string) bool {
switch {
case s == ssh.KeyAlgoRSA:
return true
default:
return false
}
}
48 changes: 38 additions & 10 deletions client/files_test.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
package client

import (
"reflect"
"testing"

"github.com/stretchr/testify/assert"
"golang.org/x/crypto/ssh"
)

func TestChooseSSHKeyType(t *testing.T) {
Expand All @@ -28,21 +30,47 @@ func TestChooseSSHKeyType(t *testing.T) {
}
}

func TestCertKeyTypeIsBuggy(t *testing.T) {
cases := []struct {
keyType string
isBuggy bool
}{
{ssh.KeyAlgoRSA, true},
{ssh.KeyAlgoDSA, false},
{ssh.KeyAlgoED25519, false},
{ssh.KeyAlgoECDSA256, false},
}

func TestCertAlgoIsDeprecated(t *testing.T) {
for _, c := range cases {
isBuggy := CertKeyTypeIsBuggy(c.keyType)
assert.Equal(t, c.isBuggy, isBuggy, c.keyType)
}
}

func TestCertInfo(t *testing.T) {
cases := []struct {
algo string
isDeprecated bool
principals []string
before uint64
keyType string
cert string
}{
{"[email protected]", true},
{"[email protected]", true},
{"[email protected]", false},
{"[email protected]", false},
{"[email protected]", false},
{[]string{"test_principal"}, 18446744073709551615, ssh.KeyAlgoRSA, "[email protected] AAAAHHNzaC1yc2EtY2VydC12MDFAb3BlbnNzaC5jb20AAAAg7y+wbqFwFYKHvB8iBG3fSUiFDJwlwrgYK29SVcfXIl8AAAADAQABAAABgQDXyccdoqiuomunLwK/u7cddjovr38Svi5M7qq3SoUSk0Y5BakECmB2QlevqXht6o938kia3THDv7HGMoEIwhuzAR/2pEqobr7bTo6yfxuBjiGU2Tbu4Ma7/gDfWlMavFKyTlOiIeQQXn+I6VG4UW9ddLc76u3VhStclW6RjdMQ1RAeO4ln+/GYTkoa8SjLfw75gZe1tXDlMxQrPK7vvoqyY7a+NYomrnatozI7mKUCxl1Du1Rkp8o23JSSGNxRi2Q7iNHE/3I6XJ8/g67Yrniky0NEyXtP9hB5u74hnnr9eAOCC/d1LKiVvlwHC8Vc8/e3DVDE4tcEvI6njIi9w4YOZ6/9aXXEzyUunGPTh0M09paEu1dgb5bwVnPs0/50g9N4efs/s0QRVvW5osAxkdjoT9krIK/pgHEZRlRN+4T1Ogj4erl2ZMqb1VTmeCSL69Ck6upotbkTUpmBuIDXK1HGt83g/HEc7LC1OMPm3UoDGndYuRORu54qJg2UHaHz9IcAAAAAAAAAAAAAAAEAAAAHdGVzdF9pZAAAABIAAAAOdGVzdF9wcmluY2lwYWwAAAAAAAAAAP//////////AAAAAAAAAIIAAAAVcGVybWl0LVgxMS1mb3J3YXJkaW5nAAAAAAAAABdwZXJtaXQtYWdlbnQtZm9yd2FyZGluZwAAAAAAAAAWcGVybWl0LXBvcnQtZm9yd2FyZGluZwAAAAAAAAAKcGVybWl0LXB0eQAAAAAAAAAOcGVybWl0LXVzZXItcmMAAAAAAAAAAAAAAZcAAAAHc3NoLXJzYQAAAAMBAAEAAAGBALLjeJw63Jc5dNQFmH50B7zRGfYzRErgJeyDzJfPO3LKHcqpOt5TQhkkO8pl2KKZmfoUoZWr158B8hdh/5khexWSVRGacb4K+cdxbO1zA6LXtH40KD2wlfYHTrA3ze4NCJ4jKrfJE8uNjnukp78mYtsuwvmwJLsRcXHXxLpMmSRsdSkf37k0BJ+maHkrXzrHhpLfu+YcNW56MKUer8+Bg3GruUPQNlhtJlIvnMJ702k8DmHV4Gdcal+ixgCDPAoZMzEYuqILtsb/gOVQPtozM/kILyVTJlBA8y1/wIjgQkcpy7vOaIOrvNjEu1FKrz/+lJEOqC4YNQs+ad2NCfvJPs8S5Qe/J/bdukGLRITmZ9FeQOyvgdf7lp+UWNAN1oiO4406DoEBV8semFwQuGyz8Co4jy1MWIBRPNrRRcB3Lkl3lpPz3uj3UQRguqpEKXp2j8gcdyhF/Ci9x+k8WYf97U+Szxsj71ZnJ/buKShFo87S44KoIAUrq5tFxI1qbuFTxQAAAZQAAAAMcnNhLXNoYTItNTEyAAABgHLb4k831Pv9RmDcxgQfZ172GVXGYuWFJiAg2AiCWy4Va5Pp4IaNFFwEQ/KtLDLfb7z18vQ+cZN1Yd6eWjLfLeqoo1K4ma0hmofNTvZ2kp70bYSDBB5mSSFifigbPrn1b6B7vJhmKZbUITesW1pKwDYOPeLv71FbLf1Z+nkVyhHj4aGXEBiZbO+FvXmWiJYs1yJHio9c+SKBq2fr3g0LBhcYAqxroCWVbji7SfzyU96vEqve0nscWQ8HSFxVAXAObc5/YBnCBQrxHzIbrpD94Wr2y9r7SOLCnA2ttEw9IUf8Vgt/s122aoLU13lOpjJb0Or4meW2pxBOMbAH4NCbAfqkDRIPzZz8RDg8kw77g8q/Mbr7on+WyjOhJoCPeZgGwVC9JRm0sXNpiziJT7Nl3rIChNkAf6ysyRF0TOHpnEEsV5u5z3o4kftjHdBh3xDKSRlQ7R+UgtM/bjD+gy9ImwuqbO4ZE0SlpgJsXkUV2N3TA1vAxbhjFakYZqvJmZ786w=="},
{[]string{"test_principal"}, 18446744073709551615, ssh.KeyAlgoECDSA256, "[email protected] AAAAKGVjZHNhLXNoYTItbmlzdHAyNTYtY2VydC12MDFAb3BlbnNzaC5jb20AAAAgJdvGTH0BYisEPsXsuF0YzbiQEzTbu8IcS84/hhboUS0AAAAIbmlzdHAyNTYAAABBBNVjZJiuR4CNpii/d16CF3mst0pvNFVC3y5iIqhRo/p86JFF3nsKliSRYUvTA+kFGJbMr7uL41KH/qrmiQL7NsoAAAAAAAAAAAAAAAEAAAAHdGVzdF9pZAAAABIAAAAOdGVzdF9wcmluY2lwYWwAAAAAAAAAAP//////////AAAAAAAAAIIAAAAVcGVybWl0LVgxMS1mb3J3YXJkaW5nAAAAAAAAABdwZXJtaXQtYWdlbnQtZm9yd2FyZGluZwAAAAAAAAAWcGVybWl0LXBvcnQtZm9yd2FyZGluZwAAAAAAAAAKcGVybWl0LXB0eQAAAAAAAAAOcGVybWl0LXVzZXItcmMAAAAAAAAAAAAAAZcAAAAHc3NoLXJzYQAAAAMBAAEAAAGBALLjeJw63Jc5dNQFmH50B7zRGfYzRErgJeyDzJfPO3LKHcqpOt5TQhkkO8pl2KKZmfoUoZWr158B8hdh/5khexWSVRGacb4K+cdxbO1zA6LXtH40KD2wlfYHTrA3ze4NCJ4jKrfJE8uNjnukp78mYtsuwvmwJLsRcXHXxLpMmSRsdSkf37k0BJ+maHkrXzrHhpLfu+YcNW56MKUer8+Bg3GruUPQNlhtJlIvnMJ702k8DmHV4Gdcal+ixgCDPAoZMzEYuqILtsb/gOVQPtozM/kILyVTJlBA8y1/wIjgQkcpy7vOaIOrvNjEu1FKrz/+lJEOqC4YNQs+ad2NCfvJPs8S5Qe/J/bdukGLRITmZ9FeQOyvgdf7lp+UWNAN1oiO4406DoEBV8semFwQuGyz8Co4jy1MWIBRPNrRRcB3Lkl3lpPz3uj3UQRguqpEKXp2j8gcdyhF/Ci9x+k8WYf97U+Szxsj71ZnJ/buKShFo87S44KoIAUrq5tFxI1qbuFTxQAAAZQAAAAMcnNhLXNoYTItNTEyAAABgJsWz2/FgCf/mfnkw2Yn7BFFcH3ZAYsO89KYTJmCzRit0o5EcHjoFtPBf7jeiieJ19nnDR4jpC+5FwqNnXiI0xpSus4Rh3D84h78xxHIRhR3nBqfK0CCP4SXl7aZLIvOf32PNLvVXehSXMfq04hwzEqYOdHBf37O5uwdukFKX0FSEzGiEPa51LPEzs8wugwNzp/IKnO/DOI8D8i5cySxGMp8Rfai1dRquPd0NxBq4qIDaDJ9wz31YGOEEAxIdWQCvwGx0hUU7U6DD/8W2NMEtyVIPZPh7dzZOgXWpIuWQ08v/yqabXZEMqayjR3yQ9/cVnFix9SdjOo3xaP9O/O5lX6jWWHWkEhU80QnlJQRgcPIoYP4LXeNVvi8dDEFOLd6m+bO3/ysjm4z+GzQhxzOuCV7Xh8BeoB9EDG3LGgROIzRWp6NWk1GEOa4S8bu/coo/fQkXuVn61SgB2Km2RRlqB3R4CSNgX5/cawczvyGs4fBcAIYmILDyg4qbvbDSKLvPg=="},
{[]string{"test_principal"}, 18446744073709551615, ssh.KeyAlgoED25519, "[email protected] AAAAIHNzaC1lZDI1NTE5LWNlcnQtdjAxQG9wZW5zc2guY29tAAAAICenegcHqkkVK5eZApDvOKoqK6TSv4yAdjyGcFNreh6bAAAAIDqolxIZDb/1QnCQfVnlv4Uy4IMjfWMmUYIn6DXG1askAAAAAAAAAAAAAAABAAAAB3Rlc3RfaWQAAAASAAAADnRlc3RfcHJpbmNpcGFsAAAAAAAAAAD//////////wAAAAAAAACCAAAAFXBlcm1pdC1YMTEtZm9yd2FyZGluZwAAAAAAAAAXcGVybWl0LWFnZW50LWZvcndhcmRpbmcAAAAAAAAAFnBlcm1pdC1wb3J0LWZvcndhcmRpbmcAAAAAAAAACnBlcm1pdC1wdHkAAAAAAAAADnBlcm1pdC11c2VyLXJjAAAAAAAAAAAAAAGXAAAAB3NzaC1yc2EAAAADAQABAAABgQCy43icOtyXOXTUBZh+dAe80Rn2M0RK4CXsg8yXzztyyh3KqTreU0IZJDvKZdiimZn6FKGVq9efAfIXYf+ZIXsVklURmnG+CvnHcWztcwOi17R+NCg9sJX2B06wN83uDQieIyq3yRPLjY57pKe/JmLbLsL5sCS7EXFx18S6TJkkbHUpH9+5NASfpmh5K186x4aS37vmHDVuejClHq/PgYNxq7lD0DZYbSZSL5zCe9NpPA5h1eBnXGpfosYAgzwKGTMxGLqiC7bG/4DlUD7aMzP5CC8lUyZQQPMtf8CI4EJHKcu7zmiDq7zYxLtRSq8//pSRDqguGDULPmndjQn7yT7PEuUHvyf23bpBi0SE5mfRXkDsr4HX+5aflFjQDdaIjuONOg6BAVfLHphcELhss/AqOI8tTFiAUTza0UXAdy5Jd5aT897o91EEYLqqRCl6do/IHHcoRfwovcfpPFmH/e1Pks8bI+9WZyf27ikoRaPO0uOCqCAFK6ubRcSNam7hU8UAAAGUAAAADHJzYS1zaGEyLTUxMgAAAYAM8l4R33l3uJ2nRjm8y5e4b98mkJ6q2tta7N3PbrOB5jnMkYv7TKDqPBnawWkP2w6+LA1M/7XWvPmNro5zkUeeSs9UUrXS9OVKN69H/cM31jNLcXeaUOE0BwSC6jXWLIsPlcC8oNXAble7fGfdzCVqgno0jU/7E/I7IMfk4K0Fo8sFdNVoT0gazVKQkY1nb7WpSgdEtMFlns2wQ8mwZBbgVUE2w7AYz78CTdozajx4EXQffQjsrPbYXA3F9Icgc7CnDyH0O5nOb+YSYBnAXFSf7UCPIzBDA4n6jp+noGwHu2EGfpGGKD+KZx0KYStIGnXGYYF+1wWK2vo3xe8U1oQ9N/EX0dvt0Pwi6WsIIpemqAvjCWz/aQwedFD8K8hEK84OBzvUxv0cAm2aWalBb+PYJl8mYmkrsgWrcb/XX8VCBka3bgKK2lw8023zFe9NvdvSKx/a6/qXspxg/zMRwaUUMzre5hmG/uRB2KoDTjIrwlpYseOGUrTAFT0jH/97Ikc="},
}

for _, c := range cases {
isDeprecated := CertAlgoIsDeprecated(c.algo)
assert.Equal(t, c.isDeprecated, isDeprecated)
principals, before, keyType, _ := CertInfo(c.cert)
if !reflect.DeepEqual(principals, c.principals) {
t.Errorf("CertInfo principal: %v, want %v", principals, c.principals)
}

if before != c.before {
t.Errorf("CertInfo before: %v, want %v", before, c.before)
}

if keyType != c.keyType {
t.Errorf("CertInfo keyType: %v, want %v", keyType, c.keyType)
}
}
}
30 changes: 18 additions & 12 deletions cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ var rootCmd = &cobra.Command{
smkAddr = smkAddr + "/"
}

deprecatedAlgos := []string{}
var deprecatedKeys, buggyKeys, totalKeys int
for _, pubKeyFile := range pubKeysFiles {
pubKey, err := client.GetUserPubKey(pubKeyFile)
if err != nil {
Expand All @@ -96,26 +96,32 @@ var rootCmd = &cobra.Command{

color.Green("\nYour SSH Key %s is successfully signed !", pubKeyFile)

principals, before, algo, err := client.CertInfo(signedKey)
principals, before, keyType, err := client.CertInfo(signedKey)
if err != nil {
return fmt.Errorf("%v, public key: %v", err, pubKeyFile)
}
color.HiBlack("\n - Valid until: %s", time.Unix(int64(before), 0))
color.HiBlack(" - Principals: %s", strings.Join(principals, ","))
if client.CertAlgoIsDeprecated(algo) {
color.HiYellow(" - Algorithm is deprecated: %s", algo)
deprecatedAlgos = append(deprecatedAlgos, algo)
if client.CertKeyTypeIsDeprecated(keyType) {
color.HiYellow(" - Key type is deprecated: %s", keyType)
deprecatedKeys++
}
if client.CertKeyTypeIsBuggy(keyType) {
buggyKeys++
}
totalKeys++
}

if len(deprecatedAlgos) != 0 {
// If all signed keys are deprecated or buggy, show a warning
if deprecatedKeys+buggyKeys == totalKeys {
color.HiYellow(`
At least one of signed certificates has type that is deprecated by openssh.
if you are experiencing connection errors, try to update signmykey signer backend.
Also, you can temporary enable deprecated algorithm by adding to ~/.ssh/config :`)
for _, algo := range deprecatedAlgos {
color.HiYellow("\n PubkeyAcceptedKeyTypes +%v", algo)
}
All signed certificates use key types that are deprecated or buggy. If
you are experiencing connection errors, try to generate a new key with
an alternative algorithm :
ssh-keygen -f ~/.ssh/id_ed25519 -t ed25519
`)
}

return nil
Expand Down

0 comments on commit 0e30ff4

Please sign in to comment.