Merge branch 'main' into tsa-inclusive

vcsjones · web-flow · commit 3d9275d0bd58 · 2024-09-17T11:04:37.000-04:00
diff --git a/.github/workflows/build-binaries.yml b/.github/workflows/build-binaries.yml
@@ -1,10 +1,13 @@
 on: [push, pull_request]
 name: Build binaries
+permissions:
+  contents: read
+  packages: write
 jobs:
   build-macos:
     strategy:
       matrix:
-        go-version: ["1.17"]
+        go-version: ["1.23"]
         os: [macos-latest]
     runs-on: ${{ matrix.os }}
     env:
@@ -34,23 +37,23 @@ jobs:
           # We cd so that the binary ends up in the top level of the tar.
           cd build/macos && tar -czvf smimesign-macos-${{ env.GIT_VERSION }}.tgz smimesign
       - name: Upload build folder to the action
-        uses: actions/upload-artifact@v2
+        uses: actions/upload-artifact@v4
         with:
           # Note: this artifact is shared across jobs:
           # https://github.com/actions/upload-artifact#uploading-to-the-same-artifact
           name: build
           path: build/
       - name: Upload macOS files to the release
-        # Pinned hash from https://github.com/softprops/action-gh-release/releases/tag/v0.1.12
-        uses: softprops/action-gh-release@2d72d869af3bf23602f9593a1e3fd739b80ac1eb
+        # Pinned hash from https://github.com/softprops/action-gh-release/releases/tag/v2.0.8
+        uses: softprops/action-gh-release@c062e08bd532815e2082a85e87e3ef29c3e6d191
         if: startsWith(github.ref, 'refs/tags/v')
         with:
           files: |
             build/macOS/smimesign-macos-${{ env.GIT_VERSION }}.tgz
   build-windows:
     strategy:
       matrix:
-        go-version: ["1.17"]
+        go-version: ["1.23"]
         os: [windows-latest]
     runs-on: ${{ matrix.os }}
     env:
@@ -88,8 +91,8 @@ jobs:
         run: |
           GOARCH=amd64 go build -o "build/amd64/smimesign.exe" -ldflags "-X main.versionString=${{ env.GIT_VERSION }}"
       - name: Switch MinGW to x86
-        # Pinned hash from https://github.com/egor-tensin/setup-mingw/releases/tag/v2
-        uses: egor-tensin/setup-mingw@f3c5d799aadf8fa230ac67a422b01dd085bbc96b
+        # Pinned hash from https://github.com/egor-tensin/setup-mingw/releases/tag/v2.2.0
+        uses: egor-tensin/setup-mingw@84c781b557efd538dec66bde06988d81cd3138cf
         with:
           platform: x86
       - name: Build 386
@@ -119,15 +122,15 @@ jobs:
           mv build/amd64/smimesign.zip build/amd64/smimesign-windows-amd64-${{ env.GIT_VERSION }}.zip
           mv build/386/smimesign.zip build/386/smimesign-windows-386-${{ env.GIT_VERSION }}.zip
       - name: Upload build folder to the action
-        uses: actions/upload-artifact@v2
+        uses: actions/upload-artifact@v4
         with:
           # Note: this artifact is shared across jobs:
           # https://github.com/actions/upload-artifact#uploading-to-the-same-artifact
           name: build
           path: build/
       - name: Upload Windows files to the release
-        # Pinned hash from https://github.com/softprops/action-gh-release/releases/tag/v0.1.12
-        uses: softprops/action-gh-release@2d72d869af3bf23602f9593a1e3fd739b80ac1eb
+        # Pinned hash from https://github.com/softprops/action-gh-release/releases/tag/v2.0.8
+        uses: softprops/action-gh-release@c062e08bd532815e2082a85e87e3ef29c3e6d191
         if: startsWith(github.ref, 'refs/tags/v')
         with:
           files: |
diff --git a/.github/workflows/test-go.yml b/.github/workflows/test-go.yml
@@ -1,11 +1,13 @@
 on: [push, pull_request]
-name: Test macOS
+name: Test
+permissions:
+  contents: read
 jobs:
   test:
     strategy:
       matrix:
-        go-version: ["1.14", "1.x"]
-        os: [macos-latest]
+        go-version: ["1.22", "1.x"]
+        os: [macos-latest, windows-latest]
     runs-on: ${{ matrix.os }}
     env:
       CGO_ENABLED: 1
diff --git a/.github/workflows/test-windows-go.yml b/.github/workflows/test-windows-go.yml
diff --git a/certstore/certstore_darwin.go b/certstore/certstore_darwin.go
@@ -160,8 +160,9 @@ func (i *macIdentity) CertificateChain() ([]*x509.Certificate, error) {
 	}
 	defer C.CFRelease(C.CFTypeRef(trustRef))
 
