bug fixes

corefoundation.go

@@ -267,7 +267,7 @@ func Convert(ref C.CFTypeRef) (interface{}, error) {
 		}
 		return b, nil
 	} else if typeID == C.CFNumberGetTypeID() {
-		return CFNumberToInterface(C.CFNumberRef(ref)), nil
+		return CFNumberToInterface(C.CFNumberRef(ref))
 	} else if typeID == C.CFBooleanGetTypeID() {
 		if C.CFBooleanGetValue(C.CFBooleanRef(ref)) != 0 {
 			return true, nil
@@ -300,72 +300,72 @@ func ConvertCFDictionary(d C.CFDictionaryRef) (map[interface{}]interface{}, erro
 // CFNumberToInterface converts the CFNumberRef to the most appropriate numeric
 // type.
 // This code is from github.com/kballard/go-osx-plist.
-func CFNumberToInterface(cfNumber C.CFNumberRef) interface{} {
+func CFNumberToInterface(cfNumber C.CFNumberRef) (interface{}, error) {
 	typ := C.CFNumberGetType(cfNumber)
 	switch typ {
 	case C.kCFNumberSInt8Type:
 		var sint C.SInt8
 		C.CFNumberGetValue(cfNumber, typ, unsafe.Pointer(&sint)) //nolint
-		return int8(sint)
+		return int8(sint), nil
 	case C.kCFNumberSInt16Type:
 		var sint C.SInt16
 		C.CFNumberGetValue(cfNumber, typ, unsafe.Pointer(&sint)) //nolint
-		return int16(sint)
+		return int16(sint), nil
 	case C.kCFNumberSInt32Type:
 		var sint C.SInt32
 		C.CFNumberGetValue(cfNumber, typ, unsafe.Pointer(&sint)) //nolint
-		return int32(sint)
+		return int32(sint), nil
 	case C.kCFNumberSInt64Type:
 		var sint C.SInt64
 		C.CFNumberGetValue(cfNumber, typ, unsafe.Pointer(&sint)) //nolint
-		return int64(sint)
+		return int64(sint), nil
 	case C.kCFNumberFloat32Type:
 		var float C.Float32
 		C.CFNumberGetValue(cfNumber, typ, unsafe.Pointer(&float)) //nolint
-		return float32(float)
+		return float32(float), nil
 	case C.kCFNumberFloat64Type:
 		var float C.Float64
 		C.CFNumberGetValue(cfNumber, typ, unsafe.Pointer(&float)) //nolint
-		return float64(float)
+		return float64(float), nil
 	case C.kCFNumberCharType:
 		var char C.char
 		C.CFNumberGetValue(cfNumber, typ, unsafe.Pointer(&char)) //nolint
-		return byte(char)
+		return byte(char), nil
 	case C.kCFNumberShortType:
 		var short C.short
 		C.CFNumberGetValue(cfNumber, typ, unsafe.Pointer(&short)) //nolint
-		return int16(short)
+		return int16(short), nil
 	case C.kCFNumberIntType:
 		var i C.int
 		C.CFNumberGetValue(cfNumber, typ, unsafe.Pointer(&i)) //nolint
-		return int32(i)
+		return int32(i), nil
 	case C.kCFNumberLongType:
 		var long C.long
 		C.CFNumberGetValue(cfNumber, typ, unsafe.Pointer(&long)) //nolint
-		return int(long)
+		return int(long), nil
 	case C.kCFNumberLongLongType:
 		// This is the only type that may actually overflow us
 		var longlong C.longlong
 		C.CFNumberGetValue(cfNumber, typ, unsafe.Pointer(&longlong)) //nolint
-		return int64(longlong)
+		return int64(longlong), nil
 	case C.kCFNumberFloatType:
 		var float C.float
 		C.CFNumberGetValue(cfNumber, typ, unsafe.Pointer(&float)) //nolint
-		return float32(float)
+		return float32(float), nil
 	case C.kCFNumberDoubleType:
 		var double C.double
 		C.CFNumberGetValue(cfNumber, typ, unsafe.Pointer(&double)) //nolint
-		return float64(double)
+		return float64(double), nil
 	case C.kCFNumberCFIndexType:
 		// CFIndex is a long
 		var index C.CFIndex
 		C.CFNumberGetValue(cfNumber, typ, unsafe.Pointer(&index)) //nolint
-		return int(index)
+		return int(index), nil
 	case C.kCFNumberNSIntegerType:
 		// We don't have a definition of NSInteger, but we know it's either an int or a long
 		var nsInt C.long
 		C.CFNumberGetValue(cfNumber, typ, unsafe.Pointer(&nsInt)) //nolint
-		return int(nsInt)
+		return int(nsInt), nil
 	}
-	panic("Unknown CFNumber type")
+	return nil, fmt.Errorf("unknown CFNumber type: %d", typ)
 }

go.mod

@@ -1,13 +1,13 @@
 module github.com/keybase/go-keychain
 
-go 1.24.0
+go 1.25.0
 
 toolchain go1.25.5
 
 require (
 	github.com/keybase/dbus v0.0.0-20220506165403-5aa21ea2c23a
 	github.com/stretchr/testify v1.11.1
-	golang.org/x/crypto v0.46.0
+	golang.org/x/crypto v0.54.0
 )
 
 require (

go.sum

@@ -6,8 +6,8 @@ github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZb
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
 github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U=
 github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U=
-golang.org/x/crypto v0.46.0 h1:cKRW/pmt1pKAfetfu+RCEvjvZkA9RimPbh7bhFjGVBU=
-golang.org/x/crypto v0.46.0/go.mod h1:Evb/oLKmMraqjZ2iQTwDwvCtJkczlDuTmdJXoZVzqU0=
+golang.org/x/crypto v0.54.0 h1:YLIA59K4fiNzHzjnZt2tUJQjQtUWfWbeHBqKtk3eScw=
+golang.org/x/crypto v0.54.0/go.mod h1:KWL8ny2AZdGR2cWmzeHrp2azQPGogOv+HeQaVEXC2dk=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
 gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=

keychain.go

@@ -558,8 +558,15 @@ func convertResult(d C.CFDictionaryRef) (*QueryResult, error) {
 		case AuthenticationTypeKey:
 			result.AuthenticationType = CFStringToString(C.CFStringRef(v))
 		case PortKey:
-			val := CFNumberToInterface(C.CFNumberRef(v))
-			result.Port = val.(int32)
+			val, err := CFNumberToInterface(C.CFNumberRef(v))
+			if err != nil {
+				return nil, fmt.Errorf("failed to convert port number: %w", err)
+			}
+			port, ok := val.(int32)
+			if !ok {
+				return nil, fmt.Errorf("port value has unexpected type %T, expected int32", val)
+			}
+			result.Port = port
 		case PathKey:
 			result.Path = CFStringToString(C.CFStringRef(v))
 		case AccountKey:

secretservice/dh_ietf1024_sha256_aes128_cbc_pkcs7_test.go

@@ -82,3 +82,36 @@ func TestPKCS7(t *testing.T) {
 	_, err = unpadPKCS7([]byte{1, 2, 3, 4, 1, 1, 1, 2}, 4)
 	require.Error(t, err)
 }
+
+// TestDecryptionErrors tests that decryption errors are properly returned
+// rather than being swallowed. This validates the fix for the security issue
+// where decryption failures would return (nil, nil) instead of (nil, err).
+func TestDecryptionErrors(t *testing.T) {
+	key := []byte("YELLOW SUBMARINE")
+
+	// Test 1: Invalid padding should return an error
+	invalidIV := make([]byte, 16)
+	invalidCiphertext := []byte{
+		0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07,
+		0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f,
+	} // 16 bytes, invalid padding
+	_, err := unauthenticatedAESCBCDecrypt(invalidIV, invalidCiphertext, key)
+	require.Error(t, err, "decryption with invalid padding should return an error")
+
+	// Test 2: Wrong key should produce unpadding errors
+	plaintext := []byte("secret message")
+	iv, ciphertext, err := unauthenticatedAESCBCEncrypt(plaintext, key)
+	require.NoError(t, err)
+
+	wrongKey := []byte("WRONG KEY HERE!!")
+	_, err = unauthenticatedAESCBCDecrypt(iv, ciphertext, wrongKey)
+	require.Error(t, err, "decryption with wrong key should return an error")
+
+	// Test 3: Corrupted ciphertext should error
+	corruptedCiphertext := make([]byte, len(ciphertext))
+	copy(corruptedCiphertext, ciphertext)
+	// Corrupt the last block (which contains padding)
+	corruptedCiphertext[len(corruptedCiphertext)-1] ^= 0xFF
+	_, err = unauthenticatedAESCBCDecrypt(iv, corruptedCiphertext, key)
+	require.Error(t, err, "decryption of corrupted ciphertext should return an error")
+}

secretservice/secretservice.go

@@ -282,7 +282,7 @@ func (s *SecretService) GetSecret(item dbus.ObjectPath, session Session) (secret
 	case AuthenticationDHAES:
 		plaintext, err := unauthenticatedAESCBCDecrypt(secret.Parameters, secret.Value, session.AESKey)
 		if err != nil {
-			return nil, nil
+			return nil, err
 		}
 		secretPlaintext = plaintext
 	default: