Merge pull request #693 from rod-hynes/master

Add code vetting CI checks and fixes

.github/workflows/tests.yml

@@ -16,7 +16,7 @@ jobs:
       matrix:
         os: [ "ubuntu" ]
         go: [ "1.22.4" ]
-        test-type: [ "detector", "coverage", "memory", "custom-build-tags" ]
+        test-type: [ "detector", "coverage", "memory", "custom-build-tags", "code-vetting" ]
 
     runs-on: ${{ matrix.os }}-latest
 
@@ -152,3 +152,39 @@ jobs:
           go build -a -v -tags "PSIPHON_DISABLE_QUIC"
           go build -a -v -tags "PSIPHON_DISABLE_GQUIC"
           go build -a -v -tags "PSIPHON_ENABLE_REFRACTION_NETWORKING"
+
+      - name: Check Go fmt
+        if: ${{ matrix.test-type == 'code-vetting' }}
+        run: |
+          cd ${{ github.workspace }}/go/src/github.com/Psiphon-Labs/psiphon-tunnel-core
+          if [ "$(gofmt -s -l ./psiphon ./ClientLibrary ./ConsoleClient ./MobileLibrary/psi ./Server | wc -l)" -gt 0 ]; then
+            gofmt -s -l ./psiphon ./ClientLibrary ./ConsoleClient ./MobileLibrary/psi ./Server
+            exit 1
+          fi
+
+      - name: Check Go vet
+        if: ${{ matrix.test-type == 'code-vetting' }}
+        run: |
+          cd ${{ github.workspace }}/go/src/github.com/Psiphon-Labs/psiphon-tunnel-core
+          go vet -tags "PSIPHON_ENABLE_INPROXY PSIPHON_ENABLE_REFRACTION_NETWORKING" ./psiphon/... ./ClientLibrary/... ./ConsoleClient/... ./MobileLibrary/psi ./Server/...
+
+      # License check ignore cases:
+      #
+      # - github.com/Psiphon-Labs,github.com/Psiphon-Inc: Psiphon code with
+      #   GPL 3 license; any dependencies within (subtree or copy) must be
+      #   manually vetted
+      #
+      # - golang.org/x,filippo.io/edwards25519,github.com/klauspost/compress:
+      #   fail with "contains non-Go code that can't be inspected for further
+      #   dependencies"; manually vetted
+      #
+      # - github.com/oschwald/maxminddb-golang,github.com/shoenig/go-m1cpu:
+      #   ISC and MPL-2.0 respectively; allowed for server only (there is an
+      #   extra pass, without this exclusion, over the main client packages)
+      #
+      - name: Check licenses
+        if: ${{ matrix.test-type == 'code-vetting' }}
+        run: |
+          cd ${{ github.workspace }}/go/src/github.com/Psiphon-Labs/psiphon-tunnel-core
+          go run github.com/google/go-licenses@latest check --ignore=github.com/Psiphon-Labs,github.com/Psiphon-Inc,golang.org/x,filippo.io/edwards25519,github.com/klauspost/compress,github.com/oschwald/maxminddb-golang,github.com/shoenig/go-m1cpu --allowed_licenses=Apache-2.0,Apache-3,BSD-2-Clause,BSD-3-Clause,BSD-4-Clause,CC0-1.0,MIT ./...
+          go run github.com/google/go-licenses@latest check --ignore=github.com/Psiphon-Labs,github.com/Psiphon-Inc,golang.org/x,filippo.io/edwards25519,github.com/klauspost/compress --allowed_licenses=Apache-2.0,Apache-3,BSD-2-Clause,BSD-3-Clause,BSD-4-Clause,CC0-1.0,MIT ./psiphon ./psiphon/common/... ./ClientLibrary/... ./ConsoleClient/... ./MobileLibrary/psi

ClientLibrary/PsiphonTunnel.go

@@ -90,8 +90,6 @@ var tunnel *clientlib.PsiphonTunnel
 // Memory managed by PsiphonTunnel which is allocated in Start and freed in Stop
 var managedStartResult *C.char
 
-//export PsiphonTunnelStart
-//
 // ******************************* WARNING ********************************
 // The underlying memory referenced by the return value of Start is managed
 // by PsiphonTunnel and attempting to free it explicitly will cause the
@@ -107,59 +105,66 @@ var managedStartResult *C.char
 // null-terminated buffer of C chars.
 // Start will return,
 // On success:
