Add custom error type for crypto backend errors

[samiponkanenssh]

I would like to start a discussion about the need for typed errors from the crypto backends.

The use case I have is detecting whether an error was triggered by OpenSSL. Currently golang-fips/openssl extracts the error string from OpenSSL and then calls errors.New() to create the error. This means that the only way to find out if error was triggered by OpenSSL is to do matching on the error strings. I find that approach error prone and not future proof.

Instead, I would find the following very handy:

Firstly, in microsoft/go, define a CryptoBackendError interface:
src/crypto/boring/boring.go:

type CryptoBackendError interface {
	error
	CryptoBackend() string
}

Next, in golang-fips/openssl, define a OpenSSLError type and wrap errors to it (note: this is not a complete diff):

diff -urN go1.23.8-1.orig/src/vendor/github.com/golang-fips/openssl/v2/openssl.go go1.23.8-1.cryptobackend-error/src/vendor/github.com/golang-fips/openssl/v2/openssl.go
--- go1.23.8-1.orig/src/vendor/github.com/golang-fips/openssl/v2/openssl.go      2025-04-22 09:34:23.824810048 +0300
+++ go1.23.8-1.cryptobackend-error/src/vendor/github.com/golang-fips/openssl/v2/openssl.go  2025-04-22 14:20:40.295540401 +0300
@@ -74,12 +74,21 @@
 }
 
 func errUnsupportedVersion() error {
-       return errors.New("openssl: OpenSSL version: " + utoa(vMajor) + "." + utoa(vMinor) + "." + utoa(vPatch))
+       return &openSSLError{err: errors.New("openssl: OpenSSL version: " + utoa(vMajor) + "." + utoa(vMinor) + "." + utoa(vPatch))}
 }
 
 type fail string
 
-func (e fail) Error() string { return "openssl: " + string(e) + " failed" }
+func (e fail) Error() string         { return "openssl: " + string(e) + " failed" }
+func (e fail) CryptoBackend() string { return "opensslcrypto" }
+
+type openSSLError struct {
+       err error
+}
+
+func (e openSSLError) Error() string         { return e.err.Error() }
+func (e openSSLError) Unwrap() error         { return e.err }
+func (e openSSLError) CryptoBackend() string { return "opensslcrypto" }
 
 // VersionText returns the version text of the OpenSSL currently loaded.
 func VersionText() string {
@@ -260,7 +269,7 @@
                C.go_openssl_ERR_error_string_n(e, (*C.char)(unsafe.Pointer(&buf[0])), C.size_t(len(buf)))
                b.WriteString(string(buf[:]) + "\n\t" + C.GoString(file) + ":" + strconv.Itoa(int(line)))
        }
-       return errors.New(b.String())
+       return &openSSLError{err: errors.New(b.String())}
 }
 
 var unknownFile = "<go code>\000"

Finally, in the application use errors.As() to check if an error is from OpenSSL:

var cryptoBackendErr boring.CryptoBackendError
if errors.As(err, &cryptoBackendErr) {
...

This is not a trivial change, as a) more work would be needed to convert all errors returned in golang-fips/openssl and 2) microsoft/go-crypto-winnative should eventually be converted, for the sake of completeness. The bright side though is, that the work could be done in multiple phases and independently for each crypto backend.

Thoughts?

[qmuntal]

Thanks for submitting this proposal @samiponkanenssh. Before further evaluating it, could you share more details about why you need to know if a given error is originated from OpenSSL?

[samiponkanenssh]

My use case is specifically related to postgres scram-sha-256 password authentication when using a OpenSSL FIPS provider that enforces minimum HMAC key length. When the password is too short, OpenSSL panics with an error "EVP_MAC_init failed\nopenssl error(s):". When this happens, I do not want to retry connecting to the database, like I would if the error was a network related or timeout error.

[qmuntal]

We try hard to be compatible with upstream Go, the bar for unilaterally adding a new API to a standard library package is very high, and I'm afraid this request doesn't make the cut. You can already get what you need by erroring inspecting the error message or by failing when running in fips mode (using fips140.Enabled) and the password is not long enough.

