Created working example SFTP server #54
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Purely gluing together the standalone_server and the SSH server examples, since it took me a while to work out how they fit together. Thought it may save the next person a few minutes!
| // Based on example server code from golang.org/x/crypto/ssh and server_standalone | ||
| func main() { | ||
|
|
||
| readOnly := false |
There was a problem hiding this comment.
try
var (
readOnly bool
debufLevelStr string
...
)
These will then have their zero value, which will then be overwritten by flag.Parse
|
Looks pretty good to me, thank you. If you can clean up the minor comments I'll merge this for you. |
Replaced panic with log.Fatal Removed old commented code line Cleaned up declaration of flag variables Fixed rootDir flag variable - was pointing to debugLevelStr Log any final error returned by SFTP subsystem
|
Thanks for this example, it was very helpful! I look forward to seeing it merged. |
|
@mdlayher if you want to take over this PR that would be great. |
|
@davecheney Sure, would you just like the other examples updated as well? I see your previous comments have been resolved. |
|
Oh, I didn't realise the OP had updated it. I'll merge this now. |
davecheney
added a commit
that referenced
this pull request
Dec 2, 2015
Created working example SFTP server
gopherbot
pushed a commit
to golang/crypto
that referenced
this pull request
Oct 3, 2016
After discussion around an example SFTP implementation: pkg/sftp#54 it has been suggested that errors should be handled using log.Fatal rather than panic, and that the actual underlying error should also be logged. In the existing SSH examples there are several different styles of error handling using both panic and log.Fatalf. This patch uses log.Fatal consistently for all of these cases. Change-Id: I2cebfae1821530dc3c5bbc46d451fe026bed582f Reviewed-on: https://go-review.googlesource.com/16736 Reviewed-by: Russ Cox <[email protected]>
tg123
pushed a commit
to tg123/sshpiper
that referenced
this pull request
Oct 9, 2016
After discussion around an example SFTP implementation: pkg/sftp#54 it has been suggested that errors should be handled using log.Fatal rather than panic, and that the actual underlying error should also be logged. In the existing SSH examples there are several different styles of error handling using both panic and log.Fatalf. This patch uses log.Fatal consistently for all of these cases. Change-Id: I2cebfae1821530dc3c5bbc46d451fe026bed582f Reviewed-on: https://go-review.googlesource.com/16736 Reviewed-by: Russ Cox <[email protected]>
tg123
added a commit
to tg123/sshpiper
that referenced
this pull request
Dec 29, 2018
sync golang ssh x/crypto/ssh: ParsePrivateKey errors out with encrypted private keys RSA and DSA keys if encrypted have the phrase ENCRYPTED in their Proc-Type block header according to RFC 1421 Section 4.6.1.1. This CL checks for that phrase and errors out if we encounter it, since we don't yet have decryption of encrypted private keys. Fixes golang/go#6650 Change-Id: I5b157716a2f93557d289af5f62994234a2e7a0ed Reviewed-on: https://go-review.googlesource.com/29676 Reviewed-by: Han-Wen Nienhuys <[email protected]> Run-TryBot: Han-Wen Nienhuys <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> ssh/terminal: implement ReadPassword and IsTerminal Fixes golang/go#13085. Change-Id: I2fcdd60e5e8db032d6fa3ce76198bdc7a63f3cf6 Reviewed-on: https://go-review.googlesource.com/16722 Run-TryBot: Russ Cox <[email protected]> Reviewed-by: Russ Cox <[email protected]> ssh: Consistent error handling in examples After discussion around an example SFTP implementation: pkg/sftp#54 it has been suggested that errors should be handled using log.Fatal rather than panic, and that the actual underlying error should also be logged. In the existing SSH examples there are several different styles of error handling using both panic and log.Fatalf. This patch uses log.Fatal consistently for all of these cases. Change-Id: I2cebfae1821530dc3c5bbc46d451fe026bed582f Reviewed-on: https://go-review.googlesource.com/16736 Reviewed-by: Russ Cox <[email protected]> x/crypto/ssh: public key authentication example Fixes golang/go#13902. Adds public key authentication to the password authentication example. Change-Id: I4af0ca627fb15b617cc1ba1c6e0954b013f4d94f Reviewed-on: https://go-review.googlesource.com/29374 Reviewed-by: Han-Wen Nienhuys <[email protected]> Run-TryBot: Han-Wen Nienhuys <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> ssh: fix height/width order in RequestPty example The RequestPty function takes the size arguments in the order height, then width, instead of the more common width, then height. 80 is a very common width for a terminal, so when the example reads RequestPty(..., 80, 40, ...), it's easy to assume that the order is width-height. Switching the order should make it more obvious what is going on. Change-Id: I1d6266b1c0dcde5ee6e31a6d26d2dcaf14fec58a Reviewed-on: https://go-review.googlesource.com/18290 Run-TryBot: Russ Cox <[email protected]> Reviewed-by: Russ Cox <[email protected]> ssh: add CryptoPublicKey interface, expose underlying crypto.PublicKey When implemented by ssh.PublicKey types, the new CryptoPublicKey interface exposes the public key in the the crypto.PublicKey form via a CryptoPublicKey() method. This is useful for example in a custom ServerConfig.PublicKeyCallback function to check or record additional details about the underlying crypto.PublicKey Change-Id: I4429df42c6fc5119f7c0023a539aaa9c59648bba Reviewed-on: https://go-review.googlesource.com/23974 Run-TryBot: Brad Fitzpatrick <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Brad Fitzpatrick <[email protected]> sync with golang/ssh upstream Allow hyphen-characters within usernames (#9) * Allow hyphen-characters within usernames. * Allow up to 32 characters for usernames. ssh: bound DH public values to [2, p-2]. Previously this code bounded the values to [1, p-1]. This protects against invalid values that could take lots of CPU time to calculate with. But the standard bounding is [2, p-2] so mirror that. Since the DH exchange is signed anyway, this is not a security fix. Change-Id: Ibef01805a596a433b0699d7a09c076344fa8c070 Reviewed-on: https://go-review.googlesource.com/30590 Reviewed-by: Brad Fitzpatrick <[email protected]> crypto/ssh: fix comment for ssh.NewPublicKey Change-Id: I88bb7859259c82cd77ab2d26b728143281761def Reviewed-on: https://go-review.googlesource.com/25232 Reviewed-by: Russ Cox <[email protected]> x/crypto/ssh: Add FingerprintLegacyMD5 and FingerprintSHA256 methods Implement a standards-compliant fingerprint format method (RFC 4716 section 4) and a newer SHA256 fingerprint format method. Fixes golang/go#12292 Change-Id: I4f3f8fc1d0a263cb3b0964d0078e69006a39d1a5 Reviewed-on: https://go-review.googlesource.com/32814 Reviewed-by: Han-Wen Nienhuys <[email protected]> Run-TryBot: Han-Wen Nienhuys <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> x/crypto/ssh/terminal: replace \n with \r\n. 911fafb28f4 made MakeRaw match C's behaviour. This included clearing the OPOST flag, which means that one now needs to write \r\n for a newline, otherwise the cursor doesn't move back to the beginning and the terminal prints a staircase. (Dear god, we're still emulating line printers.) This change causes the terminal package to do the required transformation. Fixes golang/go#17364. Change-Id: Ida15d3cf701a21eaa59161ab61b3ed4dee2ded46 Reviewed-on: https://go-review.googlesource.com/33902 Reviewed-by: Brad Fitzpatrick <[email protected]> crypto/ssh: use net.IP.Equal instead of bytes.Equal A net.IP may be represented by both by a 4 as well as a 16 byte long byte slice. Because of this, it is not safe to compare IP addresses using bytes.Equal as the same IP address using a different internal representation will produce mismatches. Change-Id: I0d228771cf363ccfb9532f8bc2a2fc8eff61f6e9 Reviewed-on: https://go-review.googlesource.com/34450 Reviewed-by: Brad Fitzpatrick <[email protected]> Run-TryBot: Brad Fitzpatrick <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> ssh/terminal: fix a typo Change-Id: Iafe2ebb6d37afd2a64aa72750a722d4860bb735e Reviewed-on: https://go-review.googlesource.com/34535 Run-TryBot: Mikio Hara <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Brad Fitzpatrick <[email protected]> all: fix some vet warnings Change-Id: I85c2912a6862c6c251450f2a0926ecd33a9fb8e7 Reviewed-on: https://go-review.googlesource.com/34815 Reviewed-by: Brad Fitzpatrick <[email protected]> Run-TryBot: Brad Fitzpatrick <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> ssh/terminal: consistent return value for Restore This patch makes the Restore function return nil on success to be consistent with other functions like MakeRaw. Change-Id: I81e63f568787dd88466a5bb30cb87c4c3be75a5c Reviewed-on: https://go-review.googlesource.com/34952 Reviewed-by: Brad Fitzpatrick <[email protected]> Run-TryBot: Brad Fitzpatrick <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> x/ssh: filter debug and ignore messages in transport.readPacket. This prevents these messages from confusing higher layers of the protocol. Fixes #16927. Change-Id: If18d8d02bdde3c0470e29a7280cd355d3e55ad78 Reviewed-on: https://go-review.googlesource.com/34959 Run-TryBot: Han-Wen Nienhuys <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Han-Wen Nienhuys <[email protected]> ssh: make client auth tests less chatty. Change-Id: Ib35ce0e7437e32a3fa24a9330c479306b7fa6880 Reviewed-on: https://go-review.googlesource.com/35011 Reviewed-by: Adam Langley <[email protected]> ssh: rewrite (re)keying logic. Use channels and a dedicated write loop for managing the rekeying process. This lets us collect packets to be written while a key exchange is in progress. Previously, the read loop ran the key exchange, and writers would block if a key exchange was going on. If a reader wrote back a packet while processing a read packet, it could block, stopping the read loop, thus causing a deadlock. Such coupled read/writes are inherent with handling requests that want a response (eg. keepalive, opening/closing channels etc.). The buffered channels (most channels have capacity 16) papered over these problems, but under load SSH connections would occasionally deadlock. Fixes #18439. Change-Id: I7c14ff4991fa3100a5d36025125d0cf1119c471d Reviewed-on: https://go-review.googlesource.com/35012 Run-TryBot: Han-Wen Nienhuys <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Han-Wen Nienhuys <[email protected]> ssh/terminal: fix line endings handling in ReadPassword Fixes golang/go#16552 Change-Id: I18a9c9b42fe042c4871b3efb3f51bef7cca335d0 Reviewed-on: https://go-review.googlesource.com/25355 Reviewed-by: Adam Langley <[email protected]> Reviewed-by: Adam Langley <[email protected]> ssh/terminal: consume data before checking for an error. According to the io.Reader docs, Alex had it right the first time. (See discussion on https://golang.org/cl/25355.) Change-Id: Ib6fb9dfb99009e034263574e82d7e9d4828df38f Reviewed-on: https://go-review.googlesource.com/35242 TryBot-Result: Gobot Gobot <[email protected]> Run-TryBot: Adam Langley <[email protected]> Reviewed-by: Alex Brainman <[email protected]> Reviewed-by: Adam Langley <[email protected]> crypto/ssh: fix parsing order for ssh.ParseDSAPrivateKey The inline struct has the wrong order for the public and private key parts. Change-Id: Ib3a5d6846296a2300241331a2ad398579e042ca9 Reviewed-on: https://go-review.googlesource.com/35351 Run-TryBot: Brad Fitzpatrick <[email protected]> Reviewed-by: Brad Fitzpatrick <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> ssh: soft code internal channel size for testing purposes Change-Id: I2ee0ed4ba82d2d156a7896551dea04b28cdeceb0 Reviewed-on: https://go-review.googlesource.com/35184 Run-TryBot: Han-Wen Nienhuys <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Adam Langley <[email protected]> ssh: make sure we execute the initial key exchange only once The initial kex is started from both sides simultaneously, and before, we could consume the the incoming kex request before we consumed from our internal channel. This would result in initiating a key exchange just after completing the initial one, which is not only an extra delay, but also an error when using OpenSSH (OpenSSH does not support key exchanges during user authentication). Change-Id: Ia7e0748ea2bca80ae97d187bcf2931ab6422276b Reviewed-on: https://go-review.googlesource.com/35851 Run-TryBot: Han-Wen Nienhuys <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Adam Langley <[email protected]> ssh: rationalize rekeying decisions. 1) Always force a key exchange if we exchange 2^31 packets. In the past this might not happen if RekeyThreshold was set to a very large interval. 2) Follow recommendations from RFC 4344 for block ciphers. For AES, we can encrypt 2^(blocksize/4) blocks under the same keys. On modern hardware, the previous default of 1Gb could force a key exchange within ~10 seconds. Since the key exchange takes 3 roundtrips (send kex init, send DH init, send NEW_KEYS), this is relatively expensive on high-latency links. Change-Id: I1297124a307c541b7bf22d814d136ec0c6d8ed97 Reviewed-on: https://go-review.googlesource.com/35410 Run-TryBot: Han-Wen Nienhuys <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Adam Langley <[email protected]> ssh: add debug print at the lowest level This is a simple minded, fast print, suitable for debugging timing sensitive issues. Change-Id: I9ae3df5fe86f1883c1fa9265b6f7f9a98d33747e Reviewed-on: https://go-review.googlesource.com/36054 Run-TryBot: Han-Wen Nienhuys <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Brad Fitzpatrick <[email protected]> ssh: reset buffered packets after sending Since encryption messes up the packets, the wrongly retained packets look like noise and cause application protocol errors or panics in the SSH library. This normally triggers very rarely: the mandatory key exchange doesn't have parallel writes, so this failure condition would be setup on the first key exchange, take effect only after the second key exchange. Fortunately, the tests against openssh exercise this. This change adds also adds a unittest. Fixes #18850. Change-Id: I656c8b94bfb265831daa118f4d614a2f0c65d2af Reviewed-on: https://go-review.googlesource.com/36056 Reviewed-by: Brad Fitzpatrick <[email protected]> Run-TryBot: Han-Wen Nienhuys <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> ssh: Support multiple source-addresses, don't require IPv4 in tests. The ssh tests currently require 127.0.0.1 to work which isn't necessarily available everywhere. To fix the source-address tests, support comma-separated source-address values per the PROTOCOL.certkeys file: Comma-separated list of source addresses from which this certificate is accepted for authentication. Addresses are specified in CIDR format (nn.nn.nn.nn/nn or hhhh::hhhh/nn). If this option is not present then certificates may be presented from any source address. Change-Id: I87536ff81ffa005c073da103021ebc0dfb12b620 Reviewed-on: https://go-review.googlesource.com/36110 Reviewed-by: Han-Wen Nienhuys <[email protected]> Run-TryBot: Heschi Kreinick <[email protected]> ssh: prevent double kex at connection start, 2nd try The previous attempt would fail in the following scenario: * select picks "first" kex from requestKex * read loop receives a remote kex, posts on requestKex (which is now empty) [*] for sending out a response, and sends pendingKex on startKex. * select picks pendingKex from startKex, and proceeds to run the key exchange. * the posting on requestKex in [*] now triggers a second key exchange. Fixes #18861. Change-Id: I443e82f1d04c7f17d1485fdb87072b9feec26aa8 Reviewed-on: https://go-review.googlesource.com/36055 Run-TryBot: Han-Wen Nienhuys <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Han-Wen Nienhuys <[email protected]> ssh/agent: fix another test to not require IPv4. Missed a copy/paste of netPipe in change 36110. Change-Id: I1a850dd9273d71fadc0519cf4cb2a2de6ecae4c2 Reviewed-on: https://go-review.googlesource.com/36259 Run-TryBot: Brad Fitzpatrick <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Han-Wen Nienhuys <[email protected]> ssh: Add the [email protected] algorithm Fixes golang/go#17676 Change-Id: I96c51431b174898a6bc0f6bec7f4561d5d64819f Reviewed-on: https://go-review.googlesource.com/35513 Reviewed-by: Han-Wen Nienhuys <[email protected]> Run-TryBot: Han-Wen Nienhuys <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> ssh: require host key checking in the ClientConfig This change breaks existing behavior. Before, a missing ClientConfig.HostKeyCallback would cause host key checking to be disabled. In this configuration, establishing a connection to any host just works, so today, most SSH client code in the wild does not perform any host key checks. This makes it easy to perform a MITM attack: * SSH installations that use keyboard-interactive or password authentication can be attacked with MITM, thereby stealing passwords. * Clients that use public-key authentication with agent forwarding are also vulnerable: the MITM server could allow the login to succeed, and then immediately ask the agent to authenticate the login to the real server. * Clients that use public-key authentication without agent forwarding are harder to attack unnoticedly: an attacker cannot authenticate the login to the real server, so it cannot in general present a convincing server to the victim. Now, a missing HostKeyCallback will cause the handshake to fail. This change also provides InsecureIgnoreHostKey() and FixedHostKey(key) as ready made host checkers. A simplistic parser for OpenSSH's known_hosts file is given as an example. This change does not provide a full-fledged parser, as it has complexity (wildcards, revocation, hashed addresses) that will need further consideration. When introduced, the host checking feature maintained backward compatibility at the expense of security. We have decided this is not the right tradeoff for the SSH library. Fixes golang/go#19767 Change-Id: I45fc7ba9bd1ea29c31ec23f115cdbab99913e814 Reviewed-on: https://go-review.googlesource.com/38701 Run-TryBot: Han-Wen Nienhuys <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Brad Fitzpatrick <[email protected]> ssh: handle error from prepareKeyChange. Fixes #18850. Change-Id: Id3ae89233f9e95ec3238462bf2ecda3e0c515f88 Reviewed-on: https://go-review.googlesource.com/36051 Run-TryBot: Han-Wen Nienhuys <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Adam Langley <[email protected]> ssh: fix typo in unexported comment Thanks to Anisse Astier (@anisse) for noticing. Change-Id: I1c282b2bb54601cf5649e194eafd5344c70331ca Reviewed-on: https://go-review.googlesource.com/38916 Reviewed-by: dnv aps Sn <[email protected]> Reviewed-by: Brad Fitzpatrick <[email protected]> ssh: improve client public key authentication Previously, the public key authentication for clients would send an enquiry to the remote for every key specified before attempting to authenticate with the server. Now, we immediately try to authenticate once a valid key is found. This results in exchanging fewer packets if the valid key is near the top of the list. If all keys fail, then the number of packets exchanged by the client and server is unaffected. For OpenSSH daemon, an enquiry into the validity of a key without authentication is still recorded as an authentication attempt, so any clients with more than MaxAuthTries public keys would not be able to authenticate using the previous implementation. This change will allow clients to succeed authentication if the successful key is at the start of the list of keys. Change-Id: I8ea42caf40c0864752218c3f6934e86b12f5b81a Reviewed-on: https://go-review.googlesource.com/38890 Reviewed-by: Adam Langley <[email protected]> ssh: reject RekeyThresholds over MaxInt64 This fixes weirdness when users use int64(-1) as sentinel value. Also, really use cipher specific default thresholds. These were added in a59c127441a8ae2ad9b0fb300ab36a6558bba697, but weren't taking effect. Add a test. Fixes golang/go#19639 Change-Id: Ie9518a0ff12fded2fca35465abb427d7a9f84340 Reviewed-on: https://go-review.googlesource.com/39431 Run-TryBot: Han-Wen Nienhuys <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Brad Fitzpatrick <[email protected]> ssh: fix format string in client_test.go Change-Id: I92c3916b0b5628dc2079af82202d9bfef032c708 Reviewed-on: https://go-review.googlesource.com/39430 Reviewed-by: Brad Fitzpatrick <[email protected]> Run-TryBot: Han-Wen Nienhuys <[email protected]> ssh: Add support for RSA keys stored in OpenSSH's new format Adds support for parsing RSA keys in the openssh-key-v1 private key format. Change-Id: Iacdcbaadf72413e4067d146203604fb50b780083 Reviewed-on: https://go-review.googlesource.com/35244 Run-TryBot: Han-Wen Nienhuys <[email protected]> Reviewed-by: Paul Querna <[email protected]> Reviewed-by: Han-Wen Nienhuys <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> ssh: support forwarding of Unix domain socket connections This commit implements OpenSSH streamlocal extension, providing the equivalent of `ssh -L local.sock:remote.sock`. Change-Id: Idd6287d5a5669c643132bba770c3b4194615e84d Reviewed-on: https://go-review.googlesource.com/38614 Reviewed-by: Han-Wen Nienhuys <[email protected]> Run-TryBot: Han-Wen Nienhuys <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> ssh: support MaxAuthTries on ServerConfig This change breaks backwards compatibility. MaxAuthTries specifies the maximum number of authentication attempts permitted per connection. If set to a negative number, the server will allow unlimited authentication attempts. MaxAuthTries defaults to 6 if not specified, which is a backwards incompatible change. On exceeding maximum authentication attempts, the server will send a disconnect message to the client. This configuration property mirrors a similar property in sshd_config and prevents bad actors from continuously trying authentication. Change-Id: Ic77d2c29ee2fd2ae5c764becf7df91d29d03131b Reviewed-on: https://go-review.googlesource.com/35230 Reviewed-by: Han-Wen Nienhuys <[email protected]> Run-TryBot: Han-Wen Nienhuys <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> ssh: set rekeying thresholds on construction The normal handshake kicks off with a waitSession(), which guarantees that we never attempt to send data before the first kex is completed, but ensuring readPacketsLeft > 0 and writePacketsLeft > 0 helps understand that thresholds can never cause spurious rekeying at the start of a connection. Change-Id: If5bcafcda0c7d16fd21f22c664101ac5f5b487d7 Reviewed-on: https://go-review.googlesource.com/38696 Reviewed-by: Adam Langley <[email protected]> Run-TryBot: Adam Langley <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> ssh: fix reset{Read,Write}Thresholds for initial setup Fixes a nil pointer dereference that slipped through buildbots because it was introduced by the last two commits. Change-Id: Ib269e910956cd8b3b46e217b03fde1b61572260a Reviewed-on: https://go-review.googlesource.com/40530 Reviewed-by: Han-Wen Nienhuys <[email protected]> Run-TryBot: Han-Wen Nienhuys <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> ssh/knownhosts: a parser for the OpenSSH known_hosts file format Change-Id: I271c90ff3a6d59e2e075c785a6bdb79e4b0849fa Reviewed-on: https://go-review.googlesource.com/40354 Run-TryBot: Han-Wen Nienhuys <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Adam Langley <[email protected]> ssh/knownhosts: fix variable reuse bug in checkAddrs Consider the following code: var p *int a := []int{0, 1, 2, 3} for _, i := range a { if i == 1 { p = &i } } fmt.Println(*p) // Prints 3 This prints 3 because the variable i is the exact same variable across all iterations of the loop. When the address is taken for some specific iteration, the user's intent is to capture the value of i at that given loop, but instead the value of i in the last loop is what remains. A bug this sort occurs in the check logic since the address of the knownKey is taken, but is changed upon subsequent iterations of the loop (which happens when there are multiple lines). Change-Id: Ic626778cdcde3968dcff4fa5e7206274957dcb04 Reviewed-on: https://go-review.googlesource.com/40937 Reviewed-by: Brad Fitzpatrick <[email protected]> Run-TryBot: Brad Fitzpatrick <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> ssh/knownhosts: support hashed hostnames Change-Id: I855a6542a2eb2ae1d223f03892c0f19da81a4f8d Reviewed-on: https://go-review.googlesource.com/40532 Run-TryBot: Han-Wen Nienhuys <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Adam Langley <[email protected]> ssh/knownhosts: add file + linenumber for parse errors Change-Id: Iddcb145ecd8a6b51c72ad3d77b242975baf4a5cf Reviewed-on: https://go-review.googlesource.com/41210 Run-TryBot: Han-Wen Nienhuys <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Sam Whited <[email protected]> Reviewed-by: Han-Wen Nienhuys <[email protected]> ssh/terminal: implement missing functions for Solaris/OmniOS terminal.MakeRaw terminal.Restore terminal.GetState terminal.GetSize Fixes golang/go#20062 Change-Id: I9ccf194215998c5b80dbedc4f248b481f0ca57a6 Reviewed-on: https://go-review.googlesource.com/41297 Reviewed-by: Brad Fitzpatrick <[email protected]> ssh/knownhosts: add IsHostAuthority. This is a breaking change. This adds a new hostkey callback which takes the hostname field restrictions into account when validating host certificates. Prior to this, a known_hosts file with the following entry @cert-authority *.example.com ssh-rsa <example.com public key> would, when passed to knownhosts.New() generate an ssh.HostKeyCallback that would accept all host certificates signed by the example.com public key, no matter what host the client was connecting to. After this change, that known_hosts entry can only be used to validate host certificates presented when connecting to hosts under *.example.com This also renames IsAuthority to IsUserAuthority to make its intended purpose more clear. Change-Id: I7188a53fdd40a8c0bc21983105317b3498f567bb Reviewed-on: https://go-review.googlesource.com/41751 Reviewed-by: Han-Wen Nienhuys <[email protected]> Run-TryBot: Han-Wen Nienhuys <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> ssh/knownhosts: test coverage for IsHostAuthority Change-Id: Iad24fed7cec998e02620ec0eb61658786156ba41 Reviewed-on: https://go-review.googlesource.com/42530 Reviewed-by: Brad Fitzpatrick <[email protected]> Run-TryBot: Brad Fitzpatrick <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> crypto/ssh: fix tests on Go 1.7 on OpenBSD and Windows Dialing the 0.0.0.0 address (as returned by net.Addr().String() for a net.Listen("tcp", ":1") address) is not yet guaranteed to work. It's currently OS-dependent. For some reason it works on Go 1.8+, but it hasn't yet been defined to work reliably. Fix the tests for now (since we need to support older Go releases), even if this might work in the future. Updates golang/go#18806 Change-Id: I2f0476b1d4f2673ab64ffedfa733f2d92fceb6ff Reviewed-on: https://go-review.googlesource.com/42496 Run-TryBot: Brad Fitzpatrick <[email protected]> Run-TryBot: Han-Wen Nienhuys <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Han-Wen Nienhuys <[email protected]> ssh: change the local copy of the ServerConfig passed to NewServerConn Otherwise callers are forced to serialize access to the ServerConfig. Change-Id: Id36f4d2877ea28b18447ef777d3839b21136c22f Reviewed-on: https://go-review.googlesource.com/42821 Reviewed-by: Brad Fitzpatrick <[email protected]> Run-TryBot: Brad Fitzpatrick <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> x/crypto/ssh: fix host certificate principal evaluation to check for hostname only SSH host certificates are expected to contain hostnames only, not "host:port" format. This change allows Go clients to connect to OpenSSH servers that use host certificates. Note, this change will break any clients that use ssh.NewClientConn() with an `addr` that is not in `host:port` format (they will see a "missing port in address" error). Fixes bug 20273. Change-Id: I5a306c6b7b419a737e1f0f9c5ca8c585e21a45a4 Reviewed-on: https://go-review.googlesource.com/43475 Reviewed-by: Han-Wen Nienhuys <[email protected]> Run-TryBot: Han-Wen Nienhuys <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> ssh: fixing a small typo in connection.go Change-Id: Iffbed7e16a8bb32c5ff7c393f3b6ad7dcffc69ac Reviewed-on: https://go-review.googlesource.com/44340 Reviewed-by: Han-Wen Nienhuys <[email protected]> Run-TryBot: Han-Wen Nienhuys <[email protected]> ssh: add ParsePrivateKeysWithPassphrase ssh package doesn't provide way to parse private keys with passphrase. Fixes golang/go#18692 Change-Id: Ic139f11b6dfe7ef61690d6125e0673d50a48db16 Reviewed-on: https://go-review.googlesource.com/36079 Run-TryBot: Han-Wen Nienhuys <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Han-Wen Nienhuys <[email protected]> ssh: clarify intended use of Permissions. The Permissions struct should be used to pass information from authentication callback to server application. Fixes golang/go#20094. Change-Id: I5542b657d053452327260707a24925286546bfdd Reviewed-on: https://go-review.googlesource.com/45311 Run-TryBot: Han-Wen Nienhuys <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Brad Fitzpatrick <[email protected]> all: gofmt ./... Change-Id: I8ffee4dc712091e424b83a9f5a3cc2a6724abefc Reviewed-on: https://go-review.googlesource.com/46050 Reviewed-by: Matt Layher <[email protected]>
bored-engineer
pushed a commit
to bored-engineer/ssh
that referenced
this pull request
Oct 13, 2019
After discussion around an example SFTP implementation: pkg/sftp#54 it has been suggested that errors should be handled using log.Fatal rather than panic, and that the actual underlying error should also be logged. In the existing SSH examples there are several different styles of error handling using both panic and log.Fatalf. This patch uses log.Fatal consistently for all of these cases. Change-Id: I2cebfae1821530dc3c5bbc46d451fe026bed582f Reviewed-on: https://go-review.googlesource.com/16736 Reviewed-by: Russ Cox <[email protected]>
bored-engineer
pushed a commit
to bored-engineer/ssh
that referenced
this pull request
Oct 13, 2019
After discussion around an example SFTP implementation: pkg/sftp#54 it has been suggested that errors should be handled using log.Fatal rather than panic, and that the actual underlying error should also be logged. In the existing SSH examples there are several different styles of error handling using both panic and log.Fatalf. This patch uses log.Fatal consistently for all of these cases. Change-Id: I2cebfae1821530dc3c5bbc46d451fe026bed582f Reviewed-on: https://go-review.googlesource.com/16736 Reviewed-by: Russ Cox <[email protected]>
c-expert-zigbee
pushed a commit
to c-expert-zigbee/crypto_go
that referenced
this pull request
Mar 28, 2022
After discussion around an example SFTP implementation: pkg/sftp#54 it has been suggested that errors should be handled using log.Fatal rather than panic, and that the actual underlying error should also be logged. In the existing SSH examples there are several different styles of error handling using both panic and log.Fatalf. This patch uses log.Fatal consistently for all of these cases. Change-Id: I2cebfae1821530dc3c5bbc46d451fe026bed582f Reviewed-on: https://go-review.googlesource.com/16736 Reviewed-by: Russ Cox <[email protected]>
c-expert-zigbee
pushed a commit
to c-expert-zigbee/crypto_go
that referenced
this pull request
Mar 29, 2022
After discussion around an example SFTP implementation: pkg/sftp#54 it has been suggested that errors should be handled using log.Fatal rather than panic, and that the actual underlying error should also be logged. In the existing SSH examples there are several different styles of error handling using both panic and log.Fatalf. This patch uses log.Fatal consistently for all of these cases. Change-Id: I2cebfae1821530dc3c5bbc46d451fe026bed582f Reviewed-on: https://go-review.googlesource.com/16736 Reviewed-by: Russ Cox <[email protected]>
LewiGoddard
pushed a commit
to LewiGoddard/crypto
that referenced
this pull request
Feb 16, 2023
After discussion around an example SFTP implementation: pkg/sftp#54 it has been suggested that errors should be handled using log.Fatal rather than panic, and that the actual underlying error should also be logged. In the existing SSH examples there are several different styles of error handling using both panic and log.Fatalf. This patch uses log.Fatal consistently for all of these cases. Change-Id: I2cebfae1821530dc3c5bbc46d451fe026bed582f Reviewed-on: https://go-review.googlesource.com/16736 Reviewed-by: Russ Cox <[email protected]>
BiiChris
pushed a commit
to BiiChris/crypto
that referenced
this pull request
Sep 15, 2023
After discussion around an example SFTP implementation: pkg/sftp#54 it has been suggested that errors should be handled using log.Fatal rather than panic, and that the actual underlying error should also be logged. In the existing SSH examples there are several different styles of error handling using both panic and log.Fatalf. This patch uses log.Fatal consistently for all of these cases. Change-Id: I2cebfae1821530dc3c5bbc46d451fe026bed582f Reviewed-on: https://go-review.googlesource.com/16736 Reviewed-by: Russ Cox <[email protected]>
desdeel2d0m
added a commit
to desdeel2d0m/crypto
that referenced
this pull request
Jul 1, 2024
After discussion around an example SFTP implementation: pkg/sftp#54 it has been suggested that errors should be handled using log.Fatal rather than panic, and that the actual underlying error should also be logged. In the existing SSH examples there are several different styles of error handling using both panic and log.Fatalf. This patch uses log.Fatal consistently for all of these cases. Change-Id: I2cebfae1821530dc3c5bbc46d451fe026bed582f Reviewed-on: https://go-review.googlesource.com/16736 Reviewed-by: Russ Cox <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Purely gluing together the standalone_server and the SSH server examples, since
it took me a while to work out how they fit together. Thought it may save the next
person a few minutes!