-//   {
-//     "Code": 0,
-//     "ConnectTimeMS": <milliseconds to establish tunnel>,
-//     "HTTPProxyPort": <http proxy port number>,
-//     "SOCKSProxyPort": <socks proxy port number>
-//   }
+//
+//	{
+//	  "Code": 0,
+//	  "ConnectTimeMS": <milliseconds to establish tunnel>,
+//	  "HTTPProxyPort": <http proxy port number>,
+//	  "SOCKSProxyPort": <socks proxy port number>
+//	}
 //
 // On timeout:
-//   {
-//     "Code": 1,
-//     "Error": <error message>
-//   }
+//
+//	{
+//	  "Code": 1,
+//	  "Error": <error message>
+//	}
 //
 // On other error:
-//   {
-//     "Code": 2,
-//     "Error": <error message>
-//   }
+//
+//	{
+//	  "Code": 2,
+//	  "Error": <error message>
+//	}
 //
 // Parameters.clientPlatform should be of the form OS_OSVersion_BundleIdentifier where
 // both the OSVersion and BundleIdentifier fields are optional. If clientPlatform is set
 // to an empty string the "ClientPlatform" field in the provided JSON config will be
 // used instead.
 //
 // Provided below are links to platform specific code which can be used to find some of the above fields:
-//   Android:
-//     - OSVersion: https://github.com/Psiphon-Labs/psiphon-tunnel-core/blob/3d344194d21b250e0f18ededa4b4459a373b0690/MobileLibrary/Android/PsiphonTunnel/PsiphonTunnel.java#L573
-//     - BundleIdentifier: https://github.com/Psiphon-Labs/psiphon-tunnel-core/blob/3d344194d21b250e0f18ededa4b4459a373b0690/MobileLibrary/Android/PsiphonTunnel/PsiphonTunnel.java#L575
-//   iOS:
-//     - OSVersion: https://github.com/Psiphon-Labs/psiphon-tunnel-core/blob/3d344194d21b250e0f18ededa4b4459a373b0690/MobileLibrary/iOS/PsiphonTunnel/PsiphonTunnel/PsiphonTunnel.m#L612
-//     - BundleIdentifier: https://github.com/Psiphon-Labs/psiphon-tunnel-core/blob/3d344194d21b250e0f18ededa4b4459a373b0690/MobileLibrary/iOS/PsiphonTunnel/PsiphonTunnel/PsiphonTunnel.m#L622
+//
+//	Android:
+//	  - OSVersion: https://github.com/Psiphon-Labs/psiphon-tunnel-core/blob/3d344194d21b250e0f18ededa4b4459a373b0690/MobileLibrary/Android/PsiphonTunnel/PsiphonTunnel.java#L573
+//	  - BundleIdentifier: https://github.com/Psiphon-Labs/psiphon-tunnel-core/blob/3d344194d21b250e0f18ededa4b4459a373b0690/MobileLibrary/Android/PsiphonTunnel/PsiphonTunnel.java#L575
+//	iOS:
+//	  - OSVersion: https://github.com/Psiphon-Labs/psiphon-tunnel-core/blob/3d344194d21b250e0f18ededa4b4459a373b0690/MobileLibrary/iOS/PsiphonTunnel/PsiphonTunnel/PsiphonTunnel.m#L612
+//	  - BundleIdentifier: https://github.com/Psiphon-Labs/psiphon-tunnel-core/blob/3d344194d21b250e0f18ededa4b4459a373b0690/MobileLibrary/iOS/PsiphonTunnel/PsiphonTunnel/PsiphonTunnel.m#L622
 //
 // Some examples of valid client platform strings are:
 //
-//   "Android_4.2.2_com.example.exampleApp"
-//   "iOS_11.4_com.example.exampleApp"
-//   "Windows"
+//	"Android_4.2.2_com.example.exampleApp"
+//	"iOS_11.4_com.example.exampleApp"
+//	"Windows"
 //
 // Parameters.networkID must be a non-empty string and follow the format specified by:
 // https://godoc.org/github.com/Psiphon-Labs/psiphon-tunnel-core/psiphon#NetworkIDGetter.
 // Provided below are links to platform specific code which can be used to generate
 // valid network identifier strings:
