adding aes128-cbc support to ssh.go

[Dennis Francis]

Hi all,


I used the following code (derived from https://github.com/davecheney/socksie/blob/master/main.go) which uses the SSHAgent support in the ssh package
to provide authentication to connect to a sftp server.


===========[test.go]========================================
package main

import (
    "code.google.com/p/go.crypto/ssh"
    "flag"
    "fmt"
    "log"
    "net"
    "os"
)

var (
    USER = flag.String("user", os.Getenv("USER"), "ssh username")
    HOST = flag.String("host", "127.0.0.1", "ssh server hostname")
    PASS = flag.String("pass", "", "ssh password")
)

func init() { flag.Parse() }


//password implements the ClientPassword interface                                                                                                                                                              
type password string

func (p password) Password(user string) (string, error) {
    return string(p), nil
}


func main() {

    var auths []ssh.ClientAuth
    if agent, err := net.Dial("unix", os.Getenv("SSH_AUTH_SOCK")); err == nil {
    fmt.Println("ssh-agent")
        auths = append(auths, ssh.ClientAuthAgent(ssh.NewAgentClient(agent)))
    }
    if *PASS != "" {
        auths = append(auths, ssh.ClientAuthPassword(password(*PASS)))
    }

    config := &ssh.ClientConfig{

    User: *USER,
    Auth: auths,
    }
    addr := fmt.Sprintf("%s:%d", *HOST, 22)
    conn, err := ssh.Dial("tcp", addr, config)
    if err != nil {
    log.Fatalf("unable to connect to [%s]: %v", addr, err)
    }
    defer conn.Close()

}

============================================================================

I also added the following debug statement to code.google.com/p/go.crypto/ssh/common.go

func findCommonCipher(clientCiphers []string, serverCiphers []string) (commonCipher string, ok bool) {
    for _, clientCipher := range clientCiphers {
    for _, serverCipher := range serverCiphers {
            // reject the cipher if we have no cipherModes definition                                                                                                                                            
            if clientCipher == serverCipher && cipherModes[clientCipher] != nil {
        return clientCipher, true
            }
        }
    }
    fmt.Println("no common cipher clientCiphers=", clientCiphers, "serverCiphers=", serverCiphers)
    return
}

Output
======

myhost ~ $ ./test -host <server> -user <username>
ssh-agent
no common cipher clientCiphers= [aes128-ctr aes192-ctr aes256-ctr arcfour256 arcfour128] serverCiphers= [aes128-cbc blowfish-cbc aes256-cbc aes192-cbc 3des-cbc]
No common writer cipher Algo
2013/07/20 22:01:33 unable to connect to [<server>:22]: handshake failed: ssh: no common algorithms


I see that from the ssh/cipher.go that there is no support for *-cbc ciphers, so I like to add support for these.

I see from crypto/cipher pkg, that CBC ciphers have different encrypter and decrypter functions and the ssh/cipher.go/createCipher() function
doesn't have an indicator that specifies whether the current operation is encryption or decryption.

Can someone give a clue on how to add cbc cipher support to ssh/cipher.go ?

Thanks a lot,
Dennis

[Taru Karttunen]

On 20.07 19:29, Dennis Francis wrote:
> Hi all,
>

Hello

There are security issues with using CBC ciphers with ssh.

http://www.kb.cert.org/vuls/id/958563

- Taru Karttunen

[John Beisley]

I started a code contribution to add this and many other ciphers quite a while ago, but this was reduced down to a much smaller set (for good reason, at least in the core functionality). You can see my code for that in earlier revisions of https://codereview.appspot.com/5342057/

Adding block mode ciphers like CBC requires some careful and fiddly coding in the transport layer to get the frame length correct, I think it's only stream mode ciphers at the moment which are much simpler to deal with.

Taru makes a good point from the security standpoint: the better option is to change the peer to be aes-ctr capable.

--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

[Dennis Francis]

Thanks a lot for pointing me to to your code. I will use this code temporarily till the sftp server shifts to aes-ctr cipher.

[mark.sh...@gmail.com]

Hi; I have worked on integrating the results from patch 6 of https://codereview.appspot.com/5342057/ to the current go.crypto/ssh source. Currently here: https://github.com/ScriptRock/ssh_block

Basically I need to be able to use aes128-cbc ciphers in order to SSH into older Cisco network equipment, which cannot be upgraded. My implementation adds aes128-cbc, aes192-cbc and aes256-cbc as non-default options to the ssh package.

Is it likely that this patch will be accepted into the official go releases? If so, what are the next steps towards getting this integrated? I attempted to clone the ssh repo on code.google.com using the web ui, but got 400 errors; I will re-attempt tomorrow.

These functions are probably inefficient; they were adapted from patch 6 with no further optimization. 
func (s *blockPacketCipher) readPacket(seqNum uint32, r io.Reader) ([]byte, error) 

func (s *blockPacketCipher) writePacket(seqNum uint32, w io.Writer, rand io.Reader, packet []byte) error 

Regards,

Mark

[ades...@instartlogic.com]

This maybe an old thread but I just encountered into this problem.

Any new updates on this issue ? 

[Ian Lance Taylor]

On Mon, Feb 16, 2015 at 2:28 AM, <ades...@instartlogic.com> wrote:
> This maybe an old thread but I just encountered into this problem.
>
> Any new updates on this issue ?

As it says on the thread, there are security issues with using SSH
with CBC. Therefore, there are no current plans to add that support
to the main Go sources.

Ian

> For more options, visit https://groups.google.com/d/optout.

[Agniva De Sarkar]

Thanks Ian. Understood.

--

Agniva De Sarker 

+91 9986158795
Software Engineer | Instart Logic
www.instartlogic.com

[Mark Sheahan]

I had intended to make some sort of plugin mechanism for ciphers, but still haven't got around to it; sorry. Recent progress is to move my fork to github: https://github.com/ScriptRock/crypto/, forked from https://github.com/golang/crypto/

--
You received this message because you are subscribed to a topic in the Google Groups "golang-nuts" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/golang-nuts/J2XCsTsNQ9o/unsubscribe.
To unsubscribe from this group and all its topics, send an email to golang-nuts...@googlegroups.com.

[Agniva De Sarkar]

Yea I tried that.

When I did go get, it said  -

can't load package: package github.com/ScriptRock/crypto: no buildable Go source files in $HOME/go/src/github.com/ScriptRock/crypto

So I left it at that.

[Mark Sheahan]

Are you running 'go get github.com/ScriptRock/crypto' or 'go get github.com/ScriptRock/crypto/ssh'? I forked the crypto repository, of which ssh is a sub-package. The import path I'm using is:

import (

        "fmt"

        "github.com/ScriptRock/crypto/ssh"

...etc

[Agniva De Sarkar]

Thanks, that helped.

But I am still getting the "ssh: handshake failed: ssh: no common algorithms" error.

This is my ssh config - 

conn, err := net.Dial("unix", os.Getenv("SSH_AUTH_SOCK"))

if err != nil {

log.Fatalln(err)

}

defer conn.Close()

ag := agent.NewClient(conn)

ssh_config := &ssh.ClientConfig{

User: "adesarkar",

Auth: []ssh.AuthMethod{

ssh.PublicKeysCallback(ag.Signers),

},

}

And this is where my error comes - 

conn, err := ssh.Dial("tcp", proxy+":22", ssh_config)

if err != nil {

log.Fatalln("Failed to dial ssh - ", err)

}

[Mark Sheahan]

Try the following code snippet: you must enable the other ciphers because the CBC ciphers are disabled by default:

        templateSshConfig := ssh.ClientConfig{

                User: ......,

               Auth: ....,

                Config: ssh.Config{

                        Ciphers: ssh.AllSupportedCiphers(), // include cbc ciphers (even though they're bad, mmmkay)

                },

        }

[Agniva De Sarkar]

There it is !! Thanks.

[Charles Li]

> As it says on the thread, there are security issues with using SSH 

with CBC.  Therefore, there are no current plans to add that support 
to the main Go sources. 

This is an unfortunate situation. I can understand the reasoning behind not using CBC ciphers by default due to security concerns; this is the route chosen by OpenSSH in their latest release: http://www.openssh.com/txt/release-6.7.  However, as expressed in this thread by others, the reality is that even modern clients need to connect to legacy systems that only support CBC ciphers, and upgrading those 3rd-party servers is usually not an option for developers. 

Completely leaving out the option of using CBC ciphers seems domineering with regards to guiding appropriate developer behavior. What is wrong with adding opt-in logic for this capability?

I'm grateful to Mark for providing a fork of crypto that enables these ciphers (would have otherwise been forced to switch to another language). It's a shame that lack of native library support for these ciphers requires developers to rewrite imports of packages that rely upon crypto/ssh (e.g. github.com/pkg/sftp) when a seemingly reasonable patch already exists.

[Mark Sheahan]

Incidentally, exactly that package for exactly that reason: https://github.com/ScriptRock/sftp

--
You received this message because you are subscribed to a topic in the Google Groups "golang-nuts" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/golang-nuts/J2XCsTsNQ9o/unsubscribe.
To unsubscribe from this group and all its topics, send an email to golang-nuts...@googlegroups.com.

[Mark Sheahan]

Incidentally, that package for that reason: https://github.com/ScriptRock/sftp

On Wed, Feb 25, 2015 at 2:55 PM, Charles Li <charles...@gmail.com> wrote:

--
You received this message because you are subscribed to a topic in the Google Groups "golang-nuts" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/golang-nuts/J2XCsTsNQ9o/unsubscribe.
To unsubscribe from this group and all its topics, send an email to golang-nuts...@googlegroups.com.

[Cosmin Nicolaescu]

Hello,

Instead of having the code for CBC and having it commented out, thus forcing people to fork in order to get the functionality, have we considered an approach of having a `crypto.Unsafe` flag that needs to be set before being able to use it? I agree with the idea of discouraging people from using things with security concerns, but in this particular case, there are constraints which may require the use, and creating multiple forks at different levels to get this enabled doesn't seem the best way to go about it, since now you may miss other important updates.

Happy to create a PR with that change, but figured I'd see how folks feel about it beforehand. 

Thanks! 

[Benjamin Measures]

On Thursday, 28 May 2015 16:02:05 UTC+1, bre...@commercev3.com wrote:

I don't think its the language's job to police security in a library.

On the contrary- library designers have a /huge/ responsibility when it comes to security: http://op-co.de/blog/posts/java_sslsocket_mitm/

[Donald King]

I agree, but that's an argument for secure-out-of-the-box, not can't-reduce-the-security-even-if-your-job-depends-on-it.

[Mike Williamson]

Hi all,

Maybe this is beating a dead horse, but has there been any change in the thinking on this issue?  As a newcomer to Go, this thread is kind of depressing.  I think we can all agree that it would be better if there were no legacy systems using outdated protocols, but they do exist and some of us need to deal with them.  Refusing even guardedly to support them in the standard library doesn't improve the situation in any way, it just inevitably means that the core library and any dependents get forked and hacked (thanks, Mark!) so people can get their jobs done.  So now the user base is split over two libraries with all the attendant inefficiencies that entails.  I can't see any advantage at all in standing on this principle.

Regards,

Mike

[Mark Sheahan]

CBC is in the current tip: https://github.com/golang/crypto/blob/master/ssh/cipher.go#L120 but commented out, so you'd need to fork or somehow distribute a patch if you wanted to use that.

Tip and my branch have diverged somewhat; I tried a pull from upstream recently, and the merge was non-trivial.

Is there a 'nice' way to do this using go 1.5 vendor imports, such that people wouldn't need to re-write the import paths in other packages?

--

[Mike Williamson]

Thanks for the update, I guess that's progress of a sort.  

The new vendoring support would help to some extent.  If I understand how that works, one should only have to copy the affected packages (crypto/ssh and for example sftp) into a vendor subdirectory and uncomment the line in cipher.go.  You shouldn't have to re-write any import paths, but you do need to vendor all the packages that you want to benefit from the change.

Obviously your solution of controlling it at run-time through ssh.Config would be much better.  It's a shame that hasn't been adopted.

[viljo...@gmail.com]

Hi,

To start off I'm fairly new to Go, I would just like to know the following:

If I make changes similar to yours in common.go, how would I be able to make use of them in my main package.

For example in my common.go file I added the following

==== common.go ====

func returnCiphers() []string {

return supportedCiphers

}

In my main package, how will I call it as the following does not seem to work:

==== main.go ====

fmt.Println(ssh.returnCiphers())

Assistance will greatly be appreciated.

[Ain]

On Wednesday, September 14, 2016 at 3:17:19 PM UTC+3, viljo...@gmail.com wrote:

Hi,

To start off I'm fairly new to Go, I would just like to know the following:

If I make changes similar to yours in common.go, how would I be able to make use of them in my main package.

For example in my common.go file I added the following

==== common.go ====

func returnCiphers() []string {

return supportedCiphers

}

In my main package, how will I call it as the following does not seem to work:

==== main.go ====

fmt.Println(ssh.returnCiphers())

Assistance will greatly be appreciated.

Your function name starts with lower case character which means that it is not exported.


ain