-	var status C.SecTrustResultType
-	if err := osStatusError(C.SecTrustEvaluate(trustRef, &status)); err != nil {
+	var cfError C.CFErrorRef
+	if C.SecTrustEvaluateWithError(trustRef, &cfError) {
+		err := cfErrorError(cfError)
 		return nil, err
 	}
 
@@ -171,18 +172,22 @@ func (i *macIdentity) CertificateChain() ([]*x509.Certificate, error) {
 	)
 
 	for i := C.CFIndex(0); i < nchain; i++ {
-		// TODO: do we need to release these?
-		chainCertref := C.SecTrustGetCertificateAtIndex(trustRef, i)
-		if chainCertref == nilSecCertificateRef {
-			return nil, errors.New("nil certificate in chain")
+		chainCertCpy := C.SecTrustCopyCertificateChain(trustRef)
+
+		if C.CFArrayRef(chainCertCpy) == nilCFArrayRef {
+			return nil, errors.New("nil certificate in the chain")
 		}
 
-		chainCert, err := exportCertRef(chainCertref)
+		chainCertRef := C.SecCertificateRef(C.CFArrayGetValueAtIndex(chainCertCpy, i))
+
+		chainCert, err := exportCertRef(chainCertRef)
 		if err != nil {
 			return nil, err
 		}
 
 		chain = append(chain, chainCert)
+
+		C.CFRelease(C.CFTypeRef(chainCertCpy))
 	}
 
 	i.chain = chain
diff --git a/certstore/certstore_windows.go b/certstore/certstore_windows.go
@@ -637,7 +637,7 @@ func (c errCode) Error() string {
 	if cmsg == nil {
 		return fmt.Sprintf("Error %X", int(c))
 	}
-	defer C.LocalFree(C.HLOCAL(cmsg))
+	defer C.LocalFree(C.HLOCAL(unsafe.Pointer(cmsg)))
 
 	gomsg := C.GoString(cmsg)
 
diff --git a/go.mod b/go.mod
@@ -1,13 +1,17 @@
 module github.com/github/smimesign
 
-go 1.12
+go 1.22.7
 
 require (
 	github.com/certifi/gocertifi v0.0.0-20180118203423-deb3ae2ef261
-	github.com/davecgh/go-spew v1.1.1 // indirect
 	github.com/pborman/getopt v0.0.0-20180811024354-2b5b3bfb099b
 	github.com/pkg/errors v0.8.1
 	github.com/stretchr/testify v1.3.0
-	golang.org/x/crypto v0.0.0-20190426145343-a29dc8fdc734
-	golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1
+	golang.org/x/crypto v0.27.0
+	golang.org/x/xerrors v0.0.0-20240903120638-7835f813f4da
+)
+
+require (
+	github.com/davecgh/go-spew v1.1.1 // indirect
+	github.com/pmezard/go-difflib v1.0.0 // indirect
 )
diff --git a/go.sum b/go.sum
@@ -12,12 +12,7 @@ github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZN
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
 github.com/stretchr/testify v1.3.0 h1:TivCn/peBQ7UY8ooIcPgZFpTNSz0Q2U6UrFlUfqbe0Q=
 github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
-golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
-golang.org/x/crypto v0.0.0-20190426145343-a29dc8fdc734 h1:p/H982KKEjUnLJkM3tt/LemDnOc1GiZL5FCVlORJ5zo=
-golang.org/x/crypto v0.0.0-20190426145343-a29dc8fdc734/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
-golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
-golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
-golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
-golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
-golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 h1:go1bK/D/BFZV2I8cIQd1NKEZ+0owSTG1fDTci4IqFcE=
-golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
+golang.org/x/crypto v0.27.0 h1:GXm2NjJrPaiv/h1tb2UH8QfgC/hOf/+z0p6PT8o1w7A=
+golang.org/x/crypto v0.27.0/go.mod h1:1Xngt8kV6Dvbssa53Ziq6Eqn0HqbZi5Z6R0ZpwQzt70=
+golang.org/x/xerrors v0.0.0-20240903120638-7835f813f4da h1:noIWHXmPHxILtqtCOPIhSt0ABwskkZKjD3bXGnZGpNY=
+golang.org/x/xerrors v0.0.0-20240903120638-7835f813f4da/go.mod h1:NDW/Ps6MPRej6fsCIbMTohpP40sJ/P/vI1MoTEGwX90=
diff --git a/ietf-cms/README.md b/ietf-cms/README.md
@@ -18,7 +18,7 @@ der, _ := cms.Sign(msg, []*x509.Certificate{cert}, key)
 //
 
 sd, _ := ParseSignedData(der)
-if err, _ := sd.Verify(x509.VerifyOptions{}); err != nil {
+if _, err := sd.Verify(x509.VerifyOptions{}); err != nil {
   panic(err)
 }
 ```
@@ -37,7 +37,7 @@ der, _ := cms.SignDetached(msg, cert, key)
 //
 
 sd, _ := ParseSignedData(der)
-if err, _ := sd.VerifyDetached(msg, x509.VerifyOptions{}); err != nil {
+if _, err := sd.VerifyDetached(msg, x509.VerifyOptions{}); err != nil {
   panic(err)
 }
 ```
diff --git a/ietf-cms/verify_test.go b/ietf-cms/verify_test.go
@@ -133,6 +133,8 @@ func TestVerifyOpenSSLDetached(t *testing.T) {
 }
 
 func TestVerifyOutlookDetached(t *testing.T) {
+	t.Skip("Test fails. See https://github.com/github/smimesign/issues/150")
+
 	sd, err := ParseSignedData(fixtureSignatureOutlookDetached)
 	if err != nil {
 		t.Fatal(err)
@@ -144,6 +146,8 @@ func TestVerifyOutlookDetached(t *testing.T) {
 }
 
 func TestVerifySmimesignAttachedWithTimestamp(t *testing.T) {
+	t.Skip("Test fails. See https://github.com/github/smimesign/issues/150")
+
 	sd, err := ParseSignedData(fixtureSmimesignAttachedWithTimestamp)
 	if err != nil {
 		t.Fatal(err)
diff --git a/parse_user_id.go b/parse_user_id.go
@@ -4,7 +4,8 @@ import "strings"
 
 // The following was copied from the crypto/openpgpg/packet package.
 
-// The original license can be found at https://git.io/vFFwQ
+// The original license can be found at
+// https://github.com/golang/crypto/blob/9f005a07e0d31d45e6656d241bb5c0f2efd4bc94/LICENSE
 //
 //     Copyright (c) 2009 The Go Authors. All rights reserved.
 //
@@ -34,7 +35,8 @@ import "strings"
 //     (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 //     OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
-// The orignal code can be found at https://git.io/vFFwX
+// The orignal code can be found at
+// https://github.com/golang/crypto/blob/9f005a07e0d31d45e6656d241bb5c0f2efd4bc94/openpgp/packet/userid.go#L89-L160
 //
 // parseUserID extracts the name, comment and email from a user id string that
 // is formatted as "Full Name (Comment) <email@example.com>".
diff --git a/status.go b/status.go
@@ -14,7 +14,8 @@ import (
 
 // This file implements gnupg's "status protocol". When the --status-fd argument
 // is passed, gpg will output machine-readable status updates to that fd.
-// Details on the "protocol" can be found at https://git.io/vFFKC
+// Details on the "protocol" can be found at
+// https://github.com/gpg/gnupg/blob/918792befd835e04b4043b9ce42ea6d829a284fa/doc/DETAILS#format-of-the-status-fd-output
 
 type status string
 

Original file line number	Diff line number	Diff line change
`@@ -160,8 +160,9 @@ func (i macIdentity) CertificateChain() ([]x509.Certificate, error) {`
`160`	`160`	`}`
`161`	`161`	`defer C.CFRelease(C.CFTypeRef(trustRef))`
`162`	`162`
`163`		`- var status C.SecTrustResultType`
`164`		`- if err := osStatusError(C.SecTrustEvaluate(trustRef, &status)); err != nil {`
	`163`	`+ var cfError C.CFErrorRef`
	`164`	`+ if C.SecTrustEvaluateWithError(trustRef, &cfError) {`
	`165`	`+ err := cfErrorError(cfError)`
`165`	`166`	`return nil, err`
`166`	`167`	`}`
`167`	`168`
`@@ -171,18 +172,22 @@ func (i macIdentity) CertificateChain() ([]x509.Certificate, error) {`
`171`	`172`	`)`
`172`	`173`
`173`	`174`	`for i := C.CFIndex(0); i < nchain; i++ {`
`174`		`- // TODO: do we need to release these?`
`175`		`- chainCertref := C.SecTrustGetCertificateAtIndex(trustRef, i)`
`176`		`- if chainCertref == nilSecCertificateRef {`
`177`		`- return nil, errors.New("nil certificate in chain")`
	`175`	`+ chainCertCpy := C.SecTrustCopyCertificateChain(trustRef)`
	`176`	`+`
	`177`	`+ if C.CFArrayRef(chainCertCpy) == nilCFArrayRef {`
	`178`	`+ return nil, errors.New("nil certificate in the chain")`
`178`	`179`	`}`
`179`	`180`
`180`		`- chainCert, err := exportCertRef(chainCertref)`
	`181`	`+ chainCertRef := C.SecCertificateRef(C.CFArrayGetValueAtIndex(chainCertCpy, i))`
	`182`	`+`
	`183`	`+ chainCert, err := exportCertRef(chainCertRef)`
`181`	`184`	`if err != nil {`
`182`	`185`	`return nil, err`
`183`	`186`	`}`
`184`	`187`
`185`	`188`	`chain = append(chain, chainCert)`
	`189`	`+`
	`190`	`+ C.CFRelease(C.CFTypeRef(chainCertCpy))`
`186`	`191`	`}`
`187`	`192`
`188`	`193`	`i.chain = chain`
Original file line number	Diff line number	Diff line change
`@@ -637,7 +637,7 @@ func (c errCode) Error() string {`
`637`	`637`	`if cmsg == nil {`
`638`	`638`	`return fmt.Sprintf("Error %X", int(c))`
`639`	`639`	`}`
`640`		`- defer C.LocalFree(C.HLOCAL(cmsg))`
	`640`	`+ defer C.LocalFree(C.HLOCAL(unsafe.Pointer(cmsg)))`
`641`	`641`
`642`	`642`	`gomsg := C.GoString(cmsg)`
`643`	`643`