-//   Android:
-//     - https://github.com/Psiphon-Labs/psiphon-tunnel-core/blob/3d344194d21b250e0f18ededa4b4459a373b0690/MobileLibrary/Android/PsiphonTunnel/PsiphonTunnel.java#L371
-//   iOS:
-//     - https://github.com/Psiphon-Labs/psiphon-tunnel-core/blob/3d344194d21b250e0f18ededa4b4459a373b0690/MobileLibrary/iOS/PsiphonTunnel/PsiphonTunnel/PsiphonTunnel.m#L1105
+//
+//	Android:
+//	  - https://github.com/Psiphon-Labs/psiphon-tunnel-core/blob/3d344194d21b250e0f18ededa4b4459a373b0690/MobileLibrary/Android/PsiphonTunnel/PsiphonTunnel.java#L371
+//	iOS:
+//	  - https://github.com/Psiphon-Labs/psiphon-tunnel-core/blob/3d344194d21b250e0f18ededa4b4459a373b0690/MobileLibrary/iOS/PsiphonTunnel/PsiphonTunnel/PsiphonTunnel.m#L1105
 //
 // Parameters.establishTunnelTimeoutSeconds specifies a time limit after which to stop
 // attempting to connect and return an error if an active tunnel has not been established.
 // A timeout of 0 will result in no timeout condition and the controller will attempt to
 // establish an active tunnel indefinitely (or until PsiphonTunnelStop is called).
 // Timeout values >= 0 override the optional `EstablishTunnelTimeoutSeconds` config field;
 // null causes the config value to be used.
