Update golang crypto library

[saliceti]

Fix https://github.com/cloudfoundry/diego-ssh/issues/31 and same problem on ssh-proxy.
Related to an issue encountered on concourse: http://concourse.ci/downloads.html#v270

The problem can be reproduced by uploading data via cf ssh. Example:

dd if=/dev/zero bs=1024k count=5000 | pv | ./cf ssh <app-name> -c "cat > /dev/null"

Without this patch the transfer stops at 2GB.

With the patch the upload can finish. However, it fails at 2GB with recent cf cli versions, see: cloudfoundry/cli#1098

[cf-gitbot]

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/142145463

The labels on this github issue will be updated when the story is started.

[cfdreddbot]

Hey saliceti!

Thanks for submitting this pull request! I'm here to inform the recipients of the pull request that you and the commit authors have already signed the CLA.

[emalm]

Thanks, @saliceti. Prioritizing for the Diego team to evaluate and pull in.

Best,
Eric, CF Diego PM

[dcarley]

We have implemented an acceptance test in our own codebase to prevent regressions of this issue:

• https://github.com/alphagov/paas-cf/blob/88643b5fe55909a47e7bece670d09d432d62d63f/platform-tests/src/acceptance/cf_ssh_test.go#L41-L75

Do you think this belongs in CATS once the issue is fixed in Diego and CLI?

[emalm]

Thanks, @dcarley! @dsabeti and the CF Release Integration team hold domain over the CATs, so I'll let them weigh in on whether it would be valuable to have a regression test in that suite for this behavior.

Best,
Eric

[saliceti]

@dsabeti: to add to @dcarley's point, our tenants rely on SSH to import data into their SQL backend service, as documented here: https://docs.cloud.service.gov.uk/#creating-tcp-tunnels-with-ssh

[dsabeti]

@dcarley @saliceti, I keep wavering back and forth on whether to ask for y'all to PR this update. In general, I don't like the idea of adding a regression test to CATs, as it's meant to illustrate and test features and happy-path workflows.

I'll bring this up with @anEXPer and get back to you. In the mean time, another thing it consider is weather there's a lower-level place to put this test. For example, could this be tested in the SSH Proxy codebase rather than in CATs?

[dcarley]

I think CATS is probably the right place. We've seen that our users expect to be able to use cf ssh to transfer data in to and out of CF services that are bound to apps, which seems like a happy path behaviour. A low-level test wouldn't have told us that the end user functionality didn't work across the 3 separate components (daemon, proxy, and CLI).

[caod123]

@dcarley @saliceti We attempted to pull in this change and verify that it in fact fixes the diego-ssh issue; however, we're running into packaging issues with crypto master (which this PR is attempting to merge in):

# golang.org/x/crypto/curve25519
../../../packages/ssh_proxy/src/golang.org/x/crypto/curve25519/freeze_amd64.s:11: #include: open /var/vcap/data/packages/golang/37065d7bd5d4c91bf9f8d8c871f4859193808516/pkg/include/const_amd64.h: no such file or directory

We found the commit that did fix it though and that appears to work fine: https://go.googlesource.com/crypto/+/641ab6b32049cabca26c30bf27baaae445bf4175

If you'd like to update your PR to pull in that commit instead, we can probably process it.

[caod123]

Actually we'll just update our packaging spec to include .h files as well. This appears to be the first .h file we've included from a package so we never actually packaged them as well.

[caod123]

@dcarley @saliceti We just pushed an update to ./scripts/sync-package-specs to package .h files as well. Could you rebase your PR and run ./scripts/sync-package-specs and include the updated package spec changes in this PR as well?

[caod123]

Actually we'll just go ahead and do that to get this pulled in.

[dcarley]

I was about to, but might be easier for you to co-ordinate it, thanks 👍

[dsabeti]

Hey @dcarley, I wanted to wrap up your question about contributing your test to CATs. Thanks for writing this test and for thinking about contributing it upstream.

For now, we want to hold off on including this test in CATs, but we have a plan to include it in the future. CATs has become incredibly crufty, and at some point soon, we'd like to reevaluate all the tests we have in CATs and reorganize them into a series of suites whose responsibilities are clearer. One such test suite will likely manage edge cases, and this test suite will likely fit in there.

Feel free to follow up with me if you have any questions.