[dagood]

Firstly, in microsoft/go, define a CryptoBackendError interface: src/crypto/boring/boring.go:

type CryptoBackendError interface {
	error
	CryptoBackend() string
}

If I'm not mistaken, this interface could be defined somewhere else and the rest could still work the same, right?

Making a helper library for things like this without touching the stdlib is something I've thought about before, but we haven't done it.

[samiponkanenssh]

If I'm not mistaken, this interface could be defined somewhere else and the rest could still work the same, right?

Making a helper library for things like this without touching the stdlib is something I've thought about before, but we haven't done it.

Yes, the interface definition could be outside of std lib and, as your linked issue comment suggests, a utility package in github.com/golang-fips repo would be a natural place for it.

[qmuntal]

If I'm not mistaken, this interface could be defined somewhere else and the rest could still work the same, right?

The errors returned from a function are part of the package API, even if not properly surfaced in the documentation, so my previous comment still holds. I don't want projects to start depending on Microsoft-specific behavior nor APIs unless there is a very good reason for it.

Also, wrapping all errors we return from the backends (via return statement or panic) would be difficult to implement, maintain and test.

@samiponkanenssh I think that the path of least resistance here is that you check the password length when running in FIPS mode before calling the ofending crypto function.

[samiponkanenssh]

@samiponkanenssh I think that the path of least resistance here is that you check the password length when running in FIPS mode before calling the ofending crypto function.

Unfortunately that will not work due to github.com/lib/pq implementation: The server side decides how to authenticate the client and the affected scram-sha-256 - while being most used nowdays - is just one possible auth method. The client is not capable of enforcing which auth method is to be used. Say the server would want to do plaintext password auth. In that case there is no problem with using a short password.

[qmuntal]

Then you can do the key length check if the authentication failed. There you can inspect the error message to infer whether the error came from openssl or cng.

[qmuntal]

Where you able to solve this issue with the provided instructions @samiponkanenssh?

[samiponkanenssh]

I did solve it, but not 100% happy with the hack'ish solution:

// errEVPMacInitFailed returns an error the HMAC init failure triggers
var errEVPMacInitFailed = sync.OnceValue(func() (err error) {
	// Trigger "EVP_MAC_init failed" error by initializing a HMAC with short key
	defer func() {
		if r := recover(); r != nil {
			if rerr, ok := r.(error); ok {
				err = rerr
			}
		}
	}()
	_ = hmac.New(sha256.New, []byte("s"))
	return nil
})

// IsFIPSCryptoError checks if specified err is a known error from FIPS crypto
// backend. This function uses string prefix matching against known error
// strings. Not optimal, but sufficient.
func IsFIPSCryptoError(err error) bool {
	errStr := err.Error()
	for _, cryptoErr := range []error{errEVPMacInitFailed()} {
		if cryptoErr != nil &&
			strings.HasPrefix(cryptoErr.Error(), errStr) {
			return true
		}
	}
	return false
}

[qmuntal]

Yep, that error checking is hairy and not really portable. hmac.New doesn't panic when using the standard library, and the only failure mode (other than an allocation failure) that I can think of is the key being too short. Knowing this, I don't see why you can't assume that if hmac.New panics, then the issue is the key so there is no need to retry whatever workflow is ongoing.

[samiponkanenssh]

The problem is that when hmac.New() panics, one of the places it does so is inside the github.com/lib/pq package. The pq package in turn recovers from all panics and just returns an error. So really, all I get is an error and I don't know its origin.

Furthermore, as of now this hmac.New() panic is specific to one FIPS crypto provider. I am trying to build our product in a way that does not tie the implementation strictly to that FIPS provider.

[qmuntal]

The pq package in turn recovers from all panics and just returns an error. So really, all I get is an error and I don't know its origin.

Thanks for the details, that was the information I was missing. Then your current approach of inspecting the error message is the right one. We don't plan to provide a custom error type for backend errors for now given how costly it would be to maintain and test. I'll keep this proposal in mind for the future, though.