Skip to content
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

crypto/elliptic: hang in doubleJacobian with Curve P-521 #25054

Closed
alwaysbespoke opened this issue Apr 24, 2018 · 18 comments
Closed

crypto/elliptic: hang in doubleJacobian with Curve P-521 #25054

alwaysbespoke opened this issue Apr 24, 2018 · 18 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@alwaysbespoke
Copy link

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

1.10.1

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

Windows 10. Intel

What did you do?

When running an http request to an https server that requires a P521 Curve the code hangs without throwing an error.

So, try hitting a server with a certificate that requires P521

Please look at the following within elliptical.go...

ScalarMult
addJacobian
doubleJacobian

The code will hang within the ScalarMult loop and it will also hang within the Jacobian processes

What did you expect to see?

We need to see a successful key exchange

What did you see instead?

No key exchange occured

@FiloSottile FiloSottile changed the title HTTPS -> Curve P521 -> Hangs Up crypto/elliptic: hang in doubleJacobian with Curve P-521 Apr 24, 2018
@FiloSottile FiloSottile added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 24, 2018
@FiloSottile FiloSottile added this to the Go1.11 milestone Apr 24, 2018
@FiloSottile
Copy link
Contributor

FiloSottile commented Apr 24, 2018

Can you provide an example server and a code sample?

Also, I assume the architecture is amd64 / x86-64?

Finally, if during a hang you could send a SIGQUIT (Ctrl-\) to get a stack trace and send the output, that would help.

@alwaysbespoke
Copy link
Author

alwaysbespoke commented Apr 24, 2018

My client computer has an Intel i7-4720HQ

This is the server...

https://gosmo.server93.com

As far as code it's a standard http request to the above URL. These are the relative bits of code...

request, err := http.NewRequest("POST", url, bytes.NewBufferString(body))

request.Header.Set("Content-Type", "application/json")
request.Header.Set("Accept", "application/json")

client := &http.Client{ }

response, err := client.Do(request)

if err != nil {
    fmt.Println(err)
}

defer response.Body.Close()

@alwaysbespoke
Copy link
Author

Running some tests, the code always hangs here within ScalarMult

            for bitNum := 0; bitNum < 8; bitNum++ {
		x, y, z = curve.doubleJacobian(x, y, z)
		if byte&0x80 == 0x80 {
			x, y, z = curve.addJacobian(Bx, By, Bz, x, y, z)
		}
		byte <<= 1
	}

@FiloSottile
Copy link
Contributor

FiloSottile commented Apr 24, 2018

Thanks for the details, if you hit Ctrl-\ during a hang the traceback might tell us how deep it gets stuck. (Not positive it works on Windows, now that I think about it.)

@FiloSottile
Copy link
Contributor

I can't reproduce on go1.10.1 darwin/amd64 with the code below, I will try from a Windows machine. Can you check if just doing go run on the file below is enough to cause the hang on your system?

// +build ignore

package main

import (
	"bytes"
	"fmt"
	"io"
	"net/http"
	"os"
)

func main() {
	request, err := http.NewRequest("POST", "https://gosmo.server93.com", bytes.NewBufferString("TEST"))

	request.Header.Set("Content-Type", "application/json")
	request.Header.Set("Accept", "application/json")

	client := &http.Client{}

	response, err := client.Do(request)

	if err != nil {
		fmt.Println(err)
	}

	defer response.Body.Close()

	io.Copy(os.Stdout, response.Body)
}

@alwaysbespoke
Copy link
Author

Your code makes it to the serialized output. Then the window exits. I added an infinite loop at the end of main and it still closes.

@alwaysbespoke
Copy link
Author

Looks like it's running now. What do you think the issue was? I'm confused now lol

@alwaysbespoke
Copy link
Author

Diagnosed the problem.

Ok, so if you run P521 it will fail if the http request is made in a separate go routine. If everything is handled in the main function then there is no failure.

Still seems like a problem. Shouldn't hang like that.

@alwaysbespoke
Copy link
Author

Do you advise launching an issue with the routine manager?

@FiloSottile
Copy link
Contributor

It would definitely be a problem. Can you try to make a single file reproduction case to share with us? This issue is ok, no need to make a separate one.

@alwaysbespoke
Copy link
Author

Perfect. I'll create it now.

@alwaysbespoke
Copy link
Author

alwaysbespoke commented Apr 24, 2018

This breaks. If the dialHTTP() is not a routine then it will run.

package main

import (

    "bytes"
    "fmt"
    "net/http"

)

func main() {

    go dialHTTP()

    for{}
}

func dialHTTP() {

    body := `{"username":"REDACTED BY @FiloSottile","password":"REDACTED BY @FiloSottile"}`

    request, err := http.NewRequest("POST", "https://gosmo.server93.com/comGpsGate/api/v.1/applications/4/tokens", bytes.NewBufferString(body))

    request.Header.Set("Content-Type", "application/json")
    request.Header.Set("Accept", "application/json")

    client := &http.Client{}

    response, err := client.Do(request)

    if err != nil {
        fmt.Println(err)
    }

    defer response.Body.Close()

    fmt.Println(response.Body)

}

@FiloSottile
Copy link
Contributor

I noticed that if you remove the for {} loop and replace it with a channel, a sync.WaitGroup or with select {} (the best way to say "wait forever" without holding resources), this doesn't happen.

What I think is going on is that the for {} is spinning and taking all resources, so the other goroutine never gets CPU time. Also, the scheduler makes no promise about the order in which it runs things, so here it "decided" to run the main goroutine instead of the other one, since you never pass the result back. It's in a way similar to #19182.

Can you reproduce it without a for {}?

@bradfitz
Copy link
Contributor

Sounds like a duplicate of #10958 (and #24543, etc)

@alwaysbespoke
Copy link
Author

alwaysbespoke commented Apr 24, 2018 via email

@alwaysbespoke
Copy link
Author

Using the select{} solves the problem entirely. It allows for spinning up goroutines that handle https requests without locking up.

@FiloSottile
Copy link
Contributor

Closing as a duplicate of #10958

Also, would you say that "select" is the best wait-forever method in go?

Yep, select {} is the appropriate way to block a goroutine forever, but in most cases you'll want to coordinate with a sync.Group or channel instead.

@davecheney
Copy link
Contributor

Deleting the Go statement on the previous line achieves the same thing.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

6 participants
@bradfitz @davecheney @FiloSottile @gopherbot @alwaysbespoke and others