-
Notifications
You must be signed in to change notification settings - Fork 268
Add initial version of the handshake command #1402
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
base: master
Are you sure you want to change the base?
Conversation
This commit adds the command `step certificate handshake`. This command performs a handshake and displays details about it.
command/certificate/handshake.go
Outdated
return tlsDialWithFallback(addr, tlsConfig) | ||
} | ||
defer conn.Close() | ||
conn.Handshake() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can use HandshakeContext
, and it would be good to check and return the error.
In a follow up we could implement some additional error handling logic for more informative errors based on some internal code we have. I think it could be nice to put that in tlsutil
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed with 6a13fa3
command/certificate/handshake.go
Outdated
// Print only the list of verified chains | ||
if printChains { | ||
for _, chain := range cs.VerifiedChains { | ||
for _, crt := range chain { | ||
fmt.Print(string(pem.EncodeToMemory(&pem.Block{ | ||
Type: "CERTIFICATE", | ||
Bytes: crt.Raw, | ||
}))) | ||
} | ||
} | ||
return nil | ||
} | ||
|
||
// Print only the peer certificates | ||
if printPeer { | ||
for _, crt := range cs.PeerCertificates { | ||
fmt.Print(string(pem.EncodeToMemory(&pem.Block{ | ||
Type: "CERTIFICATE", Bytes: crt.Raw, | ||
}))) | ||
} | ||
return nil | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe needs an option to continue down, so that the connection details are shown in addition to these too? Or make that the default, instead of returning early?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure about this. I make them independent commands because a certificate, especially if you include all the intermediates, is quite noisy. I also wanted to pipe those certificates to another command.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's fair, but in practice providing one of the printPeer
or printChains
options is doing the same as step certificate inspect https://example.com
and step certificate inspect https://example.com --bundle
, respectively, so if intend to return early to pipe the certs, those commands can be used, incl. JSON format.
If instead execution would continue after printing the details, it would be a bit more like curl -v
(with more certificate details, of course). That would be like enabling these options results in a more verbose output, which I think is fair, considering the default is to print just TLS handshake details.
Co-authored-by: Herman Slatman <hslatman@users.noreply.github.com>
import ( | ||
"strings" | ||
"testing" | ||
|
||
"github.com/pkg/errors" | ||
"github.com/stretchr/testify/assert" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
import ( | |
"strings" | |
"testing" | |
"github.com/pkg/errors" | |
"github.com/stretchr/testify/assert" | |
import ( | |
"errors" | |
"strings" | |
"testing" | |
"github.com/stretchr/testify/assert" |
"github.com/smallstep/cli-utils/command" | ||
) | ||
|
||
// Command returns the cli.Command for jwt and related subcommands. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// Command returns the cli.Command for jwt and related subcommands. | |
// Command returns the cli.Command for tls and related subcommands. |
"github.com/smallstep/cli-utils/errs" | ||
"github.com/smallstep/cli/flags" | ||
"github.com/smallstep/cli/internal/cryptoutil" | ||
"github.com/smallstep/cli/utils" | ||
"github.com/urfave/cli" | ||
"go.step.sm/crypto/pemutil" | ||
"go.step.sm/crypto/x509util" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"github.com/smallstep/cli-utils/errs" | |
"github.com/smallstep/cli/flags" | |
"github.com/smallstep/cli/internal/cryptoutil" | |
"github.com/smallstep/cli/utils" | |
"github.com/urfave/cli" | |
"go.step.sm/crypto/pemutil" | |
"go.step.sm/crypto/x509util" | |
"github.com/urfave/cli" | |
"github.com/smallstep/cli-utils/errs" | |
"go.step.sm/crypto/pemutil" | |
"go.step.sm/crypto/x509util" | |
"github.com/smallstep/cli/flags" | |
"github.com/smallstep/cli/internal/cryptoutil" | |
"github.com/smallstep/cli/utils" |
Description
This commit adds the command
step certificate handshake
. This command performs a handshake and displays details about it.For example: