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/x509: Verify on macOS does not return typed errors #56891

Open
wneessen opened this issue Nov 21, 2022 · 10 comments
Open

crypto/x509: Verify on macOS does not return typed errors #56891

wneessen opened this issue Nov 21, 2022 · 10 comments
Labels
NeedsFix The path to resolution is known, but the work has not been done. OS-Darwin
Milestone

Comments

@wneessen
Copy link

wneessen commented Nov 21, 2022

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

$ go version
go version go1.19.3 darwin/arm64

(But the issue apparently started in 1.18)

Does this issue reproduce with the latest release?

Yes

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

Darwin arm64

go env Output
$ go env

GO111MODULE=""
GOARCH="arm64"
GOBIN=""
GOCACHE="/Users/wneessen/Library/Caches/go-build"
GOENV="/Users/wneessen/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="arm64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/wneessen/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/wneessen/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/opt/homebrew/Cellar/go/1.19.3/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/opt/homebrew/Cellar/go/1.19.3/libexec/pkg/tool/darwin_arm64"
GOVCS=""
GOVERSION="go1.19.3"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/wneessen/go/src/dev-to-example/go.mod"
GOWORK=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/lw/f2psdp2s5fg4z_jw0q58rdgm0000gn/T/go-build119423787=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

package main

import (
	"crypto/x509"
	"errors"
	"fmt"
	"net/http"
	"net/url"
	"reflect"
)

func main() {
	_, err := http.Get("https://expired.badssl.com/")
	if err != nil {
		switch {
		case errors.As(err, &x509.CertificateInvalidError{Reason: x509.Expired}):
			fmt.Printf("Certificate is expired...\n")
		default:
			if ue, ok := err.(*url.Error); ok {
				fmt.Printf("Unwrapped error is type: %s\n", reflect.TypeOf(ue.Err))
			}
		}
	}
}

What did you expect to see?

I expect to see the "Certificate is expired..." output

What did you see instead?

I received this: Unwrapped error is type: *errors.errorString

@wneessen
Copy link
Author

wneessen commented Nov 21, 2022

On @FiloSottile's advise, I am pinging him and @rolandshoemaker

@rolandshoemaker
Copy link
Member

rolandshoemaker commented Nov 21, 2022

This will also be an issue on Windows.

Probably reasonable that we should try to convert to a Go style CertificateInvalidError if we can. I know macOS does provide relatively good insight into trust failures (i.e. https://developer.apple.com/documentation/security/certificate_key_and_trust_services/trust/discovering_why_a_trust_evaluation_failed), not entirely sure about Windows (I assume there is probably something, but 🤷).

@rolandshoemaker
Copy link
Member

rolandshoemaker commented Nov 21, 2022

Oh hah, we actually already do this on Windows 🤦.

@rolandshoemaker rolandshoemaker added this to the Go1.21 milestone Nov 21, 2022
@rolandshoemaker rolandshoemaker added the NeedsFix The path to resolution is known, but the work has not been done. label Nov 21, 2022
@rolandshoemaker rolandshoemaker changed the title crypto/x509: 1.18+ on macOS not returning typed errors for certificate errors (i. e. expired certificate) crypto/x509: Verify on macOS does not return typed errors Nov 21, 2022
@gopherbot
Copy link

gopherbot commented Nov 22, 2022

Change https://go.dev/cl/452620 mentions this issue: crypto/x509: return typed verification errors on macOS

@rolandshoemaker
Copy link
Member

rolandshoemaker commented Nov 22, 2022

@golang/release requesting a freeze exception for this. It's a relatively minor change which simply changes error return types and as such is not very high risk, but should fix a regression that was present in 1.19 that makes macOS behave differently from all other platforms.

@rolandshoemaker rolandshoemaker changed the title crypto/x509: Verify on macOS does not return typed errors crypto/x509: Verify on macOS does not return typed errors [freeze exception] Nov 22, 2022
@bcmills
Copy link
Member

bcmills commented Nov 22, 2022

should fix a regression that was present in 1.19 that makes macOS behave differently from all other platforms.

If approved for a freeze exception, should it also be backported to 1.19 (on the same grounds)?

@dmitshur
Copy link
Contributor

dmitshur commented Nov 22, 2022

@golang/release requesting a freeze exception for this.

@rolandshoemaker Thanks for letting us know. A freeze exception shouldn't be needed here since this looks like an unintentional bug on macOS rather than new functionality. It's relatively early in the 1.20 freeze and it seems the fix should be safe to land at this stage, so please proceed if that works well for you, otherwise leaving this for 1.21 is fine.

@dmitshur dmitshur changed the title crypto/x509: Verify on macOS does not return typed errors [freeze exception] crypto/x509: Verify on macOS does not return typed errors Nov 22, 2022
@rolandshoemaker
Copy link
Member

rolandshoemaker commented Nov 22, 2022

@dmitshur 👍 sounds good.

@rolandshoemaker
Copy link
Member

rolandshoemaker commented Nov 22, 2022

@bcmills yeah I think we should probably backport it to 1.19 as well.

@heschi
Copy link
Contributor

heschi commented Nov 22, 2022

Dmitri pointed out in chat that this constitutes an API change and as such would have to be well-justified as a backport.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsFix The path to resolution is known, but the work has not been done. OS-Darwin
Projects
None yet
Development

No branches or pull requests

7 participants