+//
+//export PsiphonTunnelStart
 func PsiphonTunnelStart(cConfigJSON, cEmbeddedServerEntryList *C.char, cParams *C.struct_Parameters) *C.char {
 	// Stop any active tunnels
 	PsiphonTunnelStop()
@@ -243,13 +248,13 @@ func PsiphonTunnelStart(cConfigJSON, cEmbeddedServerEntryList *C.char, cParams *
 	return managedStartResult
 }
 
-//export PsiphonTunnelStop
-//
 // Stop stops the controller if it is running and waits for it to clean up and exit.
 //
 // Stop should always be called after a successful call to Start to ensure the
 // controller is not left running and memory is released.
 // It is safe to call this function when the tunnel is not running.
+//
+//export PsiphonTunnelStop
 func PsiphonTunnelStop() {
 	freeManagedStartResult()
 	if tunnel != nil {
@@ -277,10 +282,11 @@ func marshalStartResult(result startResult) *C.char {
 // provided error.
 //
 // The JSON will be in the form of:
-// {
-//   "Code": 2,
-//   "Error": <error message>
-// }
+//
+//	{
+//	  "Code": 2,
+//	  "Error": <error message>
+//	}
 func startErrorJSON(err error) *C.char {
 	var result startResult
 	result.Code = startResultCodeOtherError

ConsoleClient/signal.go

@@ -1,3 +1,4 @@
+//go:build !windows
 // +build !windows
 
 /*

psiphon/common/accesscontrol/accesscontrol_test.go

@@ -75,7 +75,7 @@ func TestAuthorization(t *testing.T) {
 	// Test: invalid key ring
 
 	invalidKeyRing := &VerificationKeyRing{
-		Keys: []*VerificationKey{&VerificationKey{}},
+		Keys: []*VerificationKey{{}},
 	}
 
 	err = ValidateVerificationKeyRing(invalidKeyRing)

psiphon/common/crypto/Yawning/chacha20/chacha20_amd64.go

@@ -5,6 +5,7 @@
 // Commons "CC0" public domain dedication. See LICENSE or
 // <http://creativecommons.org/publicdomain/zero/1.0/> for full details.
 
+//go:build amd64 && !gccgo && !appengine
 // +build amd64,!gccgo,!appengine
 
 package chacha20

psiphon/common/crypto/Yawning/chacha20/chacha20_ref.go

@@ -5,6 +5,7 @@
 // Commons "CC0" public domain dedication. See LICENSE or
 // <http://creativecommons.org/publicdomain/zero/1.0/> for full details.
 
+//go:build !go1.9
 // +build !go1.9
 
 package chacha20

psiphon/common/crypto/Yawning/chacha20/chacha20_ref_go19.go

@@ -5,6 +5,7 @@
 // Commons "CC0" public domain dedication. See LICENSE or
 // <http://creativecommons.org/publicdomain/zero/1.0/> for full details.
 
+//go:build go1.9
 // +build go1.9
 
 package chacha20

psiphon/common/crypto/internal/poly1305/sum_generic.go

@@ -136,7 +136,7 @@ func shiftRightBy2(a uint128) uint128 {
 // updateGeneric absorbs msg into the state.h accumulator. For each chunk m of
 // 128 bits of message, it computes
 //
-//     h₊ = (h + m) * r  mod  2¹³⁰ - 5
+//	h₊ = (h + m) * r  mod  2¹³⁰ - 5
 //
 // If the msg length is not a multiple of TagSize, it assumes the last
 // incomplete chunk is the final one.
@@ -278,8 +278,7 @@ const (
 
 // finalize completes the modular reduction of h and computes
 //
-//     out = h + s  mod  2¹²⁸
-//
+//	out = h + s  mod  2¹²⁸
 func finalize(out *[TagSize]byte, h *[3]uint64, s *[2]uint64) {
 	h0, h1, h2 := h[0], h[1], h[2]
 

psiphon/common/crypto/internal/poly1305/sum_s390x.go

@@ -14,6 +14,7 @@ import (
 // updateVX is an assembly implementation of Poly1305 that uses vector
 // instructions. It must only be called if the vector facility (vx) is
 // available.
+//
 //go:noescape
 func updateVX(state *macState, msg []byte)
 

psiphon/common/crypto/internal/subtle/aliasing_appengine.go

@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
+//go:build appengine
 // +build appengine
 
 // Package subtle implements functions that are often useful in cryptographic

psiphon/common/errors/errors.go

@@ -18,10 +18,8 @@
  */
 
 /*
-
 Package errors provides error wrapping helpers that add inline, single frame
 stack trace information to error messages.
-
 */
 package errors
 

psiphon/common/inproxy/discoverySTUN.go

@@ -115,11 +115,8 @@ func discoverNATMapping(
 	response3Fields := parseSTUNMessage(response3)
 	if response3Fields.xorAddr.String() == response2Fields.xorAddr.String() {
 		return NATMappingAddressDependent, nil
-	} else {
-		return NATMappingAddressPortDependent, nil
 	}
-
-	return NATMappingUnknown, nil
+	return NATMappingAddressPortDependent, nil
 }
 
 // RFC5780: 4.4.  Determining NAT Filtering Behavior

psiphon/common/networkConfig_other.go

@@ -1,3 +1,4 @@
+//go:build !linux
 // +build !linux
 
 /*

psiphon/common/packetman/packetman.go

@@ -23,17 +23,17 @@ variety of strategies to evade network censorship.
 
 This implementation is entirely based on and is a subset of Geneva:
 
-  Come as You Are: Helping Unmodified Clients Bypass Censorship with
-  Server-side Evasion
-  Kevin Bock, George Hughey, Louis-Henri Merino, Tania Arya, Daniel Liscinsky,
-  Regina Pogosian, Dave Levin
-  ACM SIGCOMM 2020
+	Come as You Are: Helping Unmodified Clients Bypass Censorship with
+	Server-side Evasion
+	Kevin Bock, George Hughey, Louis-Henri Merino, Tania Arya, Daniel Liscinsky,
+	Regina Pogosian, Dave Levin
+	ACM SIGCOMM 2020
 
-  Geneva: Evolving Censorship Evasion Strategies
-  Kevin Bock, George Hughey, Xiao Qiang, Dave Levin
-  ACM CCS 2019 (Conference on Computer and Communications Security)
+	Geneva: Evolving Censorship Evasion Strategies
+	Kevin Bock, George Hughey, Xiao Qiang, Dave Levin
+	ACM CCS 2019 (Conference on Computer and Communications Security)
 
-  https://github.com/Kkevsterrr/geneva
+	https://github.com/Kkevsterrr/geneva
 
 This package implements the equivilent of the Geneva "engine", which can
 execute packet manipulation strategies. It does not implement the genetic
@@ -57,7 +57,6 @@ Security: external parties can induce the server to emit a SYN-ACK, invoking
 the packet manipulation logic. External parties cannot set the transformation
 specs, and, as the input is the server-side generated SYN-ACK packet, cannot
 influence the packet manipulation with any external input parameters.
-
 */
 package packetman
 
@@ -144,11 +143,12 @@ type Config struct {
 // "TCP-payload random|<hex>"
 //
 // For example, this Geneva strategy:
-//   [TCP:flags:SA]-duplicate(tamper{TCP:flags:replace:R},tamper{TCP:flags:replace:S})-| \/
+//
+//	[TCP:flags:SA]-duplicate(tamper{TCP:flags:replace:R},tamper{TCP:flags:replace:S})-| \/
 //
 // is represented as follows (in JSON encoding):
-//   [["TCP-flags R"], ["TCP-flags S"]]
 //
+//	[["TCP-flags R"], ["TCP-flags S"]]
 //
 // Field and option values must be the expected length (see implementation).
 //
@@ -173,7 +173,7 @@ type compiledSpec struct {
 func compileSpec(spec *Spec) (*compiledSpec, error) {
 
 	compiledPacketSpecs := make([][]transformation, len(spec.PacketSpecs))
-	for i, _ := range spec.PacketSpecs {
+	for i := range spec.PacketSpecs {
 		compiledPacketSpecs[i] = make([]transformation, len(spec.PacketSpecs[i]))
 		for j, transformationSpec := range spec.PacketSpecs[i] {
 			transform, err := compileTransformation(transformationSpec)

psiphon/common/packetman/packetman_linux_test.go

@@ -85,7 +85,7 @@ func testPacketManipulator(useIPv6 bool, t *testing.T) {
 	config := &Config{
 		Logger:        newTestLogger(),
 		ProtocolPorts: []int{listenerPort},
-		Specs:         []*Spec{&Spec{Name: testSpecName, PacketSpecs: [][]string{[]string{"TCP-flags S"}}}},
+		Specs:         []*Spec{{Name: testSpecName, PacketSpecs: [][]string{{"TCP-flags S"}}}},
 		SelectSpecName: func(protocolPort int, _ net.IP) (string, interface{}) {
 			if protocolPort == listenerPort {
 				return testSpecName, extraDataValue

psiphon/common/packetman/packetman_test.go

@@ -110,10 +110,10 @@ func TestTransformations(t *testing.T) {
 		SYN: true,
 		ACK: true,
 		Options: []layers.TCPOption{
-			layers.TCPOption{OptionType: layers.TCPOptionKindNop, OptionLength: 1},
-			layers.TCPOption{OptionType: layers.TCPOptionKindSACKPermitted, OptionLength: 2},
-			layers.TCPOption{OptionType: layers.TCPOptionKindSACK, OptionLength: 10, OptionData: bytes.Repeat([]byte{0}, 8)},
-			layers.TCPOption{OptionType: layers.TCPOptionKindTimestamps, OptionLength: 10, OptionData: bytes.Repeat([]byte{0}, 8)},
+			{OptionType: layers.TCPOptionKindNop, OptionLength: 1},
+			{OptionType: layers.TCPOptionKindSACKPermitted, OptionLength: 2},
+			{OptionType: layers.TCPOptionKindSACK, OptionLength: 10, OptionData: bytes.Repeat([]byte{0}, 8)},
+			{OptionType: layers.TCPOptionKindTimestamps, OptionLength: 10, OptionData: bytes.Repeat([]byte{0}, 8)},
 		},
 	}
 
@@ -219,12 +219,12 @@ repeatLoop:
 				// omitted.
 
 				expectedOptions := []layers.TCPOption{
-					layers.TCPOption{OptionType: layers.TCPOptionKindSACKPermitted, OptionLength: 2},
-					layers.TCPOption{OptionType: layers.TCPOptionKindSACK, OptionLength: 10, OptionData: bytes.Repeat([]byte{0xff}, 8)},
-					layers.TCPOption{OptionType: layers.TCPOptionKindTimestamps, OptionLength: 10, OptionData: bytes.Repeat([]byte{0xff}, 8)},
-					layers.TCPOption{OptionType: layers.TCPOptionKindMSS, OptionLength: 4, OptionData: bytes.Repeat([]byte{0xff}, 2)},
-					layers.TCPOption{OptionType: layers.TCPOptionKindWindowScale, OptionLength: 3, OptionData: bytes.Repeat([]byte{0xff}, 1)},
-					layers.TCPOption{OptionType: layers.TCPOptionKindEndList, OptionLength: 1},
+					{OptionType: layers.TCPOptionKindSACKPermitted, OptionLength: 2},
+					{OptionType: layers.TCPOptionKindSACK, OptionLength: 10, OptionData: bytes.Repeat([]byte{0xff}, 8)},
+					{OptionType: layers.TCPOptionKindTimestamps, OptionLength: 10, OptionData: bytes.Repeat([]byte{0xff}, 8)},
+					{OptionType: layers.TCPOptionKindMSS, OptionLength: 4, OptionData: bytes.Repeat([]byte{0xff}, 2)},
+					{OptionType: layers.TCPOptionKindWindowScale, OptionLength: 3, OptionData: bytes.Repeat([]byte{0xff}, 1)},
+					{OptionType: layers.TCPOptionKindEndList, OptionLength: 1},
 				}
 
 				if tcp.SrcPort != 0xffff ||

psiphon/common/packetman/packetman_unsupported.go

@@ -1,3 +1,4 @@
+//go:build !linux
 // +build !linux
 
 /*