diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml new file mode 100644 index 000000000..3634cd38b --- /dev/null +++ b/.github/workflows/codeql.yml @@ -0,0 +1,88 @@ +name: "CodeQL" + +on: + push: + branches: [ "main" ] + pull_request: + branches: [ "main" ] + schedule: + - cron: '43 11 * * 6' + workflow_dispatch: + +jobs: + analyze: + name: Analyze (${{ matrix.language }}) + # Runner size impacts CodeQL analysis time. To learn more, please see: + # - https://gh.io/recommended-hardware-resources-for-running-codeql + # - https://gh.io/supported-runners-and-hardware-resources + # - https://gh.io/using-larger-runners (GitHub.com only) + # Consider using larger runners or machines with greater resources for possible analysis time improvements. + runs-on: ${{ (matrix.language == 'swift' && 'macos-latest') || 'ubuntu-latest' }} + permissions: + # required for all workflows + security-events: write + + # required to fetch internal or private CodeQL packs + packages: read + + # only required for workflows in private repositories + actions: read + contents: read + + strategy: + fail-fast: false + matrix: + include: + - language: go + build-mode: autobuild + # CodeQL supports the following values keywords for 'language': 'c-cpp', 'csharp', 'go', 'java-kotlin', 'javascript-typescript', 'python', 'ruby', 'swift' + # Use `c-cpp` to analyze code written in C, C++ or both + # Use 'java-kotlin' to analyze code written in Java, Kotlin or both + # Use 'javascript-typescript' to analyze code written in JavaScript, TypeScript or both + # To learn more about changing the languages that are analyzed or customizing the build mode for your analysis, + # see https://docs.github.com/en/code-security/code-scanning/creating-an-advanced-setup-for-code-scanning/customizing-your-advanced-setup-for-code-scanning. + # If you are analyzing a compiled language, you can modify the 'build-mode' for that language to customize how + # your codebase is analyzed, see https://docs.github.com/en/code-security/code-scanning/creating-an-advanced-setup-for-code-scanning/codeql-code-scanning-for-compiled-languages + steps: + - name: Checkout repository + uses: actions/checkout@v4 + + # Add any setup steps before running the `github/codeql-action/init` action. + # This includes steps like installing compilers or runtimes (`actions/setup-node` + # or others). This is typically only required for manual builds. + # - name: Setup runtime (example) + # uses: actions/setup-example@v1 + + # Initializes the CodeQL tools for scanning. + - name: Initialize CodeQL + uses: github/codeql-action/init@v3 + with: + languages: ${{ matrix.language }} + build-mode: ${{ matrix.build-mode }} + # If you wish to specify custom queries, you can do so here or in a config file. + # By default, queries listed here will override any specified in a config file. + # Prefix the list here with "+" to use these queries and those in the config file. + + # For more details on CodeQL's query packs, refer to: https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/configuring-code-scanning#using-queries-in-ql-packs + # queries: security-extended,security-and-quality + + # If the analyze step fails for one of the languages you are analyzing with + # "We were unable to automatically build your code", modify the matrix above + # to set the build mode to "manual" for that language. Then modify this step + # to build your code. + # ℹī¸ Command-line programs to run using the OS shell. + # 📚 See https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idstepsrun + - if: matrix.build-mode == 'manual' + shell: bash + run: | + echo 'If you are using a "manual" build mode for one or more of the' \ + 'languages you are analyzing, replace this with the commands to build' \ + 'your code, for example:' + echo ' make bootstrap' + echo ' make release' + exit 1 + + - name: Perform CodeQL Analysis + uses: github/codeql-action/analyze@v3 + with: + category: "/language:${{matrix.language}}" diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 1a05c9196..e7c72e84d 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -16,7 +16,7 @@ jobs: - name: Set up Go latest uses: actions/setup-go@v3 with: - go-version: ^1.18 + go-version: ^1.22 - name: Short test run: go test -short -v ./... @@ -26,22 +26,3 @@ jobs: - name: Randomized test suite 2 run: go test -v ./... -run RandomizeSlow -count=32 - - test-old: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v3 - - - name: Set up Go 1.17 - uses: actions/setup-go@v3 - with: - go-version: 1.17 - - - name: Short test - run: go test -short -v ./... - - - name: Randomized test suite 1 - run: go test -v ./... -run RandomizeFast -count=512 - - - name: Randomized test suite 2 - run: go test -v ./... -run RandomizeSlow -count=32 \ No newline at end of file diff --git a/.github/workflows/interop-test-suite.yml b/.github/workflows/interop-test-suite.yml index 6e3077cf9..fb0960d40 100644 --- a/.github/workflows/interop-test-suite.yml +++ b/.github/workflows/interop-test-suite.yml @@ -18,7 +18,7 @@ jobs: branch-gosop: gosop-gopenpgp-v2 # Upload as artifact - name: Upload gosop artifact - uses: actions/upload-artifact@v3 + uses: actions/upload-artifact@v4 with: name: gosop-${{ github.sha }}-v1 path: ./gosop-${{ github.sha }}-v1 @@ -37,7 +37,7 @@ jobs: gosop-build-path: build_gosop.sh # Upload as artifact - name: Upload gosop artifact - uses: actions/upload-artifact@v3 + uses: actions/upload-artifact@v4 with: name: gosop-${{ github.sha }}-v2 path: ./gosop-${{ github.sha }}-v2 @@ -56,7 +56,7 @@ jobs: binary-location: ./gosop-main-v1 # Upload as artifact - name: Upload gosop-main artifact - uses: actions/upload-artifact@v3 + uses: actions/upload-artifact@v4 with: name: gosop-main-v1 path: ./gosop-main-v1 @@ -76,7 +76,7 @@ jobs: gosop-build-path: build_gosop.sh # Upload as artifact - name: Upload gosop-main artifact - uses: actions/upload-artifact@v3 + uses: actions/upload-artifact@v4 with: name: gosop-main-v2 path: ./gosop-main-v2 @@ -99,7 +99,7 @@ jobs: uses: actions/checkout@v3 # Fetch gosop from main v1 - name: Download gosop-main-v1 - uses: actions/download-artifact@v3 + uses: actions/download-artifact@v4 with: name: gosop-main-v1 # Test gosop-main-v1 @@ -109,7 +109,7 @@ jobs: run: ./gosop-main-v1 version --extended # Fetch gosop from main v2 - name: Download gosop-main-v2 - uses: actions/download-artifact@v3 + uses: actions/download-artifact@v4 with: name: gosop-main-v2 # Test gosop-main-v2 @@ -119,7 +119,7 @@ jobs: run: ./gosop-main-v2 version --extended # Fetch gosop from branch v1 - name: Download gosop-branch-v1 - uses: actions/download-artifact@v3 + uses: actions/download-artifact@v4 with: name: gosop-${{ github.sha }}-v1 - name: Rename gosop-branch-v1 @@ -131,7 +131,7 @@ jobs: run: ./gosop-branch-v1 version --extended # Fetch gosop from branch v2 - name: Download gosop-branch-v2 - uses: actions/download-artifact@v3 + uses: actions/download-artifact@v4 with: name: gosop-${{ github.sha }}-v2 - name: Rename gosop-branch-v2 @@ -157,12 +157,12 @@ jobs: RESULTS_HTML: .github/test-suite/test-suite-results.html # Upload results - name: Upload test results json artifact - uses: actions/upload-artifact@v3 + uses: actions/upload-artifact@v4 with: name: test-suite-results.json path: .github/test-suite/test-suite-results.json - name: Upload test results html artifact - uses: actions/upload-artifact@v3 + uses: actions/upload-artifact@v4 with: name: test-suite-results.html path: .github/test-suite/test-suite-results.html @@ -176,18 +176,18 @@ jobs: uses: actions/checkout@v3 - name: Download test results json artifact id: download-test-results - uses: actions/download-artifact@v3 + uses: actions/download-artifact@v4 with: name: test-suite-results.json - name: Compare with baseline v1 - uses: ProtonMail/openpgp-interop-test-analyzer@5d7f4b6868ebe3bfc909302828342c461f5f4940 + uses: ProtonMail/openpgp-interop-test-analyzer@v2.1.0 with: results: ${{ steps.download-test-results.outputs.download-path }}/test-suite-results.json output: baseline-comparison-v1.json baseline: gosop-main-v1 target: gosop-branch-v1 - name: Compare with baseline v2 - uses: ProtonMail/openpgp-interop-test-analyzer@5d7f4b6868ebe3bfc909302828342c461f5f4940 + uses: ProtonMail/openpgp-interop-test-analyzer@v2.1.0 with: results: ${{ steps.download-test-results.outputs.download-path }}/test-suite-results.json output: baseline-comparison-v2.json diff --git a/go.mod b/go.mod index d417da35c..e8095fa78 100644 --- a/go.mod +++ b/go.mod @@ -1,10 +1,10 @@ module github.com/ProtonMail/go-crypto -go 1.17 +go 1.22.0 require ( - github.com/cloudflare/circl v1.3.7 - golang.org/x/crypto v0.17.0 + github.com/cloudflare/circl v1.6.0 + golang.org/x/crypto v0.33.0 ) -require golang.org/x/sys v0.16.0 // indirect +require golang.org/x/sys v0.30.0 // indirect diff --git a/go.sum b/go.sum index 712b2d44b..03abcbcec 100644 --- a/go.sum +++ b/go.sum @@ -1,44 +1,6 @@ -github.com/bwesterb/go-ristretto v1.2.3/go.mod h1:fUIoIZaG73pV5biE2Blr2xEzDoMj7NFEuV9ekS419A0= -github.com/cloudflare/circl v1.3.7 h1:qlCDlTPz2n9fu58M0Nh1J/JzcFpfgkFHHX3O35r5vcU= -github.com/cloudflare/circl v1.3.7/go.mod h1:sRTcRWXGLrKw6yIGJ+l7amYJFfAXbZG0kBSc8r4zxgA= -github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY= -golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= -golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= -golang.org/x/crypto v0.17.0 h1:r8bRNjWL3GshPW3gkd+RpvzWrZAwPS49OmTGZ/uhM4k= -golang.org/x/crypto v0.17.0/go.mod h1:gCAAfMLgwOJRpTjQ2zCCt2OcSfYMTeZVSRtQlPC7Nq4= -golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= -golang.org/x/mod v0.8.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= -golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= -golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= -golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c= -golang.org/x/net v0.6.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs= -golang.org/x/net v0.10.0/go.mod h1:0qNGK6F8kojg2nk9dLZ2mShWaEBan6FAoqfSigmmuDg= -golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sync v0.1.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= -golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.15.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= -golang.org/x/sys v0.16.0 h1:xWw16ngr6ZMtmxDyKyIgsE93KNKz5HKmMa3b8ALHidU= -golang.org/x/sys v0.16.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= -golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= -golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= -golang.org/x/term v0.5.0/go.mod h1:jMB1sMXY+tzblOD4FWmEbocvup2/aLOaQEp7JmGp78k= -golang.org/x/term v0.8.0/go.mod h1:xPskH00ivmX89bAKVGSKKtLOWNx2+17Eiy94tnKShWo= -golang.org/x/term v0.15.0/go.mod h1:BDl952bC7+uMoWR75FIrCDx79TPU9oHkTZ9yRbYOrX0= -golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= -golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= -golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= -golang.org/x/text v0.7.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8= -golang.org/x/text v0.9.0/go.mod h1:e1OnstbJyHTd6l/uOt8jFFHp6TRDWZR/bV3emEE/zU8= -golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU= -golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= -golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= -golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= -golang.org/x/tools v0.6.0/go.mod h1:Xwgl3UAJ/d3gWutnCtw505GrjyAbvKui8lOU390QaIU= -golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +github.com/cloudflare/circl v1.6.0 h1:cr5JKic4HI+LkINy2lg3W2jF8sHCVTBncJr5gIIq7qk= +github.com/cloudflare/circl v1.6.0/go.mod h1:uddAzsPgqdMAYatqJ0lsjX1oECcQLIlRpzZh3pJrofs= +golang.org/x/crypto v0.33.0 h1:IOBPskki6Lysi0lo9qQvbxiQ+FvsCC/YWOecCHAixus= +golang.org/x/crypto v0.33.0/go.mod h1:bVdXmD7IV/4GdElGPozy6U7lWdRXA4qyRVGJV57uQ5M= +golang.org/x/sys v0.30.0 h1:QjkSwP/36a20jFYWkSue1YwXzLmsV5Gfq7Eiy72C1uc= +golang.org/x/sys v0.30.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= diff --git a/internal/byteutil/byteutil.go b/internal/byteutil/byteutil.go index affb74a76..d558b9bd8 100644 --- a/internal/byteutil/byteutil.go +++ b/internal/byteutil/byteutil.go @@ -49,16 +49,16 @@ func ShiftNBytesLeft(dst, x []byte, n int) { dst = append(dst, make([]byte, n/8)...) } -// XorBytesMut assumes equal input length, replaces X with X XOR Y +// XorBytesMut replaces X with X XOR Y. len(X) must be >= len(Y). func XorBytesMut(X, Y []byte) { - for i := 0; i < len(X); i++ { + for i := 0; i < len(Y); i++ { X[i] ^= Y[i] } } -// XorBytes assumes equal input length, puts X XOR Y into Z +// XorBytes puts X XOR Y into Z. len(Z) and len(X) must be >= len(Y). func XorBytes(Z, X, Y []byte) { - for i := 0; i < len(X); i++ { + for i := 0; i < len(Y); i++ { Z[i] = X[i] ^ Y[i] } } diff --git a/ocb/ocb.go b/ocb/ocb.go index 5022285b4..24f893017 100644 --- a/ocb/ocb.go +++ b/ocb/ocb.go @@ -109,8 +109,10 @@ func (o *ocb) Seal(dst, nonce, plaintext, adata []byte) []byte { if len(nonce) > o.nonceSize { panic("crypto/ocb: Incorrect nonce length given to OCB") } - ret, out := byteutil.SliceForAppend(dst, len(plaintext)+o.tagSize) - o.crypt(enc, out, nonce, adata, plaintext) + sep := len(plaintext) + ret, out := byteutil.SliceForAppend(dst, sep+o.tagSize) + tag := o.crypt(enc, out[:sep], nonce, adata, plaintext) + copy(out[sep:], tag) return ret } @@ -122,12 +124,10 @@ func (o *ocb) Open(dst, nonce, ciphertext, adata []byte) ([]byte, error) { return nil, ocbError("Ciphertext shorter than tag length") } sep := len(ciphertext) - o.tagSize - ret, out := byteutil.SliceForAppend(dst, len(ciphertext)) + ret, out := byteutil.SliceForAppend(dst, sep) ciphertextData := ciphertext[:sep] - tag := ciphertext[sep:] - o.crypt(dec, out, nonce, adata, ciphertextData) - if subtle.ConstantTimeCompare(ret[sep:], tag) == 1 { - ret = ret[:sep] + tag := o.crypt(dec, out, nonce, adata, ciphertextData) + if subtle.ConstantTimeCompare(tag, ciphertext[sep:]) == 1 { return ret, nil } for i := range out { @@ -137,7 +137,8 @@ func (o *ocb) Open(dst, nonce, ciphertext, adata []byte) ([]byte, error) { } // On instruction enc (resp. dec), crypt is the encrypt (resp. decrypt) -// function. It returns the resulting plain/ciphertext with the tag appended. +// function. It writes the resulting plain/ciphertext into Y and returns +// the tag. func (o *ocb) crypt(instruction int, Y, nonce, adata, X []byte) []byte { // // Consider X as a sequence of 128-bit blocks @@ -194,13 +195,14 @@ func (o *ocb) crypt(instruction int, Y, nonce, adata, X []byte) []byte { byteutil.XorBytesMut(offset, o.mask.L[bits.TrailingZeros(uint(i+1))]) blockX := X[i*blockSize : (i+1)*blockSize] blockY := Y[i*blockSize : (i+1)*blockSize] - byteutil.XorBytes(blockY, blockX, offset) switch instruction { case enc: + byteutil.XorBytesMut(checksum, blockX) + byteutil.XorBytes(blockY, blockX, offset) o.block.Encrypt(blockY, blockY) byteutil.XorBytesMut(blockY, offset) - byteutil.XorBytesMut(checksum, blockX) case dec: + byteutil.XorBytes(blockY, blockX, offset) o.block.Decrypt(blockY, blockY) byteutil.XorBytesMut(blockY, offset) byteutil.XorBytesMut(checksum, blockY) @@ -216,31 +218,24 @@ func (o *ocb) crypt(instruction int, Y, nonce, adata, X []byte) []byte { o.block.Encrypt(pad, offset) chunkX := X[blockSize*m:] chunkY := Y[blockSize*m : len(X)] - byteutil.XorBytes(chunkY, chunkX, pad[:len(chunkX)]) - // P_* || bit(1) || zeroes(127) - len(P_*) switch instruction { case enc: - paddedY := append(chunkX, byte(128)) - paddedY = append(paddedY, make([]byte, blockSize-len(chunkX)-1)...) - byteutil.XorBytesMut(checksum, paddedY) + byteutil.XorBytesMut(checksum, chunkX) + checksum[len(chunkX)] ^= 128 + byteutil.XorBytes(chunkY, chunkX, pad[:len(chunkX)]) + // P_* || bit(1) || zeroes(127) - len(P_*) case dec: - paddedX := append(chunkY, byte(128)) - paddedX = append(paddedX, make([]byte, blockSize-len(chunkY)-1)...) - byteutil.XorBytesMut(checksum, paddedX) + byteutil.XorBytes(chunkY, chunkX, pad[:len(chunkX)]) + // P_* || bit(1) || zeroes(127) - len(P_*) + byteutil.XorBytesMut(checksum, chunkY) + checksum[len(chunkY)] ^= 128 } - byteutil.XorBytes(tag, checksum, offset) - byteutil.XorBytesMut(tag, o.mask.lDol) - o.block.Encrypt(tag, tag) - byteutil.XorBytesMut(tag, o.hash(adata)) - copy(Y[blockSize*m+len(chunkY):], tag[:o.tagSize]) - } else { - byteutil.XorBytes(tag, checksum, offset) - byteutil.XorBytesMut(tag, o.mask.lDol) - o.block.Encrypt(tag, tag) - byteutil.XorBytesMut(tag, o.hash(adata)) - copy(Y[blockSize*m:], tag[:o.tagSize]) } - return Y + byteutil.XorBytes(tag, checksum, offset) + byteutil.XorBytesMut(tag, o.mask.lDol) + o.block.Encrypt(tag, tag) + byteutil.XorBytesMut(tag, o.hash(adata)) + return tag[:o.tagSize] } // This hash function is used to compute the tag. Per design, on empty input it diff --git a/ocb/ocb_test.go b/ocb/ocb_test.go index d3fc4cf57..974a0ac97 100644 --- a/ocb/ocb_test.go +++ b/ocb/ocb_test.go @@ -127,8 +127,20 @@ func TestEncryptDecryptRFC7253TestVectors(t *testing.T) { adata, _ := hex.DecodeString(test.header) targetPt, _ := hex.DecodeString(test.plaintext) targetCt, _ := hex.DecodeString(test.ciphertext) - ct := ocbInstance.Seal(nil, nonce, targetPt, adata) // Encrypt + ct := ocbInstance.Seal(nil, nonce, targetPt, adata) + if !bytes.Equal(ct, targetCt) { + t.Errorf( + `RFC7253 Test vectors Encrypt error (ciphertexts don't match): + Got: + %X + Want: + %X`, ct, targetCt) + } + // Encrypt reusing buffer + pt := make([]byte, len(targetPt) + ocbInstance.Overhead()) + copy(pt, targetPt) + ct = ocbInstance.Seal(pt[:0], nonce, pt[:len(targetPt)], adata) if !bytes.Equal(ct, targetCt) { t.Errorf( `RFC7253 Test vectors Encrypt error (ciphertexts don't match): @@ -138,14 +150,14 @@ func TestEncryptDecryptRFC7253TestVectors(t *testing.T) { %X`, ct, targetCt) } // Decrypt - pt, err := ocbInstance.Open(nil, nonce, targetCt, adata) + pt, err := ocbInstance.Open(nil, nonce, ct, adata) if err != nil { t.Errorf( `RFC7253 Valid ciphertext was refused decryption: plaintext %X nonce %X header %X - ciphertext %X`, targetPt, nonce, adata, targetCt) + ciphertext %X`, targetPt, nonce, adata, ct) } if !bytes.Equal(pt, targetPt) { t.Errorf( @@ -155,6 +167,24 @@ func TestEncryptDecryptRFC7253TestVectors(t *testing.T) { Want: %X`, pt, targetPt) } + // Decrypt reusing buffer + pt, err = ocbInstance.Open(ct[:0], nonce, ct, adata) + if err != nil { + t.Errorf( + `RFC7253 Valid ciphertext was refused decryption: + plaintext %X + nonce %X + header %X + ciphertext %X`, targetPt, nonce, adata, ct) + } + if !bytes.Equal(pt, targetPt) { + t.Errorf( + `RFC7253 test vectors Decrypt error (plaintexts don't match): + Got: + %X + Want: + %X`, targetPt, pt) + } } } @@ -182,7 +212,30 @@ func TestEncryptDecryptRFC7253TagLen96(t *testing.T) { Want: %X`, ct, targetCt) } - pt, err := ocbInstance.Open(nil, nonce, targetCt, adata) + pt := make([]byte, len(targetPt) + ocbInstance.Overhead()) + copy(pt, targetPt) + ct = ocbInstance.Seal(pt[:0], nonce, pt[:len(targetPt)], adata) + if !bytes.Equal(ct, targetCt) { + t.Errorf( + `RFC7253 test tagLen96 error (ciphertexts don't match): + Got: + %X + Want: + %X`, ct, targetCt) + } + pt, err = ocbInstance.Open(nil, nonce, ct, adata) + if err != nil { + t.Errorf(`RFC7253 test tagLen96 was refused decryption`) + } + if !bytes.Equal(pt, targetPt) { + t.Errorf( + `RFC7253 test tagLen96 error (plaintexts don't match): + Got: + %X + Want: + %X`, pt, targetPt) + } + pt, err = ocbInstance.Open(ct[:0], nonce, ct, adata) if err != nil { t.Errorf(`RFC7253 test tagLen96 was refused decryption`) } @@ -274,15 +327,47 @@ func TestEncryptDecryptGoTestVectors(t *testing.T) { %X`, ct, targetCt) } + // Encrypt reusing buffer + pt := make([]byte, len(targetPt) + ocbInstance.Overhead()) + copy(pt, targetPt) + ct = ocbInstance.Seal(pt[:0], nonce, pt[:len(targetPt)], adata) + if !bytes.Equal(ct, targetCt) { + t.Errorf( + `Go Test vectors Encrypt error (ciphertexts don't match): + Got: + %X + Want: + %X`, ct, targetCt) + } + // Decrypt - pt, err := ocbInstance.Open(nil, nonce, targetCt, adata) + pt, err = ocbInstance.Open(nil, nonce, ct, adata) if err != nil { t.Errorf( `Valid Go ciphertext was refused decryption: plaintext %X nonce %X header %X - ciphertext %X`, targetPt, nonce, adata, targetCt) + ciphertext %X`, targetPt, nonce, adata, ct) + } + if !bytes.Equal(pt, targetPt) { + t.Errorf( + `Go Test vectors Decrypt error (plaintexts don't match): + Got: + %X + Want: + %X`, pt, targetPt) + } + + // Decrypt reusing buffer + pt, err = ocbInstance.Open(ct[:0], nonce, ct, adata) + if err != nil { + t.Errorf( + `Valid Go ciphertext was refused decryption: + plaintext %X + nonce %X + header %X + ciphertext %X`, targetPt, nonce, adata, ct) } if !bytes.Equal(pt, targetPt) { t.Errorf( @@ -333,6 +418,17 @@ func TestEncryptDecryptVectorsWithPreviousDataRandomizeSlow(t *testing.T) { `Random Encrypt/Decrypt error (plaintexts don't match)`) break } + decrypted, err = ocb.Open(ct[:0], nonce, ct, header) + if err != nil { + t.Errorf( + `Decrypt refused valid tag (not displaying long output)`) + break + } + if !bytes.Equal(pt, decrypted) { + t.Errorf( + `Random Encrypt/Decrypt error (plaintexts don't match)`) + break + } } } @@ -369,6 +465,12 @@ func TestRejectTamperedCiphertextRandomizeSlow(t *testing.T) { "Tampered ciphertext was not refused decryption (OCB did not return an error)") return } + _, err = ocb.Open(tampered[:0], nonce, tampered, header) + if err == nil { + t.Errorf( + "Tampered ciphertext was not refused decryption (OCB did not return an error)") + return + } } func TestParameters(t *testing.T) { diff --git a/openpgp/armor/armor.go b/openpgp/armor/armor.go index e0a677f28..39c9b1223 100644 --- a/openpgp/armor/armor.go +++ b/openpgp/armor/armor.go @@ -69,6 +69,8 @@ func (l *lineReader) Read(p []byte) (n int, err error) { if isPrefix { return 0, ArmorCorrupt } + // Trim the line to remove any whitespace + line = bytes.TrimSpace(line) if bytes.HasPrefix(line, armorEnd) { l.eof = true diff --git a/openpgp/armor/armor_test.go b/openpgp/armor/armor_test.go index 861b35473..c1ec49a9a 100644 --- a/openpgp/armor/armor_test.go +++ b/openpgp/armor/armor_test.go @@ -89,6 +89,34 @@ func TestLongHeader(t *testing.T) { } } +func TestWithWhitespace(t *testing.T) { + buff := bytes.NewBuffer([]byte(armorWithWhitespace)) + armorWithWhitespace, err := Decode(buff) + if err != nil { + t.Error(err) + } + + armorWithWhitespaceBody, err := io.ReadAll(armorWithWhitespace.Body) + if err != nil { + t.Error(err) + } + + buff = bytes.NewBuffer([]byte(armorExampleEmptyVersion)) + armorWithOutWhitespace, err := Decode(buff) + if err != nil { + t.Error(err) + } + + armorWithOutWhitespaceBody, err := io.ReadAll(armorWithOutWhitespace.Body) + if err != nil { + t.Error(err) + } + + if !bytes.Equal(armorWithWhitespaceBody, armorWithOutWhitespaceBody) { + t.Errorf("got: %s want: %s", armorWithWhitespaceBody, armorWithOutWhitespaceBody) + } +} + const armorExample1 = `-----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.10 (GNU/Linux) @@ -129,3 +157,19 @@ k3hXy+XB5mZW6iisjUnUBknJEa43AMX+zGSaGHljEgfTGLbgEK+deOhPqKEkhUKr J0YF6lYvjcTVBtmQlYeOfZsz4EABEeBYe/rbDmJC =b+IB -----END PGP SIGNATURE-----` + +const armorWithWhitespace = `-----BEGIN PGP SIGNATURE----- + + wsE7BAABCgBvBYJkbfmWCRD7/MgqAV5zMEcUAAAAAAAeACBzYWx0QG5vdGF0aW9u + cy5zZXF1b2lhLXBncC5vcmeMXzsJEgIm228SdxV22XgYny4adwqEgyIT9UL3F92C + OhYhBNGmbhojsYLJmA94jPv8yCoBXnMwAAAj1AwAiSkJPxsEcyaoYWbxc657xPW1 + MlrbNhDBIWpKVrqQgyz7NdDZvvY0Ty+/h62HK5GQ5obAzVmQVwtUVG950TxCksg1 + F18mqticpxg1veZQdw7DBYTk0RJTpdVBRYJ5UOtHaSJUAnqGh7OQE6Lu74vfFhNv + dDjpjgEc6TnJrEBOOR7+RVp7+0i4HhM3+JdfSOMMOEb6ixWEYLtfC2Zd/p0f7vP8 + tHiqllDXDbfBCXlFl5h2LAh39o/LE0vZvwf+C9i9PgRARawWIh+xeAJsVne8FZ12 + FD+hWZJdNUCv4iE1H7QDVv8nuPAz3WB/DQWNSfeGTZnN+ouB1cjPFscBuunO5Dss + k3hXy+XB5mZW6iisjUnUBknJEa43AMX+zGSaGHljEgfTGLbgEK+deOhPqKEkhUKr + /VlIVBXgfjQuoizme9S9juxXHdDHa+Y5Wb9rTUc1y9YPArRem51VI0OzbJ2cRnLH + J0YF6lYvjcTVBtmQlYeOfZsz4EABEeBYe/rbDmJC + =b+IB +-----END PGP SIGNATURE-----` diff --git a/openpgp/armor/encode.go b/openpgp/armor/encode.go index 112f98b83..550efddf0 100644 --- a/openpgp/armor/encode.go +++ b/openpgp/armor/encode.go @@ -7,6 +7,7 @@ package armor import ( "encoding/base64" "io" + "sort" ) var armorHeaderSep = []byte(": ") @@ -159,8 +160,15 @@ func encode(out io.Writer, blockType string, headers map[string]string, checksum return } - for k, v := range headers { - err = writeSlices(out, []byte(k), armorHeaderSep, []byte(v), newline) + keys := make([]string, len(headers)) + i := 0 + for k := range headers { + keys[i] = k + i++ + } + sort.Strings(keys) + for _, k := range keys { + err = writeSlices(out, []byte(k), armorHeaderSep, []byte(headers[k]), newline) if err != nil { return } diff --git a/openpgp/clearsign/clearsign.go b/openpgp/clearsign/clearsign.go index 90bb9fc24..4c80db756 100644 --- a/openpgp/clearsign/clearsign.go +++ b/openpgp/clearsign/clearsign.go @@ -173,6 +173,7 @@ func Decode(data []byte) (b *Block, rest []byte) { b.Plaintext = append(b.Plaintext, line...) b.Plaintext = append(b.Plaintext, lf) } + b.Plaintext = b.Plaintext[:len(b.Plaintext)-1] // We want to find the extent of the armored data (including any newlines at // the end). @@ -205,7 +206,7 @@ func Decode(data []byte) (b *Block, rest []byte) { type dashEscaper struct { buffered *bufio.Writer hashers []hash.Hash // one per key in privateKeys - hashType crypto.Hash + hashTypes []crypto.Hash toHash io.Writer // writes to all the hashes in hashers salts [][]byte // salts for the signatures if v6 armorHeader map[string]string // Armor headers @@ -327,7 +328,7 @@ func (d *dashEscaper) Close() (err error) { sig.Version = k.Version sig.SigType = packet.SigTypeText sig.PubKeyAlgo = k.PubKeyAlgo - sig.Hash = d.hashType + sig.Hash = d.hashTypes[i] sig.CreationTime = t sig.IssuerKeyId = &k.KeyId sig.IssuerFingerprint = k.Fingerprint @@ -389,19 +390,22 @@ func EncodeMultiWithHeader(w io.Writer, privateKeys []*packet.PrivateKey, config } hashType := config.Hash() - name := nameOfHash(hashType) - if len(name) == 0 { - return nil, errors.UnsupportedError("unknown hash type: " + strconv.Itoa(int(hashType))) - } - if !hashType.Available() { - return nil, errors.UnsupportedError("unsupported hash type: " + strconv.Itoa(int(hashType))) - } var hashers []hash.Hash + var hashTypes []crypto.Hash var ws []io.Writer var salts [][]byte for _, sk := range privateKeys { - h := hashType.New() + acceptedHashes := acceptableHashesToWrite(&sk.PublicKey) + // acceptedHashes contains at least one hash + selectedHashType := acceptedHashes[0] + for _, acceptedHash := range acceptedHashes { + if hashType == acceptedHash { + selectedHashType = hashType + break + } + } + h := selectedHashType.New() if sk.Version == 6 { // generate salt var salt []byte @@ -415,6 +419,7 @@ func EncodeMultiWithHeader(w io.Writer, privateKeys []*packet.PrivateKey, config salts = append(salts, salt) } hashers = append(hashers, h) + hashTypes = append(hashTypes, selectedHashType) ws = append(ws, h) } toHash := io.MultiWriter(ws...) @@ -431,11 +436,8 @@ func EncodeMultiWithHeader(w io.Writer, privateKeys []*packet.PrivateKey, config nonV6 := len(salts) < len(hashers) // Crypto refresh: Headers SHOULD NOT be emitted if nonV6 { // Emit header if non v6 signatures are present for compatibility - if _, err = buffered.WriteString(fmt.Sprintf("%s: %s", hashHeader, name)); err != nil { - return - } - if err = buffered.WriteByte(lf); err != nil { - return + if err := writeHashHeader(buffered, hashTypes); err != nil { + return nil, err } } if err = buffered.WriteByte(lf); err != nil { @@ -445,7 +447,7 @@ func EncodeMultiWithHeader(w io.Writer, privateKeys []*packet.PrivateKey, config plaintext = &dashEscaper{ buffered: buffered, hashers: hashers, - hashType: hashType, + hashTypes: hashTypes, toHash: toHash, salts: salts, armorHeader: headers, @@ -469,6 +471,40 @@ func (b *Block) VerifySignature(keyring openpgp.KeyRing, config *packet.Config) return } +// writeHashHeader writes the legacy cleartext hash header to buffered. +func writeHashHeader(buffered *bufio.Writer, hashTypes []crypto.Hash) error { + seen := make(map[string]bool, len(hashTypes)) + if _, err := buffered.WriteString(fmt.Sprintf("%s: ", hashHeader)); err != nil { + return err + } + + for index, sigHashType := range hashTypes { + first := index == 0 + name := nameOfHash(sigHashType) + if len(name) == 0 { + return errors.UnsupportedError("unknown hash type: " + strconv.Itoa(int(sigHashType))) + } + + switch { + case !seen[name] && first: + if _, err := buffered.WriteString(name); err != nil { + return err + } + case !seen[name]: + if _, err := buffered.WriteString(fmt.Sprintf(",%s", name)); err != nil { + return err + } + } + seen[name] = true + } + + if err := buffered.WriteByte(lf); err != nil { + return err + } + + return nil +} + // nameOfHash returns the OpenPGP name for the given hash, or the empty string // if the name isn't known. See RFC 4880, section 9.4. func nameOfHash(h crypto.Hash) string { @@ -488,3 +524,38 @@ func nameOfHash(h crypto.Hash) string { } return "" } + +func acceptableHashesToWrite(singingKey *packet.PublicKey) []crypto.Hash { + switch singingKey.PubKeyAlgo { + case packet.PubKeyAlgoEd448: + return []crypto.Hash{ + crypto.SHA512, + crypto.SHA3_512, + } + case packet.PubKeyAlgoECDSA, packet.PubKeyAlgoEdDSA: + if curve, err := singingKey.Curve(); err == nil { + if curve == packet.Curve448 || + curve == packet.CurveNistP521 || + curve == packet.CurveBrainpoolP512 { + return []crypto.Hash{ + crypto.SHA512, + crypto.SHA3_512, + } + } else if curve == packet.CurveBrainpoolP384 || + curve == packet.CurveNistP384 { + return []crypto.Hash{ + crypto.SHA384, + crypto.SHA512, + crypto.SHA3_512, + } + } + } + } + return []crypto.Hash{ + crypto.SHA256, + crypto.SHA384, + crypto.SHA512, + crypto.SHA3_256, + crypto.SHA3_512, + } +} diff --git a/openpgp/clearsign/clearsign_test.go b/openpgp/clearsign/clearsign_test.go index 0e93c8641..dffcd7ba8 100644 --- a/openpgp/clearsign/clearsign_test.go +++ b/openpgp/clearsign/clearsign_test.go @@ -5,6 +5,7 @@ package clearsign import ( + "bufio" "bytes" "crypto" "fmt" @@ -58,8 +59,8 @@ func testParse(t *testing.T, input []byte, expected, expectedPlaintext string) { } func TestParse(t *testing.T) { - testParse(t, clearsignInput, "Hello world\r\nline 2", "Hello world\nline 2\n") - testParse(t, clearsignInput2, "\r\n\r\n(This message has a couple of blank lines at the start and end.)\r\n\r\n", "\n\n(This message has a couple of blank lines at the start and end.)\n\n\n") + testParse(t, clearsignInput, "Hello world\r\nline 2", "Hello world\nline 2") + testParse(t, clearsignInput2, "\r\n\r\n(This message has a couple of blank lines at the start and end.)\r\n\r\n", "\n\n(This message has a couple of blank lines at the start and end.)\n\n") } func TestParseWithNoNewlineAtEnd(t *testing.T) { @@ -77,21 +78,21 @@ func TestParseWithNoNewlineAtEnd(t *testing.T) { var signingTests = []struct { in, signed, plaintext string }{ - {"", "", "\n"}, - {"a", "a", "a\n"}, - {"a\n", "a\r\n", "a\n\n"}, - {"-a\n", "-a\r\n", "-a\n\n"}, - {"--a\nb", "--a\r\nb", "--a\nb\n"}, + {"", "", ""}, + {"a", "a", "a"}, + {"a\n", "a\r\n", "a\n"}, + {"-a\n", "-a\r\n", "-a\n"}, + {"--a\nb", "--a\r\nb", "--a\nb"}, // leading whitespace - {" a\n", " a\r\n", " a\n\n"}, - {" a\n", " a\r\n", " a\n\n"}, + {" a\n", " a\r\n", " a\n"}, + {" a\n", " a\r\n", " a\n"}, // trailing whitespace (should be stripped) - {"a \n", "a\r\n", "a\n\n"}, - {"a ", "a", "a\n"}, - {" \n", "\r\n", "\n\n"}, - {" ", "", "\n"}, - {"a\n \n \nb\n", "a\r\n\r\n\r\nb\r\n", "a\n\n\nb\n\n"}, - {"a\n \n \nb\n", "a\r\n\r\n\r\nb\r\n", "a\n\n\nb\n\n"}, + {"a \n", "a\r\n", "a\n"}, + {"a ", "a", "a"}, + {" \n", "\r\n", "\n"}, + {" ", "", ""}, + {"a\n \n \nb\n", "a\r\n\r\n\r\nb\r\n", "a\n\n\nb\n"}, + {"a\n \n \nb\n", "a\r\n\r\n\r\nb\r\n", "a\n\n\nb\n"}, } func TestVerifyV6(t *testing.T) { @@ -171,15 +172,61 @@ func TestSigningInterop(t *testing.T) { } } -// We use this to make test keys, so that they aren't all the same. -type quickRand byte +func TestHashHeader(t *testing.T) { + tests := []struct { + name string + hashTypes []crypto.Hash + expected string + }{ + { + name: "unique hashes", + hashTypes: []crypto.Hash{ + crypto.SHA256, + crypto.SHA512, + crypto.SHA3_512, + }, + expected: "Hash: SHA256,SHA512,SHA3-512\n", + }, + { + name: "with duplicates", + hashTypes: []crypto.Hash{ + crypto.SHA256, + crypto.SHA512, + crypto.SHA512, + crypto.SHA3_512, + }, + expected: "Hash: SHA256,SHA512,SHA3-512\n", + }, + { + name: "with duplicates", + hashTypes: []crypto.Hash{ + crypto.SHA256, + crypto.SHA256, + crypto.SHA256, + crypto.SHA256, + }, + expected: "Hash: SHA256\n", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + var buf bytes.Buffer + writer := bufio.NewWriter(&buf) + if err := writeHashHeader(writer, tc.hashTypes); err != nil { + t.Fatalf("unexpected error: %v", err) + } -func (qr *quickRand) Read(p []byte) (int, error) { - for i := range p { - p[i] = byte(*qr) + if err := writer.Flush(); err != nil { + t.Fatalf("flush failed: %v", err) + } + + actual := buf.String() + if actual != tc.expected { + t.Errorf("output mismatch:\nExpected: %q\nActual: %q", tc.expected, actual) + } + }) } - *qr++ - return len(p), nil } func testMultiSign(t *testing.T, v6 bool) { @@ -187,8 +234,7 @@ func testMultiSign(t *testing.T, v6 bool) { t.Skip("skipping long test in -short mode") } - zero := quickRand(0) - config := packet.Config{Rand: &zero, V6Keys: v6} + config := packet.Config{V6Keys: v6} for nKeys := 1; nKeys < 4; nKeys++ { nextTest: diff --git a/openpgp/errors/errors.go b/openpgp/errors/errors.go index c42b01cb0..2e341507a 100644 --- a/openpgp/errors/errors.go +++ b/openpgp/errors/errors.go @@ -6,9 +6,22 @@ package errors // import "github.com/ProtonMail/go-crypto/openpgp/errors" import ( + "fmt" "strconv" ) +var ( + // ErrDecryptSessionKeyParsing is a generic error message for parsing errors in decrypted data + // to reduce the risk of oracle attacks. + ErrDecryptSessionKeyParsing = DecryptWithSessionKeyError("parsing error") + // ErrAEADTagVerification is returned if one of the tag verifications in SEIPDv2 fails + ErrAEADTagVerification error = DecryptWithSessionKeyError("AEAD tag verification failed") + // ErrMDCHashMismatch + ErrMDCHashMismatch error = SignatureError("MDC hash mismatch") + // ErrMDCMissing + ErrMDCMissing error = SignatureError("MDC packet not found") +) + // A StructuralError is returned when OpenPGP data is found to be syntactically // invalid. type StructuralError string @@ -17,6 +30,34 @@ func (s StructuralError) Error() string { return "openpgp: invalid data: " + string(s) } +// A DecryptWithSessionKeyError is returned when a failure occurs when reading from symmetrically decrypted data or +// an authentication tag verification fails. +// Such an error indicates that the supplied session key is likely wrong or the data got corrupted. +type DecryptWithSessionKeyError string + +func (s DecryptWithSessionKeyError) Error() string { + return "openpgp: decryption with session key failed: " + string(s) +} + +// HandleSensitiveParsingError handles parsing errors when reading data from potentially decrypted data. +// The function makes parsing errors generic to reduce the risk of oracle attacks in SEIPDv1. +func HandleSensitiveParsingError(err error, decrypted bool) error { + if !decrypted { + // Data was not encrypted so we return the inner error. + return err + } + // The data is read from a stream that decrypts using a session key; + // therefore, we need to handle parsing errors appropriately. + // This is essential to mitigate the risk of oracle attacks. + if decError, ok := err.(*DecryptWithSessionKeyError); ok { + return decError + } + if decError, ok := err.(DecryptWithSessionKeyError); ok { + return decError + } + return ErrDecryptSessionKeyParsing +} + // UnsupportedError indicates that, although the OpenPGP data is valid, it // makes use of currently unimplemented features. type UnsupportedError string @@ -41,9 +82,6 @@ func (b SignatureError) Error() string { return "openpgp: invalid signature: " + string(b) } -var ErrMDCHashMismatch error = SignatureError("MDC hash mismatch") -var ErrMDCMissing error = SignatureError("MDC packet not found") - type signatureExpiredError int func (se signatureExpiredError) Error() string { @@ -141,3 +179,32 @@ type ErrMalformedMessage string func (dke ErrMalformedMessage) Error() string { return "openpgp: malformed message " + string(dke) } + +type messageTooLargeError int + +func (e messageTooLargeError) Error() string { + return "openpgp: decompressed message size exceeds provided limit" +} + +// ErrMessageTooLarge is returned if the read data from +// a compressed packet exceeds the provided limit. +var ErrMessageTooLarge error = messageTooLargeError(0) + +// ErrEncryptionKeySelection is returned if encryption key selection fails (v2 API). +type ErrEncryptionKeySelection struct { + PrimaryKeyId string + PrimaryKeyErr error + EncSelectionKeyId *string + EncSelectionErr error +} + +func (eks ErrEncryptionKeySelection) Error() string { + prefix := fmt.Sprintf("openpgp: key selection for primary key %s:", eks.PrimaryKeyId) + if eks.PrimaryKeyErr != nil { + return fmt.Sprintf("%s invalid primary key: %s", prefix, eks.PrimaryKeyErr) + } + if eks.EncSelectionKeyId != nil { + return fmt.Sprintf("%s invalid encryption key %s: %s", prefix, *eks.EncSelectionKeyId, eks.EncSelectionErr) + } + return fmt.Sprintf("%s no encryption key: %s", prefix, eks.EncSelectionErr) +} diff --git a/openpgp/key_generation.go b/openpgp/key_generation.go index c9502c25f..77213f66b 100644 --- a/openpgp/key_generation.go +++ b/openpgp/key_generation.go @@ -91,13 +91,15 @@ func (t *Entity) AddUserId(name, comment, email string, config *packet.Config) e } func writeKeyProperties(selfSignature *packet.Signature, creationTime time.Time, keyLifetimeSecs uint32, config *packet.Config) error { + advertiseAead := config.AEAD() != nil + selfSignature.CreationTime = creationTime selfSignature.KeyLifetimeSecs = &keyLifetimeSecs selfSignature.FlagsValid = true selfSignature.FlagSign = true selfSignature.FlagCertify = true selfSignature.SEIPDv1 = true // true by default, see 5.8 vs. 5.14 - selfSignature.SEIPDv2 = config.AEAD() != nil + selfSignature.SEIPDv2 = advertiseAead // Set the PreferredHash for the SelfSignature from the packet.Config. // If it is not the must-implement algorithm from rfc4880bis, append that. @@ -126,16 +128,19 @@ func writeKeyProperties(selfSignature *packet.Signature, creationTime time.Time, selfSignature.PreferredCompression = append(selfSignature.PreferredCompression, uint8(config.Compression())) } - // And for DefaultMode. - modes := []uint8{uint8(config.AEAD().Mode())} - if config.AEAD().Mode() != packet.AEADModeOCB { - modes = append(modes, uint8(packet.AEADModeOCB)) - } + if advertiseAead { + // Get the preferred AEAD mode from the packet.Config. + // If it is not the must-implement algorithm from rfc9580, append that. + modes := []uint8{uint8(config.AEAD().Mode())} + if config.AEAD().Mode() != packet.AEADModeOCB { + modes = append(modes, uint8(packet.AEADModeOCB)) + } - // For preferred (AES256, GCM), we'll generate (AES256, GCM), (AES256, OCB), (AES128, GCM), (AES128, OCB) - for _, cipher := range selfSignature.PreferredSymmetric { - for _, mode := range modes { - selfSignature.PreferredCipherSuites = append(selfSignature.PreferredCipherSuites, [2]uint8{cipher, mode}) + // For preferred (AES256, GCM), we'll generate (AES256, GCM), (AES256, OCB), (AES128, GCM), (AES128, OCB) + for _, cipher := range selfSignature.PreferredSymmetric { + for _, mode := range modes { + selfSignature.PreferredCipherSuites = append(selfSignature.PreferredCipherSuites, [2]uint8{cipher, mode}) + } } } return nil diff --git a/openpgp/packet/aead_config.go b/openpgp/packet/aead_config.go index fec41a0e7..ef100d372 100644 --- a/openpgp/packet/aead_config.go +++ b/openpgp/packet/aead_config.go @@ -37,7 +37,7 @@ func (conf *AEADConfig) Mode() AEADMode { // ChunkSizeByte returns the byte indicating the chunk size. The effective // chunk size is computed with the formula uint64(1) << (chunkSizeByte + 6) -// limit to 16 = 4 MiB +// limit chunkSizeByte to 16 which equals to 2^22 = 4 MiB // https://www.ietf.org/archive/id/draft-ietf-openpgp-crypto-refresh-07.html#section-5.13.2 func (conf *AEADConfig) ChunkSizeByte() byte { if conf == nil || conf.ChunkSize == 0 { @@ -49,8 +49,8 @@ func (conf *AEADConfig) ChunkSizeByte() byte { switch { case exponent < 6: exponent = 6 - case exponent > 16: - exponent = 16 + case exponent > 22: + exponent = 22 } return byte(exponent - 6) diff --git a/openpgp/packet/aead_crypter.go b/openpgp/packet/aead_crypter.go index 7171387f9..5e4604656 100644 --- a/openpgp/packet/aead_crypter.go +++ b/openpgp/packet/aead_crypter.go @@ -3,7 +3,6 @@ package packet import ( - "bytes" "crypto/cipher" "encoding/binary" "io" @@ -15,12 +14,11 @@ import ( type aeadCrypter struct { aead cipher.AEAD chunkSize int - initialNonce []byte + nonce []byte associatedData []byte // Chunk-independent associated data chunkIndex []byte // Chunk counter packetTag packetType // SEIP packet (v2) or AEAD Encrypted Data packet bytesProcessed int // Amount of plaintext bytes encrypted/decrypted - buffer bytes.Buffer // Buffered bytes across chunks } // computeNonce takes the incremental index and computes an eXclusive OR with @@ -28,12 +26,12 @@ type aeadCrypter struct { // 5.16.1 and 5.16.2). It returns the resulting nonce. func (wo *aeadCrypter) computeNextNonce() (nonce []byte) { if wo.packetTag == packetTypeSymmetricallyEncryptedIntegrityProtected { - return append(wo.initialNonce, wo.chunkIndex...) + return wo.nonce } - nonce = make([]byte, len(wo.initialNonce)) - copy(nonce, wo.initialNonce) - offset := len(wo.initialNonce) - 8 + nonce = make([]byte, len(wo.nonce)) + copy(nonce, wo.nonce) + offset := len(wo.nonce) - 8 for i := 0; i < 8; i++ { nonce[i+offset] ^= wo.chunkIndex[i] } @@ -62,8 +60,9 @@ func (wo *aeadCrypter) incrementIndex() error { type aeadDecrypter struct { aeadCrypter // Embedded ciphertext opener reader io.Reader // 'reader' is a partialLengthReader + chunkBytes []byte peekedBytes []byte // Used to detect last chunk - eof bool + buffer []byte // Buffered decrypted bytes } // Read decrypts bytes and reads them into dst. It decrypts when necessary and @@ -71,59 +70,44 @@ type aeadDecrypter struct { // and an error. func (ar *aeadDecrypter) Read(dst []byte) (n int, err error) { // Return buffered plaintext bytes from previous calls - if ar.buffer.Len() > 0 { - return ar.buffer.Read(dst) - } - - // Return EOF if we've previously validated the final tag - if ar.eof { - return 0, io.EOF + if len(ar.buffer) > 0 { + n = copy(dst, ar.buffer) + ar.buffer = ar.buffer[n:] + return } // Read a chunk tagLen := ar.aead.Overhead() - cipherChunkBuf := new(bytes.Buffer) - _, errRead := io.CopyN(cipherChunkBuf, ar.reader, int64(ar.chunkSize+tagLen)) - cipherChunk := cipherChunkBuf.Bytes() - if errRead != nil && errRead != io.EOF { + copy(ar.chunkBytes, ar.peekedBytes) // Copy bytes peeked in previous chunk or in initialization + bytesRead, errRead := io.ReadFull(ar.reader, ar.chunkBytes[tagLen:]) + if errRead != nil && errRead != io.EOF && errRead != io.ErrUnexpectedEOF { return 0, errRead } - if len(cipherChunk) > 0 { - decrypted, errChunk := ar.openChunk(cipherChunk) + if bytesRead > 0 { + ar.peekedBytes = ar.chunkBytes[bytesRead:bytesRead+tagLen] + + decrypted, errChunk := ar.openChunk(ar.chunkBytes[:bytesRead]) if errChunk != nil { return 0, errChunk } // Return decrypted bytes, buffering if necessary - if len(dst) < len(decrypted) { - n = copy(dst, decrypted[:len(dst)]) - ar.buffer.Write(decrypted[len(dst):]) - } else { - n = copy(dst, decrypted) - } + n = copy(dst, decrypted) + ar.buffer = decrypted[n:] + return } - // Check final authentication tag - if errRead == io.EOF { - errChunk := ar.validateFinalTag(ar.peekedBytes) - if errChunk != nil { - return n, errChunk - } - ar.eof = true // Mark EOF for when we've returned all buffered data - } - return + return 0, io.EOF } -// Close is noOp. The final authentication tag of the stream was already -// checked in the last Read call. In the future, this function could be used to -// wipe the reader and peeked, decrypted bytes, if necessary. +// Close checks the final authentication tag of the stream. +// In the future, this function could also be used to wipe the reader +// and peeked & decrypted bytes, if necessary. func (ar *aeadDecrypter) Close() (err error) { - if !ar.eof { - errChunk := ar.validateFinalTag(ar.peekedBytes) - if errChunk != nil { - return errChunk - } + errChunk := ar.validateFinalTag(ar.peekedBytes) + if errChunk != nil { + return errChunk } return nil } @@ -132,22 +116,15 @@ func (ar *aeadDecrypter) Close() (err error) { // the underlying plaintext and an error. It accesses peeked bytes from next // chunk, to identify the last chunk and decrypt/validate accordingly. func (ar *aeadDecrypter) openChunk(data []byte) ([]byte, error) { - tagLen := ar.aead.Overhead() - // Restore carried bytes from last call - chunkExtra := append(ar.peekedBytes, data...) - // 'chunk' contains encrypted bytes, followed by an authentication tag. - chunk := chunkExtra[:len(chunkExtra)-tagLen] - ar.peekedBytes = chunkExtra[len(chunkExtra)-tagLen:] - adata := ar.associatedData if ar.aeadCrypter.packetTag == packetTypeAEADEncrypted { adata = append(ar.associatedData, ar.chunkIndex...) } nonce := ar.computeNextNonce() - plainChunk, err := ar.aead.Open(nil, nonce, chunk, adata) + plainChunk, err := ar.aead.Open(data[:0:len(data)], nonce, data, adata) if err != nil { - return nil, err + return nil, errors.ErrAEADTagVerification } ar.bytesProcessed += len(plainChunk) if err = ar.aeadCrypter.incrementIndex(); err != nil { @@ -172,8 +149,10 @@ func (ar *aeadDecrypter) validateFinalTag(tag []byte) error { // ... and total number of encrypted octets adata = append(adata, amountBytes...) nonce := ar.computeNextNonce() - _, err := ar.aead.Open(nil, nonce, tag, adata) - return err + if _, err := ar.aead.Open(nil, nonce, tag, adata); err != nil { + return errors.ErrAEADTagVerification + } + return nil } // aeadEncrypter encrypts and writes bytes. It encrypts when necessary according @@ -181,27 +160,29 @@ func (ar *aeadDecrypter) validateFinalTag(tag []byte) error { type aeadEncrypter struct { aeadCrypter // Embedded plaintext sealer writer io.WriteCloser // 'writer' is a partialLengthWriter + chunkBytes []byte + offset int } // Write encrypts and writes bytes. It encrypts when necessary and buffers extra // plaintext bytes for next call. When the stream is finished, Close() MUST be // called to append the final tag. func (aw *aeadEncrypter) Write(plaintextBytes []byte) (n int, err error) { - // Append plaintextBytes to existing buffered bytes - n, err = aw.buffer.Write(plaintextBytes) - if err != nil { - return n, err - } - // Encrypt and write chunks - for aw.buffer.Len() >= aw.chunkSize { - plainChunk := aw.buffer.Next(aw.chunkSize) - encryptedChunk, err := aw.sealChunk(plainChunk) - if err != nil { - return n, err - } - _, err = aw.writer.Write(encryptedChunk) - if err != nil { - return n, err + for n != len(plaintextBytes) { + copied := copy(aw.chunkBytes[aw.offset:aw.chunkSize], plaintextBytes[n:]) + n += copied + aw.offset += copied + + if aw.offset == aw.chunkSize { + encryptedChunk, err := aw.sealChunk(aw.chunkBytes[:aw.offset]) + if err != nil { + return n, err + } + _, err = aw.writer.Write(encryptedChunk) + if err != nil { + return n, err + } + aw.offset = 0 } } return @@ -213,9 +194,8 @@ func (aw *aeadEncrypter) Write(plaintextBytes []byte) (n int, err error) { func (aw *aeadEncrypter) Close() (err error) { // Encrypt and write a chunk if there's buffered data left, or if we haven't // written any chunks yet. - if aw.buffer.Len() > 0 || aw.bytesProcessed == 0 { - plainChunk := aw.buffer.Bytes() - lastEncryptedChunk, err := aw.sealChunk(plainChunk) + if aw.offset > 0 || aw.bytesProcessed == 0 { + lastEncryptedChunk, err := aw.sealChunk(aw.chunkBytes[:aw.offset]) if err != nil { return err } @@ -261,7 +241,7 @@ func (aw *aeadEncrypter) sealChunk(data []byte) ([]byte, error) { } nonce := aw.computeNextNonce() - encrypted := aw.aead.Seal(nil, nonce, data, adata) + encrypted := aw.aead.Seal(data[:0], nonce, data, adata) aw.bytesProcessed += len(data) if err := aw.aeadCrypter.incrementIndex(); err != nil { return nil, err diff --git a/openpgp/packet/aead_encrypted.go b/openpgp/packet/aead_encrypted.go index 98bd876bf..583765d87 100644 --- a/openpgp/packet/aead_encrypted.go +++ b/openpgp/packet/aead_encrypted.go @@ -65,24 +65,28 @@ func (ae *AEADEncrypted) decrypt(key []byte) (io.ReadCloser, error) { blockCipher := ae.cipher.new(key) aead := ae.mode.new(blockCipher) // Carry the first tagLen bytes + chunkSize := decodeAEADChunkSize(ae.chunkSizeByte) tagLen := ae.mode.TagLength() - peekedBytes := make([]byte, tagLen) + chunkBytes := make([]byte, chunkSize+tagLen*2) + peekedBytes := chunkBytes[chunkSize+tagLen:] n, err := io.ReadFull(ae.Contents, peekedBytes) if n < tagLen || (err != nil && err != io.EOF) { return nil, errors.AEADError("Not enough data to decrypt:" + err.Error()) } - chunkSize := decodeAEADChunkSize(ae.chunkSizeByte) + return &aeadDecrypter{ aeadCrypter: aeadCrypter{ aead: aead, chunkSize: chunkSize, - initialNonce: ae.initialNonce, + nonce: ae.initialNonce, associatedData: ae.associatedData(), chunkIndex: make([]byte, 8), packetTag: packetTypeAEADEncrypted, }, reader: ae.Contents, - peekedBytes: peekedBytes}, nil + chunkBytes: chunkBytes, + peekedBytes: peekedBytes, + }, nil } // associatedData for chunks: tag, version, cipher, mode, chunk size byte diff --git a/openpgp/packet/aead_encrypted_test.go b/openpgp/packet/aead_encrypted_test.go index 97736071a..e2b328a08 100644 --- a/openpgp/packet/aead_encrypted_test.go +++ b/openpgp/packet/aead_encrypted_test.go @@ -407,6 +407,7 @@ func readDecryptedStream(rc io.ReadCloser) (got []byte, err error) { } } } + err = rc.Close() return got, err } @@ -448,15 +449,18 @@ func SerializeAEADEncrypted(w io.Writer, key []byte, config *Config) (io.WriteCl alg := aeadConf.Mode().new(blockCipher) chunkSize := decodeAEADChunkSize(aeadConf.ChunkSizeByte()) + tagLen := alg.Overhead() + chunkBytes := make([]byte, chunkSize+tagLen) return &aeadEncrypter{ aeadCrypter: aeadCrypter{ aead: alg, chunkSize: chunkSize, associatedData: prefix, chunkIndex: make([]byte, 8), - initialNonce: nonce, + nonce: nonce, packetTag: packetTypeAEADEncrypted, }, - writer: writer, + writer: writer, + chunkBytes: chunkBytes, }, nil } diff --git a/openpgp/packet/compressed.go b/openpgp/packet/compressed.go index 0bcb38cac..931f55a4e 100644 --- a/openpgp/packet/compressed.go +++ b/openpgp/packet/compressed.go @@ -98,6 +98,16 @@ func (c *Compressed) parse(r io.Reader) error { return err } +// LimitedBodyReader wraps the provided body reader with a limiter that restricts +// the number of bytes read to the specified limit. +// If limit is nil, the reader is unbounded. +func (c *Compressed) LimitedBodyReader(limit *int64) io.Reader { + if limit == nil { + return c.Body + } + return &LimitReader{R: c.Body, N: *limit} +} + // compressedWriterCloser represents the serialized compression stream // header and the compressor. Its Close() method ensures that both the // compressor and serialized stream header are closed. Its Write() @@ -159,3 +169,24 @@ func SerializeCompressed(w io.WriteCloser, algo CompressionAlgo, cc *Compression return } + +// LimitReader is an io.Reader that fails with MessageToLarge if read bytes exceed N. +type LimitReader struct { + R io.Reader // underlying reader + N int64 // max bytes allowed +} + +func (l *LimitReader) Read(p []byte) (int, error) { + if l.N <= 0 { + return 0, errors.ErrMessageTooLarge + } + + n, err := l.R.Read(p) + l.N -= int64(n) + + if err == nil && l.N <= 0 { + err = errors.ErrMessageTooLarge + } + + return n, err +} diff --git a/openpgp/packet/config.go b/openpgp/packet/config.go index fb21e6d1b..142be0aa0 100644 --- a/openpgp/packet/config.go +++ b/openpgp/packet/config.go @@ -139,6 +139,11 @@ type Config struct { // might be no other way than to tolerate the missing MDC. Setting this flag, allows this // mode of operation. It should be considered a measure of last resort. InsecureAllowUnauthenticatedMessages bool + // InsecureAllowDecryptionWithSigningKeys allows decryption with keys marked as signing keys in the v2 API. + // This setting is potentially insecure, but it is needed as some libraries + // ignored key flags when selecting a key for encryption. + // Not relevant for the v1 API, as all keys were allowed in decryption. + InsecureAllowDecryptionWithSigningKeys bool // KnownNotations is a map of Notation Data names to bools, which controls // the notation names that are allowed to be present in critical Notation Data // signature subpackets. @@ -168,6 +173,28 @@ type Config struct { // weaknesses in the hash algo, potentially hindering e.g. some chosen-prefix attacks. // The default behavior, when the config or flag is nil, is to enable the feature. NonDeterministicSignaturesViaNotation *bool + + // InsecureAllowAllKeyFlagsWhenMissing determines how a key without valid key flags is handled. + // When set to true, a key without flags is treated as if all flags are enabled. + // This behavior is consistent with GPG. + InsecureAllowAllKeyFlagsWhenMissing bool + // InsecureGenerateNonCriticalKeyFlags causes the "Key Flags" signature subpacket + // to be non-critical in newly generated signatures. + // This may be needed for keys to be accepted by older clients who do not recognize + // the subpacket. + // For example, rpm 4.14.3-150400.59.3.1 in OpenSUSE Leap 15.4 does not recognize it. + InsecureGenerateNonCriticalKeyFlags bool + // InsecureGenerateNonCriticalSignatureCreationTime causes the "Signature Creation Time" signature subpacket + // to be non-critical in newly generated signatures. + // This may be needed for keys to be accepted by older clients who do not recognize + // the subpacket. + // For example, yum 3.4.3-168 in CentOS 7 and yum 3.4.3-158 in Amazon Linux 2 do not recognize it. + InsecureGenerateNonCriticalSignatureCreationTime bool + + // MaxDecompressedMessageSize specifies the maximum number of bytes that can be + // read from a compressed packet. This serves as an upper limit to prevent + // excessively large decompressed messages. + MaxDecompressedMessageSize *int64 } func (c *Config) Random() io.Reader { @@ -291,6 +318,13 @@ func (c *Config) AllowUnauthenticatedMessages() bool { return c.InsecureAllowUnauthenticatedMessages } +func (c *Config) AllowDecryptionWithSigningKeys() bool { + if c == nil { + return false + } + return c.InsecureAllowDecryptionWithSigningKeys +} + func (c *Config) KnownNotation(notationName string) bool { if c == nil { return false @@ -391,6 +425,34 @@ func (c *Config) RandomizeSignaturesViaNotation() bool { return *c.NonDeterministicSignaturesViaNotation } +func (c *Config) AllowAllKeyFlagsWhenMissing() bool { + if c == nil { + return false + } + return c.InsecureAllowAllKeyFlagsWhenMissing +} + +func (c *Config) GenerateNonCriticalKeyFlags() bool { + if c == nil { + return false + } + return c.InsecureGenerateNonCriticalKeyFlags +} + +func (c *Config) GenerateNonCriticalSignatureCreationTime() bool { + if c == nil { + return false + } + return c.InsecureGenerateNonCriticalSignatureCreationTime +} + +func (c *Config) DecompressedMessageSizeLimit() *int64 { + if c == nil { + return nil + } + return c.MaxDecompressedMessageSize +} + // BoolPointer is a helper function to set a boolean pointer in the Config. // e.g., config.CheckPacketSequence = BoolPointer(true) func BoolPointer(value bool) *bool { diff --git a/openpgp/packet/encrypted_key.go b/openpgp/packet/encrypted_key.go index 583409456..b90bb2891 100644 --- a/openpgp/packet/encrypted_key.go +++ b/openpgp/packet/encrypted_key.go @@ -321,7 +321,8 @@ func (e *EncryptedKey) Serialize(w io.Writer) error { // SerializeEncryptedKeyAEAD serializes an encrypted key packet to w that contains // key, encrypted to pub. -// If aeadSupported is set, PKESK v6 is used else v4. +// If aeadSupported is set, PKESK v6 is used, otherwise v3. +// Note: aeadSupported MUST match the value passed to SerializeSymmetricallyEncrypted. // If config is nil, sensible defaults will be used. func SerializeEncryptedKeyAEAD(w io.Writer, pub *PublicKey, cipherFunc CipherFunction, aeadSupported bool, key []byte, config *Config) error { return SerializeEncryptedKeyAEADwithHiddenOption(w, pub, cipherFunc, aeadSupported, key, false, config) @@ -330,7 +331,8 @@ func SerializeEncryptedKeyAEAD(w io.Writer, pub *PublicKey, cipherFunc CipherFun // SerializeEncryptedKeyAEADwithHiddenOption serializes an encrypted key packet to w that contains // key, encrypted to pub. // Offers the hidden flag option to indicated if the PKESK packet should include a wildcard KeyID. -// If aeadSupported is set, PKESK v6 is used else v4. +// If aeadSupported is set, PKESK v6 is used, otherwise v3. +// Note: aeadSupported MUST match the value passed to SerializeSymmetricallyEncrypted. // If config is nil, sensible defaults will be used. func SerializeEncryptedKeyAEADwithHiddenOption(w io.Writer, pub *PublicKey, cipherFunc CipherFunction, aeadSupported bool, key []byte, hidden bool, config *Config) error { var buf [36]byte // max possible header size is v6 @@ -426,6 +428,7 @@ func SerializeEncryptedKeyAEADwithHiddenOption(w io.Writer, pub *PublicKey, ciph // key, encrypted to pub. // PKESKv6 is used if config.AEAD() is not nil. // If config is nil, sensible defaults will be used. +// Deprecated: Use SerializeEncryptedKeyAEAD instead. func SerializeEncryptedKey(w io.Writer, pub *PublicKey, cipherFunc CipherFunction, key []byte, config *Config) error { return SerializeEncryptedKeyAEAD(w, pub, cipherFunc, config.AEAD() != nil, key, config) } @@ -434,6 +437,7 @@ func SerializeEncryptedKey(w io.Writer, pub *PublicKey, cipherFunc CipherFunctio // key, encrypted to pub. PKESKv6 is used if config.AEAD() is not nil. // The hidden option controls if the packet should be anonymous, i.e., omit key metadata. // If config is nil, sensible defaults will be used. +// Deprecated: Use SerializeEncryptedKeyAEADwithHiddenOption instead. func SerializeEncryptedKeyWithHiddenOption(w io.Writer, pub *PublicKey, cipherFunc CipherFunction, key []byte, hidden bool, config *Config) error { return SerializeEncryptedKeyAEADwithHiddenOption(w, pub, cipherFunc, config.AEAD() != nil, key, hidden, config) } diff --git a/openpgp/packet/public_key.go b/openpgp/packet/public_key.go index f8da781bb..e2813396e 100644 --- a/openpgp/packet/public_key.go +++ b/openpgp/packet/public_key.go @@ -1048,12 +1048,17 @@ func (pk *PublicKey) VerifyDirectKeySignature(sig *Signature) (err error) { // KeyIdString returns the public key's fingerprint in capital hex // (e.g. "6C7EE1B8621CC013"). func (pk *PublicKey) KeyIdString() string { - return fmt.Sprintf("%X", pk.Fingerprint[12:20]) + return fmt.Sprintf("%016X", pk.KeyId) } // KeyIdShortString returns the short form of public key's fingerprint // in capital hex, as shown by gpg --list-keys (e.g. "621CC013"). +// This function will return the full key id for v5 and v6 keys +// since the short key id is undefined for them. func (pk *PublicKey) KeyIdShortString() string { + if pk.Version >= 5 { + return pk.KeyIdString() + } return fmt.Sprintf("%X", pk.Fingerprint[16:20]) } diff --git a/openpgp/packet/signature.go b/openpgp/packet/signature.go index 3a4b366d8..4490fdf83 100644 --- a/openpgp/packet/signature.go +++ b/openpgp/packet/signature.go @@ -933,7 +933,7 @@ func (sig *Signature) Sign(h hash.Hash, priv *PrivateKey, config *Config) (err e } sig.Notations = append(sig.Notations, ¬ation) } - sig.outSubpackets, err = sig.buildSubpackets(priv.PublicKey) + sig.outSubpackets, err = sig.buildSubpackets(priv.PublicKey, config) if err != nil { return err } @@ -1254,11 +1254,11 @@ type outputSubpacket struct { contents []byte } -func (sig *Signature) buildSubpackets(issuer PublicKey) (subpackets []outputSubpacket, err error) { +func (sig *Signature) buildSubpackets(issuer PublicKey, config *Config) (subpackets []outputSubpacket, err error) { creationTime := make([]byte, 4) binary.BigEndian.PutUint32(creationTime, uint32(sig.CreationTime.Unix())) // Signature Creation Time - subpackets = append(subpackets, outputSubpacket{true, creationTimeSubpacket, true, creationTime}) + subpackets = append(subpackets, outputSubpacket{true, creationTimeSubpacket, !config.GenerateNonCriticalSignatureCreationTime(), creationTime}) // Signature Expiration Time if sig.SigLifetimeSecs != nil && *sig.SigLifetimeSecs != 0 { sigLifetime := make([]byte, 4) @@ -1288,7 +1288,9 @@ func (sig *Signature) buildSubpackets(issuer PublicKey) (subpackets []outputSubp if sig.IssuerKeyId != nil && sig.Version == 4 { keyId := make([]byte, 8) binary.BigEndian.PutUint64(keyId, *sig.IssuerKeyId) - subpackets = append(subpackets, outputSubpacket{true, issuerSubpacket, true, keyId}) + // Note: making this critical breaks RPM <=4.16. + // See: https://github.com/ProtonMail/go-crypto/issues/263 + subpackets = append(subpackets, outputSubpacket{true, issuerSubpacket, false, keyId}) } // Notation Data for _, notation := range sig.Notations { @@ -1355,7 +1357,7 @@ func (sig *Signature) buildSubpackets(issuer PublicKey) (subpackets []outputSubp if sig.FlagGroupKey { flags |= KeyFlagGroupKey } - subpackets = append(subpackets, outputSubpacket{true, keyFlagsSubpacket, true, []byte{flags}}) + subpackets = append(subpackets, outputSubpacket{true, keyFlagsSubpacket, !config.GenerateNonCriticalKeyFlags(), []byte{flags}}) } // Signer's User ID if sig.SignerUserId != nil { diff --git a/openpgp/packet/signature_test.go b/openpgp/packet/signature_test.go index edc431142..78ee37595 100644 --- a/openpgp/packet/signature_test.go +++ b/openpgp/packet/signature_test.go @@ -389,6 +389,108 @@ func TestSignatureWithTrustAndRegex(t *testing.T) { } } +func TestGenerateSignatureWithNonCriticalKeyFlagsSubpacket(t *testing.T) { + sig := &Signature{ + SigType: SigTypeGenericCert, + PubKeyAlgo: PubKeyAlgoRSA, + Hash: crypto.SHA256, + FlagsValid: true, + FlagSign: true, + } + + packet, err := Read(readerFromHex(rsaPkDataHex)) + if err != nil { + t.Fatalf("failed to deserialize public key: %v", err) + } + pubKey := packet.(*PublicKey) + + packet, err = Read(readerFromHex(privKeyRSAHex)) + if err != nil { + t.Fatalf("failed to deserialize private key: %v", err) + } + privKey := packet.(*PrivateKey) + + err = privKey.Decrypt([]byte("testing")) + if err != nil { + t.Fatalf("failed to decrypt private key: %v", err) + } + + err = sig.SignUserId("", pubKey, privKey, &Config{ + InsecureGenerateNonCriticalKeyFlags: true, + }) + if err != nil { + t.Errorf("failed to sign user id: %v", err) + } + + buf := bytes.NewBuffer([]byte{}) + err = sig.Serialize(buf) + if err != nil { + t.Errorf("failed to serialize signature: %v", err) + } + + packet, _ = Read(bytes.NewReader(buf.Bytes())) + sig = packet.(*Signature) + + for _, subPacket := range sig.rawSubpackets { + if subPacket.subpacketType == keyFlagsSubpacket { + if subPacket.isCritical { + t.Errorf("key flags subpacket is marked as critical") + } + } + } +} + +func TestGenerateSignatureWithNonCriticalSignatureCreationTimeSubpacket(t *testing.T) { + sig := &Signature{ + SigType: SigTypeGenericCert, + PubKeyAlgo: PubKeyAlgoRSA, + Hash: crypto.SHA256, + FlagsValid: true, + FlagSign: true, + } + + packet, err := Read(readerFromHex(rsaPkDataHex)) + if err != nil { + t.Fatalf("failed to deserialize public key: %v", err) + } + pubKey := packet.(*PublicKey) + + packet, err = Read(readerFromHex(privKeyRSAHex)) + if err != nil { + t.Fatalf("failed to deserialize private key: %v", err) + } + privKey := packet.(*PrivateKey) + + err = privKey.Decrypt([]byte("testing")) + if err != nil { + t.Fatalf("failed to decrypt private key: %v", err) + } + + err = sig.SignUserId("", pubKey, privKey, &Config{ + InsecureGenerateNonCriticalSignatureCreationTime: true, + }) + if err != nil { + t.Errorf("failed to sign user id: %v", err) + } + + buf := bytes.NewBuffer([]byte{}) + err = sig.Serialize(buf) + if err != nil { + t.Errorf("failed to serialize signature: %v", err) + } + + packet, _ = Read(bytes.NewReader(buf.Bytes())) + sig = packet.(*Signature) + + for _, subPacket := range sig.rawSubpackets { + if subPacket.subpacketType == creationTimeSubpacket { + if subPacket.isCritical { + t.Errorf("signature creation time subpacket is marked as critical") + } + } + } +} + func TestCanParseSignatureWithExportableCert(t *testing.T) { packet, err := Read(readerFromHex(signatureWithExportableCertHex)) if err != nil { diff --git a/openpgp/packet/symmetric_key_encrypted.go b/openpgp/packet/symmetric_key_encrypted.go index f843c35bf..2812a1db8 100644 --- a/openpgp/packet/symmetric_key_encrypted.go +++ b/openpgp/packet/symmetric_key_encrypted.go @@ -195,9 +195,21 @@ func SerializeSymmetricKeyEncrypted(w io.Writer, passphrase []byte, config *Conf // the given passphrase. The returned session key must be passed to // SerializeSymmetricallyEncrypted. // If config is nil, sensible defaults will be used. +// Deprecated: Use SerializeSymmetricKeyEncryptedAEADReuseKey instead. func SerializeSymmetricKeyEncryptedReuseKey(w io.Writer, sessionKey []byte, passphrase []byte, config *Config) (err error) { + return SerializeSymmetricKeyEncryptedAEADReuseKey(w, sessionKey, passphrase, config.AEAD() != nil, config) +} + +// SerializeSymmetricKeyEncryptedAEADReuseKey serializes a symmetric key packet to w. +// The packet contains the given session key, encrypted by a key derived from +// the given passphrase. The returned session key must be passed to +// SerializeSymmetricallyEncrypted. +// If aeadSupported is set, SKESK v6 is used, otherwise v4. +// Note: aeadSupported MUST match the value passed to SerializeSymmetricallyEncrypted. +// If config is nil, sensible defaults will be used. +func SerializeSymmetricKeyEncryptedAEADReuseKey(w io.Writer, sessionKey []byte, passphrase []byte, aeadSupported bool, config *Config) (err error) { var version int - if config.AEAD() != nil { + if aeadSupported { version = 6 } else { version = 4 diff --git a/openpgp/packet/symmetric_key_encrypted_data_test.go b/openpgp/packet/symmetric_key_encrypted_data_test.go index d8cf294f2..6aada24db 100644 --- a/openpgp/packet/symmetric_key_encrypted_data_test.go +++ b/openpgp/packet/symmetric_key_encrypted_data_test.go @@ -4,16 +4,17 @@ package packet // by an integrity protected packet (SEIPD v1 or v2). type packetSequence struct { - password string - packets string - contents string + password string + packets string + contents string + faultyDataPacket string } func keyAndIpePackets() []*packetSequence { if V5Disabled { - return []*packetSequence{symEncTestv6} + return []*packetSequence{symEncRFC9580, symEncRFC4880} } - return []*packetSequence{symEncTestv6, aeadEaxRFC, aeadOcbRFC} + return []*packetSequence{symEncRFC9580, symEncRFC4880, aeadEaxRFC, aeadOcbRFC} } // https://www.ietf.org/archive/id/draft-koch-openpgp-2015-rfc4880bis-00.html#name-complete-aead-eax-encrypted- @@ -30,9 +31,18 @@ var aeadOcbRFC = &packetSequence{ contents: "cb1462000000000048656c6c6f2c20776f726c64210a", } -// OpenPGP crypto refresh A.7.1. -var symEncTestv6 = &packetSequence{ +// From OpenPGP RFC9580 A.9 https://www.rfc-editor.org/rfc/rfc9580.html#name-sample-aead-eax-encryption- +var symEncRFC9580 = &packetSequence{ password: "password", - packets: "c33c061a07030b0308e9d39785b2070008ffb42e7c483ef4884457cb3726b9b3db9ff776e5f4d9a40952e2447298851abfff7526df2dd554417579a7799fd26902070306fcb94490bcb98bbdc9d106c6090266940f72e89edc21b5596b1576b101ed0f9ffc6fc6d65bbfd24dcd0790966e6d1e85a30053784cb1d8b6a0699ef12155a7b2ad6258531b57651fd7777912fa95e35d9b40216f69a4c248db28ff4331f1632907399e6ff9", - contents: "cb1362000000000048656c6c6f2c20776f726c6421d50e1ce2269a9eddef81032172b7ed7c", + packets: "c340061e07010b0308a5ae579d1fc5d82bff69224f919993b3506fa3b59a6a73cff8c5efc5f41c57fb54e1c226815d7828f5f92c454eb65ebe00ab5986c68e6e7c55d269020701069ff90e3b321964f3a42913c8dcc6619325015227efb7eaeaa49f04c2e674175d4a3d226ed6afcb9ca9ac122c1470e11c63d4c0ab241c6a938ad48bf99a5a99b90bba8325de61047540258ab7959a95ad051dda96eb15431dfef5f5e2255ca78261546e339a", + contents: "cb1362000000000048656c6c6f2c20776f726c6421d50eae5bf0cd6705500355816cb0c8ff", + // Missing last authentication chunk + faultyDataPacket: "d259020701069ff90e3b321964f3a42913c8dcc6619325015227efb7eaeaa49f04c2e674175d4a3d226ed6afcb9ca9ac122c1470e11c63d4c0ab241c6a938ad48bf99a5a99b90bba8325de61047540258ab7959a95ad051dda96eb", +} + +// From the OpenPGP interoperability test suite (Test: S2K mechanisms, iterated min + esk) +var symEncRFC4880 = &packetSequence{ + password: "password", + packets: "c32e0409030873616c7a6967657200080674a0d96a4a6e122b1d5bbaa3fac117b9cbb46c7e38f12967386b57e2f79d11d23f01cee77ceed8544e6d52c78bd33c81bd366c8673b68955ddbd1ade98fe6a9b4e27ae54cd10dda7cd3a4637f44e0ead895ebebdcf0c679f1342745628f104e7", + contents: "cb1462000000000048656c6c6f20576f726c64203a29", } diff --git a/openpgp/packet/symmetric_key_encrypted_test.go b/openpgp/packet/symmetric_key_encrypted_test.go index 6bb1f2a34..c96f13ec5 100644 --- a/openpgp/packet/symmetric_key_encrypted_test.go +++ b/openpgp/packet/symmetric_key_encrypted_test.go @@ -12,6 +12,7 @@ import ( mathrand "math/rand" "testing" + "github.com/ProtonMail/go-crypto/openpgp/errors" "github.com/ProtonMail/go-crypto/openpgp/s2k" ) @@ -20,25 +21,12 @@ const maxPassLen = 64 // Tests against RFC vectors func TestDecryptSymmetricKeyAndEncryptedDataPacket(t *testing.T) { for _, testCase := range keyAndIpePackets() { - // Key - buf := readerFromHex(testCase.packets) - packet, err := Read(buf) - if err != nil { - t.Fatalf("failed to read SymmetricKeyEncrypted: %s", err) - } - ske, ok := packet.(*SymmetricKeyEncrypted) - if !ok { - t.Fatal("didn't find SymmetricKeyEncrypted packet") - } - // Decrypt key - key, cipherFunc, err := ske.Decrypt([]byte(testCase.password)) - if err != nil { - t.Fatal(err) - } - packet, err = Read(buf) - if err != nil { - t.Fatalf("failed to read SymmetricallyEncrypted: %s", err) - } + // Read and verify the key packet + ske, dataPacket := readSymmetricKeyEncrypted(t, testCase.packets) + key, cipherFunc := decryptSymmetricKey(t, ske, []byte(testCase.password)) + + packet := readSymmetricallyEncrypted(t, dataPacket) + // Decrypt contents var edp EncryptedDataPacket switch p := packet.(type) { @@ -49,6 +37,7 @@ func TestDecryptSymmetricKeyAndEncryptedDataPacket(t *testing.T) { default: t.Fatal("no integrity protected packet") } + r, err := edp.Decrypt(cipherFunc, key) if err != nil { t.Fatal(err) @@ -66,6 +55,110 @@ func TestDecryptSymmetricKeyAndEncryptedDataPacket(t *testing.T) { } } +func TestTagVerificationError(t *testing.T) { + for _, testCase := range keyAndIpePackets() { + ske, dataPacket := readSymmetricKeyEncrypted(t, testCase.packets) + key, cipherFunc := decryptSymmetricKey(t, ske, []byte(testCase.password)) + + // Corrupt chunk + tmp := make([]byte, len(dataPacket)) + copy(tmp, dataPacket) + tmp[38] += 1 + packet := readSymmetricallyEncrypted(t, tmp) + // Decrypt contents and check integrity + checkIntegrityError(t, packet, cipherFunc, key) + + // Corrupt final tag or mdc + dataPacket[len(dataPacket)-1] += 1 + packet = readSymmetricallyEncrypted(t, dataPacket) + // Decrypt contents and check integrity + checkIntegrityError(t, packet, cipherFunc, key) + + if len(testCase.faultyDataPacket) > 0 { + dataPacket, err := hex.DecodeString(testCase.faultyDataPacket) + if err != nil { + t.Fatal(err) + } + packet = readSymmetricallyEncrypted(t, dataPacket) + // Decrypt contents and check integrity + checkIntegrityError(t, packet, cipherFunc, key) + } + } +} + +func readSymmetricKeyEncrypted(t *testing.T, packetHex string) (*SymmetricKeyEncrypted, []byte) { + t.Helper() + + buf := readerFromHex(packetHex) + packet, err := Read(buf) + if err != nil { + t.Fatalf("failed to read SymmetricKeyEncrypted: %s", err) + } + + ske, ok := packet.(*SymmetricKeyEncrypted) + if !ok { + t.Fatal("didn't find SymmetricKeyEncrypted packet") + } + + dataPacket, err := io.ReadAll(buf) + if err != nil { + t.Fatalf("failed to read data packet: %s", err) + } + return ske, dataPacket +} + +func decryptSymmetricKey(t *testing.T, ske *SymmetricKeyEncrypted, password []byte) ([]byte, CipherFunction) { + t.Helper() + + key, cipherFunc, err := ske.Decrypt(password) + if err != nil { + t.Fatalf("failed to decrypt symmetric key: %s", err) + } + + return key, cipherFunc +} + +func readSymmetricallyEncrypted(t *testing.T, dataPacket []byte) Packet { + t.Helper() + packet, err := Read(bytes.NewReader(dataPacket)) + if err != nil { + t.Fatalf("failed to read SymmetricallyEncrypted: %s", err) + } + return packet +} + +func checkIntegrityError(t *testing.T, packet Packet, cipherFunc CipherFunction, key []byte) { + t.Helper() + + switch p := packet.(type) { + case *SymmetricallyEncrypted: + edp := p + data, err := edp.Decrypt(cipherFunc, key) + if err != nil { + t.Fatal(err) + } + + _, err = io.ReadAll(data) + if err == nil { + err = data.Close() + } + if err != nil { + if edp.Version == 1 && err != errors.ErrMDCHashMismatch { + t.Fatalf("no integrity error (expected MDC hash mismatch)") + } + if edp.Version == 2 && err != errors.ErrAEADTagVerification { + t.Fatalf("no integrity error (expected AEAD tag verification failure)") + } + } else { + t.Fatalf("no error (expected integrity check failure)") + } + case *AEADEncrypted: + return + default: + t.Fatal("no integrity protected packet found") + } +} + func TestSerializeSymmetricKeyEncryptedV6RandomizeSlow(t *testing.T) { ciphers := map[string]CipherFunction{ "AES128": CipherAES128, diff --git a/openpgp/packet/symmetrically_encrypted.go b/openpgp/packet/symmetrically_encrypted.go index e9bbf0327..0e898742c 100644 --- a/openpgp/packet/symmetrically_encrypted.go +++ b/openpgp/packet/symmetrically_encrypted.go @@ -74,6 +74,10 @@ func (se *SymmetricallyEncrypted) Decrypt(c CipherFunction, key []byte) (io.Read // SerializeSymmetricallyEncrypted serializes a symmetrically encrypted packet // to w and returns a WriteCloser to which the to-be-encrypted packets can be // written. +// If aeadSupported is set to true, SEIPDv2 is used with the indicated CipherSuite. +// Otherwise, SEIPDv1 is used with the indicated CipherFunction. +// Note: aeadSupported MUST match the value passed to SerializeEncryptedKeyAEAD +// and/or SerializeSymmetricKeyEncryptedAEADReuseKey. // If config is nil, sensible defaults will be used. func SerializeSymmetricallyEncrypted(w io.Writer, c CipherFunction, aeadSupported bool, cipherSuite CipherSuite, key []byte, config *Config) (Contents io.WriteCloser, err error) { writeCloser := noOpCloser{w} diff --git a/openpgp/packet/symmetrically_encrypted_aead.go b/openpgp/packet/symmetrically_encrypted_aead.go index 3957b2d53..3ddc4fe4a 100644 --- a/openpgp/packet/symmetrically_encrypted_aead.go +++ b/openpgp/packet/symmetrically_encrypted_aead.go @@ -70,8 +70,10 @@ func (se *SymmetricallyEncrypted) decryptAead(inputKey []byte) (io.ReadCloser, e aead, nonce := getSymmetricallyEncryptedAeadInstance(se.Cipher, se.Mode, inputKey, se.Salt[:], se.associatedData()) // Carry the first tagLen bytes + chunkSize := decodeAEADChunkSize(se.ChunkSizeByte) tagLen := se.Mode.TagLength() - peekedBytes := make([]byte, tagLen) + chunkBytes := make([]byte, chunkSize+tagLen*2) + peekedBytes := chunkBytes[chunkSize+tagLen:] n, err := io.ReadFull(se.Contents, peekedBytes) if n < tagLen || (err != nil && err != io.EOF) { return nil, errors.StructuralError("not enough data to decrypt:" + err.Error()) @@ -81,12 +83,13 @@ func (se *SymmetricallyEncrypted) decryptAead(inputKey []byte) (io.ReadCloser, e aeadCrypter: aeadCrypter{ aead: aead, chunkSize: decodeAEADChunkSize(se.ChunkSizeByte), - initialNonce: nonce, + nonce: nonce, associatedData: se.associatedData(), - chunkIndex: make([]byte, 8), + chunkIndex: nonce[len(nonce)-8:], packetTag: packetTypeSymmetricallyEncryptedIntegrityProtected, }, reader: se.Contents, + chunkBytes: chunkBytes, peekedBytes: peekedBytes, }, nil } @@ -130,16 +133,20 @@ func serializeSymmetricallyEncryptedAead(ciphertext io.WriteCloser, cipherSuite aead, nonce := getSymmetricallyEncryptedAeadInstance(cipherSuite.Cipher, cipherSuite.Mode, inputKey, salt, prefix) + chunkSize := decodeAEADChunkSize(chunkSizeByte) + tagLen := aead.Overhead() + chunkBytes := make([]byte, chunkSize+tagLen) return &aeadEncrypter{ aeadCrypter: aeadCrypter{ aead: aead, - chunkSize: decodeAEADChunkSize(chunkSizeByte), + chunkSize: chunkSize, associatedData: prefix, - chunkIndex: make([]byte, 8), - initialNonce: nonce, + nonce: nonce, + chunkIndex: nonce[len(nonce)-8:], packetTag: packetTypeSymmetricallyEncryptedIntegrityProtected, }, - writer: ciphertext, + writer: ciphertext, + chunkBytes: chunkBytes, }, nil } @@ -149,10 +156,10 @@ func getSymmetricallyEncryptedAeadInstance(c CipherFunction, mode AEADMode, inpu encryptionKey := make([]byte, c.KeySize()) _, _ = readFull(hkdfReader, encryptionKey) - // Last 64 bits of nonce are the counter - nonce = make([]byte, mode.IvLength()-8) + nonce = make([]byte, mode.IvLength()) - _, _ = readFull(hkdfReader, nonce) + // Last 64 bits of nonce are the counter + _, _ = readFull(hkdfReader, nonce[:len(nonce)-8]) blockCipher := c.new(encryptionKey) aead = mode.new(blockCipher) diff --git a/openpgp/packet/symmetrically_encrypted_mdc.go b/openpgp/packet/symmetrically_encrypted_mdc.go index 0a3aecadf..8b1862368 100644 --- a/openpgp/packet/symmetrically_encrypted_mdc.go +++ b/openpgp/packet/symmetrically_encrypted_mdc.go @@ -148,7 +148,7 @@ const mdcPacketTagByte = byte(0x80) | 0x40 | 19 func (ser *seMDCReader) Close() error { if ser.error { - return errors.ErrMDCMissing + return errors.ErrMDCHashMismatch } for !ser.eof { @@ -159,7 +159,7 @@ func (ser *seMDCReader) Close() error { break } if err != nil { - return errors.ErrMDCMissing + return errors.ErrMDCHashMismatch } } @@ -172,7 +172,7 @@ func (ser *seMDCReader) Close() error { // The hash already includes the MDC header, but we still check its value // to confirm encryption correctness if ser.trailer[0] != mdcPacketTagByte || ser.trailer[1] != sha1.Size { - return errors.ErrMDCMissing + return errors.ErrMDCHashMismatch } return nil } diff --git a/openpgp/read.go b/openpgp/read.go index 8a69b44a5..5578797ed 100644 --- a/openpgp/read.go +++ b/openpgp/read.go @@ -233,7 +233,7 @@ FindKey: } mdFinal, sensitiveParsingErr := readSignedMessage(packets, md, keyring, config) if sensitiveParsingErr != nil { - return nil, errors.StructuralError("parsing error") + return nil, errors.HandleSensitiveParsingError(sensitiveParsingErr, md.decrypted != nil) } return mdFinal, nil } @@ -259,7 +259,7 @@ FindLiteralData: } switch p := p.(type) { case *packet.Compressed: - if err := packets.Push(p.Body); err != nil { + if err := packets.Push(p.LimitedBodyReader(config.DecompressedMessageSizeLimit())); err != nil { return nil, err } case *packet.OnePassSignature: @@ -368,7 +368,7 @@ func (cr *checkReader) Read(buf []byte) (int, error) { } if sensitiveParsingError != nil { - return n, errors.StructuralError("parsing error") + return n, errors.HandleSensitiveParsingError(sensitiveParsingError, true) } return n, nil @@ -392,6 +392,7 @@ func (scr *signatureCheckReader) Read(buf []byte) (int, error) { scr.wrappedHash.Write(buf[:n]) } + readsDecryptedData := scr.md.decrypted != nil if sensitiveParsingError == io.EOF { var p packet.Packet var readError error @@ -434,16 +435,15 @@ func (scr *signatureCheckReader) Read(buf []byte) (int, error) { // unsigned hash of its own. In order to check this we need to // close that Reader. if scr.md.decrypted != nil { - mdcErr := scr.md.decrypted.Close() - if mdcErr != nil { - return n, mdcErr + if sensitiveParsingError := scr.md.decrypted.Close(); sensitiveParsingError != nil { + return n, errors.HandleSensitiveParsingError(sensitiveParsingError, true) } } return n, io.EOF } if sensitiveParsingError != nil { - return n, errors.StructuralError("parsing error") + return n, errors.HandleSensitiveParsingError(sensitiveParsingError, readsDecryptedData) } return n, nil diff --git a/openpgp/read_test.go b/openpgp/read_test.go index 318d927ef..23fd4aec1 100644 --- a/openpgp/read_test.go +++ b/openpgp/read_test.go @@ -11,6 +11,7 @@ import ( "encoding/hex" "io" "io/ioutil" + "math/bits" "os" "strings" "testing" @@ -634,10 +635,13 @@ func TestReadV6Messages(t *testing.T) { t.Errorf("inline message is wrong: %s", contents) } } - func TestSymmetricDecryptionArgon2(t *testing.T) { + if bits.UintSize == 32 { + // 32-bit platforms cannot allocate 2GiB of RAM + // required by the test vector. + t.Skip() + } // Appendix IETF OpenPGP crypto refresh draft v08 A.8.1 - passphrase := []byte("password") file, err := os.Open("test_data/argon2-sym-message.asc") if err != nil { t.Fatal(err) @@ -646,6 +650,22 @@ func TestSymmetricDecryptionArgon2(t *testing.T) { if err != nil { t.Fatal(err) } + testSymmetricDecryptionArgon2Run(t, armoredEncryptedMessage) +} + +func TestSymmetricDecryptionArgon2LessMemory(t *testing.T) { + armoredEncryptedMessage := []byte(`-----BEGIN PGP MESSAGE----- + +w0gGJgcCFATa3KMW/4/9RsPME+un+MBqAwQQljCpv3dPfmVTFJAcqn+YRcIFrbY4 +iiVOkxM5uAKScyYn/T2su2j2fu+uPl/HpgLSWQIHAgx/1caHYWvwl7tyjJ/tSYwK +m8OMKQHidSWi7UM88mN17ltnLCV/Wa3bLDIyAgJr9XKubHXeUK6/FqmtPxepd4y/ +SXkqZq0XEafMIbynK2gH6JHjctFX +-----END PGP MESSAGE-----`) + testSymmetricDecryptionArgon2Run(t, armoredEncryptedMessage) +} + +func testSymmetricDecryptionArgon2Run(t *testing.T, armoredEncryptedMessage []byte) { + passphrase := []byte("password") // Unarmor string raw, err := armor.Decode(strings.NewReader(string(armoredEncryptedMessage))) if err != nil { @@ -799,7 +819,7 @@ func TestCorruptedMessageInvalidSigHeader(t *testing.T) { promptFunc := func(keys []Key, symmetric bool) ([]byte, error) { return passphrase, nil } - const expectedErr string = "openpgp: invalid data: parsing error" + const expectedErr string = "openpgp: decryption with session key failed: parsing error" _, observedErr := ReadMessage(raw.Body, nil, promptFunc, nil) if observedErr.Error() != expectedErr { t.Errorf("Expected error '%s', but got error '%s'", expectedErr, observedErr) @@ -813,7 +833,7 @@ func TestCorruptedMessageWrongLength(t *testing.T) { promptFunc := func(keys []Key, symmetric bool) ([]byte, error) { return passphrase, nil } - const expectedErr string = "openpgp: invalid data: parsing error" + const expectedErr string = "openpgp: decryption with session key failed: parsing error" file, err := os.Open("test_data/sym-corrupted-message-long-length.asc") if err != nil { diff --git a/openpgp/s2k/s2k.go b/openpgp/s2k/s2k.go index 925115807..6871b84fc 100644 --- a/openpgp/s2k/s2k.go +++ b/openpgp/s2k/s2k.go @@ -199,8 +199,8 @@ func Generate(rand io.Reader, c *Config) (*Params, error) { } params = &Params{ - mode: SaltedS2K, - hashId: hashId, + mode: SaltedS2K, + hashId: hashId, } } else { // Enforce IteratedSaltedS2K method otherwise hashId, ok := algorithm.HashToHashId(c.hash()) @@ -283,6 +283,9 @@ func ParseIntoParams(r io.Reader) (params *Params, err error) { params.passes = buf[Argon2SaltSize] params.parallelism = buf[Argon2SaltSize+1] params.memoryExp = buf[Argon2SaltSize+2] + if err := validateArgon2Params(params); err != nil { + return nil, err + } return params, nil case GnuS2K: // This is a GNU extension. See @@ -412,3 +415,22 @@ func Serialize(w io.Writer, key []byte, rand io.Reader, passphrase []byte, c *Co f(key, passphrase) return nil } + +// validateArgon2Params checks that the argon2 parameters are valid according to RFC9580. +func validateArgon2Params(params *Params) error { + // The number of passes t and the degree of parallelism p MUST be non-zero. + if params.parallelism == 0 { + return errors.StructuralError("invalid argon2 params: parallelism is 0") + } + if params.passes == 0 { + return errors.StructuralError("invalid argon2 params: iterations is 0") + } + + // The encoded memory size MUST be a value from 3+ceil(log2(p)) to 31, + // such that the decoded memory size m is a value from 8*p to 2^31. + if params.memoryExp > 31 || decodeMemory(params.memoryExp) < 8*uint32(params.parallelism) { + return errors.StructuralError("invalid argon2 params: memory is out of bounds") + } + + return nil +} diff --git a/openpgp/s2k/s2k_test.go b/openpgp/s2k/s2k_test.go index 8a0720574..030911bf2 100644 --- a/openpgp/s2k/s2k_test.go +++ b/openpgp/s2k/s2k_test.go @@ -276,3 +276,49 @@ func testSerializeConfigOK(t *testing.T, c *Config) *Params { return params } + +func TestValidateArgon2Params(t *testing.T) { + tests := []struct { + params Params + wantErr bool + }{ + { + params: Params{parallelism: 4, passes: 3, memoryExp: 6}, + wantErr: false, + }, + { + params: Params{parallelism: 0, passes: 3, memoryExp: 6}, + wantErr: true, + }, + { + params: Params{parallelism: 4, passes: 0, memoryExp: 6}, + wantErr: true, + }, + { + params: Params{parallelism: 4, passes: 3, memoryExp: 4}, + wantErr: true, + }, + { + params: Params{parallelism: 4, passes: 3, memoryExp: 32}, + wantErr: true, + }, + { + params: Params{parallelism: 4, passes: 3, memoryExp: 5}, + wantErr: false, + }, + { + params: Params{parallelism: 4, passes: 3, memoryExp: 31}, + wantErr: false, + }, + } + + for _, tt := range tests { + err := validateArgon2Params(&tt.params) + if tt.wantErr && err == nil { + t.Errorf("validateArgon2Params: expected an error") + } + if !tt.wantErr && err != nil { + t.Error("validateArgon2Params: expected no error") + } + } +} diff --git a/openpgp/v2/key_generation.go b/openpgp/v2/key_generation.go index 1617d48b4..f0767cf91 100644 --- a/openpgp/v2/key_generation.go +++ b/openpgp/v2/key_generation.go @@ -147,13 +147,15 @@ func (t *Entity) AddDirectKeySignature(selectedKeyProperties *keyProperties, con } func writeKeyProperties(selfSignature *packet.Signature, selectedKeyProperties *keyProperties) error { + advertiseAead := selectedKeyProperties.aead != nil + selfSignature.CreationTime = selectedKeyProperties.creationTime selfSignature.KeyLifetimeSecs = &selectedKeyProperties.keyLifetimeSecs selfSignature.FlagsValid = true selfSignature.FlagSign = true selfSignature.FlagCertify = true selfSignature.SEIPDv1 = true // true by default, see 5.8 vs. 5.14 - selfSignature.SEIPDv2 = selectedKeyProperties.aead != nil + selfSignature.SEIPDv2 = advertiseAead // Set the PreferredHash for the SelfSignature from the packet.Config. // If it is not the must-implement algorithm from rfc4880bis, append that. @@ -167,8 +169,8 @@ func writeKeyProperties(selfSignature *packet.Signature, selectedKeyProperties * // appropriate a matching hash function is available. acceptableHashes := acceptableHashesToWrite(&selectedKeyProperties.primaryKey.PublicKey) var match bool - for _, acceptableHashes := range acceptableHashes { - if acceptableHashes == hash { + for _, acceptableHash := range acceptableHashes { + if acceptableHash == hash { match = true break } @@ -197,18 +199,22 @@ func writeKeyProperties(selfSignature *packet.Signature, selectedKeyProperties * selfSignature.PreferredCompression = append(selfSignature.PreferredCompression, uint8(selectedKeyProperties.compression)) } - // And for DefaultMode. - modes := []uint8{uint8(selectedKeyProperties.aead.Mode())} - if selectedKeyProperties.aead.Mode() != packet.AEADModeOCB { - modes = append(modes, uint8(packet.AEADModeOCB)) - } + if advertiseAead { + // Get the preferred AEAD mode from the packet.Config. + // If it is not the must-implement algorithm from rfc9580, append that. + modes := []uint8{uint8(selectedKeyProperties.aead.Mode())} + if selectedKeyProperties.aead.Mode() != packet.AEADModeOCB { + modes = append(modes, uint8(packet.AEADModeOCB)) + } - // For preferred (AES256, GCM), we'll generate (AES256, GCM), (AES256, OCB), (AES128, GCM), (AES128, OCB) - for _, cipher := range selfSignature.PreferredSymmetric { - for _, mode := range modes { - selfSignature.PreferredCipherSuites = append(selfSignature.PreferredCipherSuites, [2]uint8{cipher, mode}) + // For preferred (AES256, GCM), we'll generate (AES256, GCM), (AES256, OCB), (AES128, GCM), (AES128, OCB) + for _, cipher := range selfSignature.PreferredSymmetric { + for _, mode := range modes { + selfSignature.PreferredCipherSuites = append(selfSignature.PreferredCipherSuites, [2]uint8{cipher, mode}) + } } } + return nil } diff --git a/openpgp/v2/keys.go b/openpgp/v2/keys.go index b4a7cc1e8..2ff1202b3 100644 --- a/openpgp/v2/keys.go +++ b/openpgp/v2/keys.go @@ -61,7 +61,8 @@ func (e *Entity) PrimaryIdentity(date time.Time, config *packet.Config) (*packet var primaryIdentityCandidatesSelfSigs []*packet.Signature for _, identity := range e.Identities { selfSig, err := identity.Verify(date, config) // identity must be valid at date - if err == nil { // verification is successful + if err == nil { + // verification is successful primaryIdentityCandidates = append(primaryIdentityCandidates, identity) primaryIdentityCandidatesSelfSigs = append(primaryIdentityCandidatesSelfSigs, selfSig) } @@ -97,27 +98,62 @@ func shouldPreferIdentity(existingId, potentialNewId *packet.Signature) bool { // EncryptionKey returns the best candidate Key for encrypting a message to the // given Entity. func (e *Entity) EncryptionKey(now time.Time, config *packet.Config) (Key, bool) { + encryptionKey, err := e.EncryptionKeyWithError(now, config) + return encryptionKey, err == nil +} + +// EncryptionKeyWithError returns the best candidate Key for encrypting a message to the +// given Entity. +// Provides an error if the function fails to find an encryption key. +func (e *Entity) EncryptionKeyWithError(now time.Time, config *packet.Config) (Key, error) { // The primary key has to be valid at time now primarySelfSignature, err := e.VerifyPrimaryKey(now, config) if err != nil { // primary key is not valid - return Key{}, false + return Key{}, errors.ErrEncryptionKeySelection{ + PrimaryKeyId: e.PrimaryKey.KeyIdString(), + PrimaryKeyErr: err, + } } - if checkKeyRequirements(e.PrimaryKey, config) != nil { + if err := checkKeyRequirements(e.PrimaryKey, config); err != nil { // The primary key produces weak signatures - return Key{}, false + return Key{}, errors.ErrEncryptionKeySelection{ + PrimaryKeyId: e.PrimaryKey.KeyIdString(), + PrimaryKeyErr: err, + } } // Iterate the keys to find the newest, unexpired one + var latestSelectionError *errors.ErrEncryptionKeySelection candidateSubkey := -1 var maxTime time.Time var selectedSubkeySelfSig *packet.Signature for i, subkey := range e.Subkeys { + subkeyErr := func(encSelectionErr error) *errors.ErrEncryptionKeySelection { + subkeyKeyId := subkey.PublicKey.KeyIdString() + return &errors.ErrEncryptionKeySelection{ + PrimaryKeyId: e.PrimaryKey.KeyIdString(), + EncSelectionKeyId: &subkeyKeyId, + EncSelectionErr: encSelectionErr, + } + + } + // Verify the subkey signature. subkeySelfSig, err := subkey.Verify(now, config) // subkey has to be valid at time now - if err == nil && - isValidEncryptionKey(subkeySelfSig, subkey.PublicKey.PubKeyAlgo) && - checkKeyRequirements(subkey.PublicKey, config) == nil && - (maxTime.IsZero() || subkeySelfSig.CreationTime.Unix() >= maxTime.Unix()) { + if err != nil { + latestSelectionError = subkeyErr(err) + continue + } + // Check the algorithm and key flags. + if !isValidEncryptionKey(subkeySelfSig, subkey.PublicKey.PubKeyAlgo, config) { + continue + } + // Check if the key fulfils the requirements + if err := checkKeyRequirements(subkey.PublicKey, config); err != nil { + latestSelectionError = subkeyErr(err) + continue + } + if maxTime.IsZero() || subkeySelfSig.CreationTime.Unix() >= maxTime.Unix() { candidateSubkey = i selectedSubkeySelfSig = subkeySelfSig maxTime = subkeySelfSig.CreationTime @@ -132,22 +168,29 @@ func (e *Entity) EncryptionKey(now time.Time, config *packet.Config) (Key, bool) PublicKey: subkey.PublicKey, PrivateKey: subkey.PrivateKey, SelfSignature: selectedSubkeySelfSig, - }, true + }, nil } // If we don't have any subkeys for encryption and the primary key // is marked as OK to encrypt with, then we can use it. - if isValidEncryptionKey(primarySelfSignature, e.PrimaryKey.PubKeyAlgo) { + if isValidEncryptionKey(primarySelfSignature, e.PrimaryKey.PubKeyAlgo, config) { return Key{ Entity: e, PrimarySelfSignature: primarySelfSignature, PublicKey: e.PrimaryKey, PrivateKey: e.PrivateKey, SelfSignature: primarySelfSignature, - }, true + }, nil } - return Key{}, false + if latestSelectionError == nil { + latestSelectionError = &errors.ErrEncryptionKeySelection{ + PrimaryKeyId: e.PrimaryKey.KeyIdString(), + EncSelectionErr: goerrors.New("no encryption-capable key found (no key flags or invalid algorithm)"), + } + } + + return Key{}, latestSelectionError } // DecryptionKeys returns all keys that are available for decryption, matching the keyID when given @@ -163,12 +206,12 @@ func (e *Entity) DecryptionKeys(id uint64, date time.Time, config *packet.Config for _, subkey := range e.Subkeys { subkeySelfSig, err := subkey.LatestValidBindingSignature(date, config) if err == nil && - isValidEncryptionKey(subkeySelfSig, subkey.PublicKey.PubKeyAlgo) && + (config.AllowDecryptionWithSigningKeys() || isValidEncryptionKey(subkeySelfSig, subkey.PublicKey.PubKeyAlgo, config)) && (id == 0 || subkey.PublicKey.KeyId == id) { keys = append(keys, Key{subkey.Primary, primarySelfSignature, subkey.PublicKey, subkey.PrivateKey, subkeySelfSig}) } } - if isValidEncryptionKey(primarySelfSignature, e.PrimaryKey.PubKeyAlgo) { + if config.AllowDecryptionWithSigningKeys() || isValidEncryptionKey(primarySelfSignature, e.PrimaryKey.PubKeyAlgo, config) { keys = append(keys, Key{e, primarySelfSignature, e.PrimaryKey, e.PrivateKey, primarySelfSignature}) } return @@ -218,8 +261,8 @@ func (e *Entity) signingKeyByIdUsage(now time.Time, id uint64, flags int, config for idx, subkey := range e.Subkeys { subkeySelfSig, err := subkey.Verify(now, config) if err == nil && - (flags&packet.KeyFlagCertify == 0 || isValidCertificationKey(subkeySelfSig, subkey.PublicKey.PubKeyAlgo)) && - (flags&packet.KeyFlagSign == 0 || isValidSigningKey(subkeySelfSig, subkey.PublicKey.PubKeyAlgo)) && + (flags&packet.KeyFlagCertify == 0 || isValidCertificationKey(subkeySelfSig, subkey.PublicKey.PubKeyAlgo, config)) && + (flags&packet.KeyFlagSign == 0 || isValidSigningKey(subkeySelfSig, subkey.PublicKey.PubKeyAlgo, config)) && checkKeyRequirements(subkey.PublicKey, config) == nil && (maxTime.IsZero() || subkeySelfSig.CreationTime.Unix() >= maxTime.Unix()) && (id == 0 || subkey.PublicKey.KeyId == id) { @@ -242,8 +285,8 @@ func (e *Entity) signingKeyByIdUsage(now time.Time, id uint64, flags int, config // If we don't have any subkeys for signing and the primary key // is marked as OK to sign with, then we can use it. - if (flags&packet.KeyFlagCertify == 0 || isValidCertificationKey(primarySelfSignature, e.PrimaryKey.PubKeyAlgo)) && - (flags&packet.KeyFlagSign == 0 || isValidSigningKey(primarySelfSignature, e.PrimaryKey.PubKeyAlgo)) && + if (flags&packet.KeyFlagCertify == 0 || isValidCertificationKey(primarySelfSignature, e.PrimaryKey.PubKeyAlgo, config)) && + (flags&packet.KeyFlagSign == 0 || isValidSigningKey(primarySelfSignature, e.PrimaryKey.PubKeyAlgo, config)) && (id == 0 || e.PrimaryKey.KeyId == id) { return Key{ Entity: e, @@ -675,7 +718,7 @@ func (e *Entity) LatestValidDirectSignature(date time.Time, config *packet.Confi if sig.Valid == nil { err := e.PrimaryKey.VerifyDirectKeySignature(sig.Packet) if err == nil { - err = checkSignatureDetails(e.PrimaryKey, sig.Packet, date, config) + err = checkSignatureDetails(sig.Packet, date, config) } valid := err == nil sig.Valid = &valid @@ -688,7 +731,7 @@ func (e *Entity) LatestValidDirectSignature(date time.Time, config *packet.Confi if selectedSig == nil { return nil, errors.StructuralError("no valid direct key signature found") } - return + return selectedSig, nil } // PrimarySelfSignature searches the entity for the self-signature that stores key preferences. @@ -769,20 +812,38 @@ func checkKeyRequirements(usedKey *packet.PublicKey, config *packet.Config) erro return nil } -func isValidSigningKey(signature *packet.Signature, algo packet.PublicKeyAlgorithm) bool { - return algo.CanSign() && - signature.FlagsValid && - signature.FlagSign +func isValidSigningKey(signature *packet.Signature, algo packet.PublicKeyAlgorithm, config *packet.Config) bool { + if !algo.CanSign() { + return false + } + + if signature.FlagsValid { + return signature.FlagSign + } + + return config.AllowAllKeyFlagsWhenMissing() } -func isValidCertificationKey(signature *packet.Signature, algo packet.PublicKeyAlgorithm) bool { - return algo.CanSign() && - signature.FlagsValid && - signature.FlagCertify +func isValidCertificationKey(signature *packet.Signature, algo packet.PublicKeyAlgorithm, config *packet.Config) bool { + if !algo.CanSign() { + return false + } + + if signature.FlagsValid { + return signature.FlagCertify + } + + return config.AllowAllKeyFlagsWhenMissing() } -func isValidEncryptionKey(signature *packet.Signature, algo packet.PublicKeyAlgorithm) bool { - return algo.CanEncrypt() && - signature.FlagsValid && - (signature.FlagEncryptCommunications || signature.FlagEncryptStorage) +func isValidEncryptionKey(signature *packet.Signature, algo packet.PublicKeyAlgorithm, config *packet.Config) bool { + if !algo.CanEncrypt() { + return false + } + + if signature.FlagsValid { + return signature.FlagEncryptCommunications || signature.FlagEncryptStorage + } + + return config.AllowAllKeyFlagsWhenMissing() } diff --git a/openpgp/v2/keys_test.go b/openpgp/v2/keys_test.go index 0b276c23e..47f2d4930 100644 --- a/openpgp/v2/keys_test.go +++ b/openpgp/v2/keys_test.go @@ -585,7 +585,6 @@ func TestKeyWithRevokedSubKey(t *testing.T) { if len(subKey.Bindings) == 0 { t.Fatalf("no binding subkey signature") } - } func TestSubkeyRevocation(t *testing.T) { @@ -683,7 +682,6 @@ func TestKeyWithSubKeyAndBadSelfSigOrder(t *testing.T) { if lifetime := selfSig.KeyLifetimeSecs; lifetime != nil { t.Errorf("The signature has a key lifetime (%d), but it should be nil", *lifetime) } - } func TestIdVerification(t *testing.T) { @@ -1063,6 +1061,7 @@ func TestAddUserId(t *testing.T) { t.Fatal(err) } } + func TestAddSubkey(t *testing.T) { entity, err := NewEntity("Golang Gopher", "Test Key", "no-reply@golang.com", nil) if err != nil { @@ -2022,3 +2021,91 @@ NciH07RTRuMS/aRhRg4OB8PQROmTnZ+iZS0= t.Fatal(err) } } + +func TestAllowAllKeyFlagsWhenMissing(t *testing.T) { + // Make a master key. + entity, err := NewEntity("Golang Gopher", "Test Key", "no-reply@golang.com", nil) + if err != nil { + t.Fatal(err) + } + + config := &packet.Config{} + + primarySelfSignature, err := entity.VerifyPrimaryKey(time.Now(), config) + if err != nil { + t.Fatal(err) + } + + if !entity.PrimaryKey.PubKeyAlgo.CanEncrypt() || + !entity.PrimaryKey.PubKeyAlgo.CanSign() { + t.Fatal("PubKeyAlgo must be valid for signature and encryption") + } + + /// Flags valid, but not set. + primarySelfSignature.FlagsValid = true + primarySelfSignature.FlagSign = false + primarySelfSignature.FlagCertify = false + primarySelfSignature.FlagEncryptCommunications = false + + if isValidSigningKey(primarySelfSignature, entity.PrimaryKey.PubKeyAlgo, config) { + t.Error("isValidSigningKey must be false") + } + + if isValidEncryptionKey(primarySelfSignature, entity.PrimaryKey.PubKeyAlgo, config) { + t.Error("isValidEncryptionKey must be false") + } + + if isValidCertificationKey(primarySelfSignature, entity.PrimaryKey.PubKeyAlgo, config) { + t.Error("isValidCertificationKey must be false") + } + + /// Flags not valid, but InsecureAllowAllKeyFlagsWhenMissing set. + primarySelfSignature.FlagsValid = false + config = &packet.Config{InsecureAllowAllKeyFlagsWhenMissing: true} + + if !isValidSigningKey(primarySelfSignature, entity.PrimaryKey.PubKeyAlgo, config) { + t.Error("isValidSigningKey must be true when InsecureAllowAllKeyFlagsWhenMissing is true") + } + + if !isValidEncryptionKey(primarySelfSignature, entity.PrimaryKey.PubKeyAlgo, config) { + t.Error("isValidEncryptionKey must be true when InsecureAllowAllKeyFlagsWhenMissing is true") + } + + if !isValidCertificationKey(primarySelfSignature, entity.PrimaryKey.PubKeyAlgo, config) { + t.Error("isValidCertificationKey must be true when InsecureAllowAllKeyFlagsWhenMissing is true") + } +} + +func TestEncryptionKeyError(t *testing.T) { + // Make a master key. + entity, err := NewEntity("Golang Gopher", "Test Key", "no-reply@golang.com", nil) + if err != nil { + t.Fatal(err) + } + + _, err = entity.EncryptionKeyWithError(time.Unix(1405544146, 0), nil) + if err == nil { + t.Fatal("should fail") + } + if !strings.Contains(err.Error(), "no valid self signature found") { + t.Fatal("wrong error") + } + + entity.Subkeys[0].PublicKey.Version = 20 + _, err = entity.EncryptionKeyWithError(time.Now(), nil) + if err == nil { + t.Fatal("should fail") + } + if !strings.Contains(err.Error(), "no valid binding signature found for subkey") { + t.Fatal("wrong error") + } + + entity.Subkeys = nil + _, err = entity.EncryptionKeyWithError(time.Now(), nil) + if err == nil { + t.Fatal("should fail") + } + if !strings.Contains(err.Error(), "no encryption-capable key found (no key flags or invalid algorithm)") { + t.Fatal("wrong error") + } +} diff --git a/openpgp/v2/read.go b/openpgp/v2/read.go index 24c8c8f0d..7980c9b2a 100644 --- a/openpgp/v2/read.go +++ b/openpgp/v2/read.go @@ -268,7 +268,7 @@ FindKey: } mdFinal, sensitiveParsingErr := readSignedMessage(packets, md, keyring, config) if sensitiveParsingErr != nil { - return nil, errors.StructuralError("parsing error") + return nil, errors.HandleSensitiveParsingError(sensitiveParsingErr, md.decrypted != nil) } return mdFinal, nil } @@ -364,7 +364,7 @@ FindLiteralData: } switch p := p.(type) { case *packet.Compressed: - if err := packets.Push(p.Body); err != nil { + if err := packets.Push(p.LimitedBodyReader(config.DecompressedMessageSizeLimit())); err != nil { return nil, err } case *packet.OnePassSignature: @@ -481,15 +481,15 @@ func (cr *checkReader) Read(buf []byte) (int, error) { } } if cr.md.decrypted != nil { - if mdcErr := cr.md.decrypted.Close(); mdcErr != nil { - return n, mdcErr + if sensitiveParsingError := cr.md.decrypted.Close(); sensitiveParsingError != nil { + return n, errors.HandleSensitiveParsingError(sensitiveParsingError, true) } } cr.checked = true return n, io.EOF } if sensitiveParsingError != nil { - return n, errors.StructuralError("parsing error") + return n, errors.HandleSensitiveParsingError(sensitiveParsingError, true) } return n, nil } @@ -513,18 +513,19 @@ func (scr *signatureCheckReader) Read(buf []byte) (int, error) { } } + readsDecryptedData := scr.md.decrypted != nil if sensitiveParsingError == io.EOF { - var signatures []*packet.Signature - // Read all signature packets. + var signatures []*packet.Signature + // Read all signature packets and discard others. for { - p, err := scr.packets.Next() - if err == io.EOF { + p, sensitiveParsingErrorPacket := scr.packets.Next() + if sensitiveParsingErrorPacket == io.EOF { break } - if err != nil { - return n, errors.StructuralError("parsing error") + if sensitiveParsingErrorPacket != nil { + return n, errors.HandleSensitiveParsingError(sensitiveParsingErrorPacket, readsDecryptedData) } if sig, ok := p.(*packet.Signature); ok { if sig.Version == 5 && scr.md.LiteralData != nil && (sig.SigType == 0x00 || sig.SigType == 0x01) { @@ -533,6 +534,15 @@ func (scr *signatureCheckReader) Read(buf []byte) (int, error) { signatures = append(signatures, sig) } } + + // Verify the integrity of the decrypted data before verifying the signatures. + if scr.md.decrypted != nil { + sensitiveParsingErrorPacket := scr.md.decrypted.Close() + if sensitiveParsingErrorPacket != nil { + return n, errors.HandleSensitiveParsingError(sensitiveParsingErrorPacket, true) + } + } + numberOfOpsSignatures := 0 for _, candidate := range scr.md.SignatureCandidates { if candidate.CorrespondingSig == nil { @@ -625,32 +635,11 @@ func (scr *signatureCheckReader) Read(buf []byte) (int, error) { } scr.md.IsVerified = true - - for { - _, err := scr.packets.Next() - if err == io.EOF { - break - } - if err != nil { - return 0, errors.StructuralError("parsing error") - } - - } - - // The SymmetricallyEncrypted packet, if any, might have an - // unsigned hash of its own. In order to check this we need to - // close that Reader. - if scr.md.decrypted != nil { - mdcErr := scr.md.decrypted.Close() - if mdcErr != nil { - return n, mdcErr - } - } return n, io.EOF } if sensitiveParsingError != nil { - return n, errors.StructuralError("parsing error") + return n, errors.HandleSensitiveParsingError(sensitiveParsingError, readsDecryptedData) } return n, nil @@ -741,21 +730,16 @@ func verifyDetachedSignatureReader(keyring KeyRing, signed, signature io.Reader, // checkSignatureDetails verifies the metadata of the signature. // It checks the following: -// - Hash function should not be invalid according to -// config.RejectHashAlgorithms. -// - Verification key must be older than the signature creation time. -// - Check signature notations. -// - Signature is not expired (unless a zero time is passed to -// explicitly ignore expiration). -func checkSignatureDetails(pk *packet.PublicKey, signature *packet.Signature, now time.Time, config *packet.Config) error { +// - Hash function should not be invalid according to +// config.RejectHashAlgorithms. +// - Check signature notations. +// - Signature is not expired (unless a zero time is passed to +// explicitly ignore expiration). +func checkSignatureDetails(signature *packet.Signature, now time.Time, config *packet.Config) error { if config.RejectHashAlgorithm(signature.Hash) { return errors.SignatureError("insecure hash algorithm: " + signature.Hash.String()) } - if pk.CreationTime.Unix() > signature.CreationTime.Unix() { - return errors.ErrSignatureOlderThanKey - } - for _, notation := range signature.Notations { if notation.IsCritical && !config.KnownNotation(notation.Name) { return errors.SignatureError("unknown critical notation: " + notation.Name) @@ -773,30 +757,29 @@ func checkSignatureDetails(pk *packet.PublicKey, signature *packet.Signature, no // signature and all relevant binding signatures. // In addition, the message signature hash algorithm is checked against // config.RejectMessageHashAlgorithms. +// Finally, the signature must be newer than the verification key. func checkMessageSignatureDetails(verifiedKey *Key, signature *packet.Signature, config *packet.Config) error { if config.RejectMessageHashAlgorithm(signature.Hash) { return errors.SignatureError("insecure message hash algorithm: " + signature.Hash.String()) } + if signature.CreationTime.Unix() < verifiedKey.PublicKey.CreationTime.Unix() { + return errors.ErrSignatureOlderThanKey + } + sigsToCheck := []*packet.Signature{signature, verifiedKey.PrimarySelfSignature} if !verifiedKey.IsPrimary() { sigsToCheck = append(sigsToCheck, verifiedKey.SelfSignature, verifiedKey.SelfSignature.EmbeddedSignature) } var errs []error for _, sig := range sigsToCheck { - var pk *packet.PublicKey - if sig == verifiedKey.PrimarySelfSignature || sig == verifiedKey.SelfSignature { - pk = verifiedKey.Entity.PrimaryKey - } else { - pk = verifiedKey.PublicKey - } var time time.Time if sig == signature { time = config.Now() } else { time = signature.CreationTime } - if err := checkSignatureDetails(pk, sig, time, config); err != nil { + if err := checkSignatureDetails(sig, time, config); err != nil { errs = append(errs, err) } } diff --git a/openpgp/v2/read_test.go b/openpgp/v2/read_test.go index 2feaf392f..f167c34e2 100644 --- a/openpgp/v2/read_test.go +++ b/openpgp/v2/read_test.go @@ -9,8 +9,10 @@ import ( "crypto/sha512" "encoding/base64" "encoding/hex" + goerrors "errors" "io" "io/ioutil" + "math/bits" "os" "strings" "testing" @@ -667,8 +669,12 @@ func TestReadV6Messages(t *testing.T) { } func TestSymmetricDecryptionArgon2(t *testing.T) { + if bits.UintSize == 32 { + // 32-bit platforms cannot allocate 2GiB of RAM + // required by the test vector. + t.Skip() + } // Appendix IETF OpenPGP crypto refresh draft v08 A.8.1 - passphrase := []byte("password") file, err := os.Open("../test_data/argon2-sym-message.asc") if err != nil { t.Fatal(err) @@ -677,6 +683,22 @@ func TestSymmetricDecryptionArgon2(t *testing.T) { if err != nil { t.Fatal(err) } + testSymmetricDecryptionArgon2Run(t, armoredEncryptedMessage) +} + +func TestSymmetricDecryptionArgon2LessMemory(t *testing.T) { + armoredEncryptedMessage := []byte(`-----BEGIN PGP MESSAGE----- + +w0gGJgcCFATa3KMW/4/9RsPME+un+MBqAwQQljCpv3dPfmVTFJAcqn+YRcIFrbY4 +iiVOkxM5uAKScyYn/T2su2j2fu+uPl/HpgLSWQIHAgx/1caHYWvwl7tyjJ/tSYwK +m8OMKQHidSWi7UM88mN17ltnLCV/Wa3bLDIyAgJr9XKubHXeUK6/FqmtPxepd4y/ +SXkqZq0XEafMIbynK2gH6JHjctFX +-----END PGP MESSAGE-----`) + testSymmetricDecryptionArgon2Run(t, armoredEncryptedMessage) +} + +func testSymmetricDecryptionArgon2Run(t *testing.T, armoredEncryptedMessage []byte) { + passphrase := []byte("password") // Unarmor string raw, err := armor.Decode(strings.NewReader(string(armoredEncryptedMessage))) if err != nil { @@ -830,7 +852,7 @@ func TestCorruptedMessageInvalidSigHeader(t *testing.T) { promptFunc := func(keys []Key, symmetric bool) ([]byte, error) { return passphrase, nil } - const expectedErr string = "openpgp: invalid data: parsing error" + const expectedErr string = "openpgp: decryption with session key failed: parsing error" _, observedErr := ReadMessage(raw.Body, nil, promptFunc, nil) if observedErr.Error() != expectedErr { t.Errorf("Expected error '%s', but got error '%s'", expectedErr, observedErr) @@ -844,7 +866,7 @@ func TestCorruptedMessageWrongLength(t *testing.T) { promptFunc := func(keys []Key, symmetric bool) ([]byte, error) { return passphrase, nil } - const expectedErr string = "openpgp: invalid data: parsing error" + const expectedErr string = "openpgp: decryption with session key failed: parsing error" file, err := os.Open("../test_data/sym-corrupted-message-long-length.asc") if err != nil { @@ -1002,3 +1024,85 @@ func testMalformedMessage(t *testing.T, keyring EntityList, message string) { return } } + +func TestReadMessageWithSignOnly(t *testing.T) { + config := packet.Config{ + InsecureAllowDecryptionWithSigningKeys: true, + } + key, err := ReadArmoredKeyRing(strings.NewReader(rsaSignOnly)) + if err != nil { + t.Error(err) + return + } + // Success + msgReader, err := armor.Decode(strings.NewReader(armoredMessageRsaSignOnly)) + if err != nil { + t.Error(err) + return + } + md, err := ReadMessage(msgReader.Body, key, nil, &config) + if err != nil { + t.Error(err) + return + } + _, err = io.ReadAll(md.UnverifiedBody) + if err != nil { + t.Error(err) + return + } + + // Fail + msgReader, err = armor.Decode(strings.NewReader(armoredMessageRsaSignOnly)) + if err != nil { + t.Error(err) + return + } + _, err = ReadMessage(msgReader.Body, key, nil, nil) + if err == nil { + t.Fatal("Should not decrypt") + } +} + +func TestReadMessageCompressionLimit(t *testing.T) { + // Limit 1KB + max := int64(1024) + config := packet.Config{ + MaxDecompressedMessageSize: &max, + } + + msgReader, err := armor.Decode(strings.NewReader(compressed2KB)) + if err != nil { + t.Error(err) + return + } + md, err := ReadMessage(msgReader.Body, nil, nil, &config) + if err != nil { + t.Error(err) + return + } + // Should not be able to read all data + _, err = io.ReadAll(md.UnverifiedBody) + if !goerrors.Is(err, errors.ErrMessageTooLarge) { + t.Errorf("Wrong error") + } + + // Limit 4KB + max = int64(4 * 1024) + config = packet.Config{ + MaxDecompressedMessageSize: &max, + } + msgReader, err = armor.Decode(strings.NewReader(compressed2KB)) + if err != nil { + t.Error(err) + return + } + md, err = ReadMessage(msgReader.Body, nil, nil, &config) + if err != nil { + t.Error(err) + return + } + // Should be able to read all data + if _, err = io.ReadAll(md.UnverifiedBody); err != nil { + t.Fatal(err) + } +} diff --git a/openpgp/v2/read_write_test_data.go b/openpgp/v2/read_write_test_data.go index 2f0efc228..802387030 100644 --- a/openpgp/v2/read_write_test_data.go +++ b/openpgp/v2/read_write_test_data.go @@ -740,3 +740,105 @@ NVniEke6hM3CNBXYPAMhQBMWhCulcoz+0lxi8L34rMN+Dsbma96psdUrn7uLaB91 xqAY9Bwizt4FWgXuLm1a4+So4V9j1TRCXd12Uc2l2RNmgDE= =miES -----END PGP PRIVATE KEY BLOCK-----` + +const rsaSignOnly = `-----BEGIN PGP PRIVATE KEY BLOCK----- + +xcLYBF9Gl+MBCACc09O3gjyO0B1ledGxGFSUpPmhhJzkxKoY1WDX8VlASCHz +bAA/BytgYBXHTe7N+N3yJ6uiN3DIQ2j5uGWk/h5jyIOsRuzQxJ40n8AdK/71 +SGDCG1X5l1h9vmVTJxkQ3pcOxqRg55EEuJWKN1v7B1hIPxhaM5hgH/7s+PNn +lQddckQJqYkpm9Hy6EI7f9oHrOtWJWZoCHkWZVld7+9ZVPi34ex5ofYOuvNL +AIKZCc7lAiUiDJYQ+hIJRoYwLYhjIshpYoHgNeG4snlupNO32BiwDbHFDjeu +eoBLQ0rxZV7B664ceCmIl+VRht9G20hfGoTjAiop5tyrN1ZeL4EaI+aTABEB +AAEAB/oCKTQnftvHwrkBVlyzSN6tfXylF2551Q3n4CZGg3efI/9PCa9wF58+ +WApqmgsUqcNbVnDfl2T58ow05FLMxnFFNnHJq8ltfnXl+gG6c7iy94p79SQE +AGCOL7xNassXrDAQZhqWkCdiLK3b6r9F8Y3URb/AYbWH2BkFkS0oWQDav+Tw +lABt5vG2L5QtnShdqi8CCitcHGEKHocPHp0yAQlp3oAMq09YubgrzESDJ7Pe +l93cT35NlyimAZ6IYk/gumX0/6spqcw7205KfG6P84WlMp3WmE0IUWtiOp+7 +rjMjDki0WeVKtuLbHBhOwKvxcILWz+0vQf3uu6aXOKQ3JlsVBADHoXa6QjrT +RmKD9ch65Pkd+EZiKhe+pqqIArVj4QsVBEnaggR59SD8uXhtpyBnvOp3xpof +Vut3SKWl/jmH7vKansFbHOo8xLUyVctu7lCL2/v85FcRJxfPK00MBic+z/vf +mWOAY1VBoi5I9qi6o8vVHA5BJ/xw2uV9VpxfiLG0vwQAyRxHmoZl/OxaZUsm +J9eDYV9xyYumkTCYvHPk9X+ehS+XeYh24z1q9a/1jEnSR3A5XE67UCLaspiA ++Px7nSU1+ftJ9bC2bnRR0Upop+3UkPeCBVp4tYAhsNnPXhSWC0gCgeGU7EmW +DechFg29LId35LXKgmXls9u5yDy2w978Hy0D/jbKZaxNMMwlx/XCFCoBEcXS +DBzg7GHNXdillJqy215j46lfVqOCB3IiffNKjHua2l6fQc0BoiWIZnElMnIa +faEBBSpOVqKhktDFacHa5xChjqXZVyw68qc0I36xkCfcwvYCpNKKkXv90r8A +tRI6gpBLeMJvkL3VkmKd6AZymxFxRGjNEkJvYiA8aW5mb0Bib2IuY29tPsLA +jQQQAQgAIAUCX0aX4wYLCQcIAwIEFQgKAgQWAgEAAhkBAhsDAh4BACEJEAr9 +x5ZY6oZmFiEEm+B7p+lshgEOwGGZCv3HlljqhmaUWgf/efmGSpOKIGQ3Kh32 +HUqn/4ARvUmqMtZz4xUA9P3GAPY8XwJf00jSQlAo4//3aA1eEOJFHCr2qzCk +/4gIoZEScTTZp4itfL/Fer3UX+bV/VeTNgZGi+MRylSDQxLRQNpRgu+FmRAi +E6fr8D8GMvEcGb0jTRgWGj1EVtfOHfDg+EyPrtw+Z8u/bErUJ+Fnxz+KOGSN +SBQVAOflUYFoQhUNgZiq1s8WFD55sfes3UdBwsmHquDtYGo9dvWLJXxTEF8q +QCyKHYdk25ShIlNpRUqOH3CHqY/38z7QeV7INwtZaQvoES08RlD6ZMtczYLj +BZou86lozq7ISvRg1RSIWZ0ZRA== +=A9Ts +-----END PGP PRIVATE KEY BLOCK----- +` + +const armoredMessageRsaSignOnly = `-----BEGIN PGP MESSAGE----- + +wcBMAwr9x5ZY6oZmAQf+Lxghg4keIFpEq8a65gFkIfW+chHTDPlfI8xnx6U9 +HdsICX3Oye5V0ToCVKkEWDxfN1yCfXiYalSNo7ScRZKR7C+j02/pC+FfR6AJ +2cvdFoGIrLaXdjXXc/oXbsCCZA4C1DhQqpdORo2qGF0Q6Sm8659B0CfOgYSL +fBfKQ5VJngUT5JG8Uek3YuXBufPNhzdmXLHyB2Y2CwKkldi2vo4YNAukDhrR +2TojxdNoouhnMm+gloCE1n8huY1vw5F78/uiHen0tmHQ0dxtfk8cc1burgl/ +zUdJ3Sg6Eu+OC2ae5II63iB5fG+lCwZtfuepWnePDv8RDKNHCVP/LoBNpGOZ +U9I6AUkZWdcsueib9ghKDDy+HbUbf2kCJWUnuyeOCKqQifDb8bsLmdQY4Wb6 +EBeLgD8oZHVsH3NLjPakPw== +=STqy +-----END PGP MESSAGE-----` + +const compressed2KB = `-----BEGIN PGP MESSAGE----- + +yOsCeAEABAn79sQNAwAIAaNNfhjCDDG7AcvHRmIAAAAAAFRUVFRUVFRUVFRUVFRU +VFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRU +VFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRU +VFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRU +VFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRU +VFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRU +VFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRU +VFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRU +VFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRU +VFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRU +VFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRU +VFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRU +VFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRU +VFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRU +VFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRU +VFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRU +VFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRU +VFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRU +VFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRU +VFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRU +VFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRU +VFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRU +VFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRU +VFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRU +VFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRU +VFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRU +VFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRU +VFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRU +VFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRU +VFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRU +VFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRU +VFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRU +VFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRU +VFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRU +VFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRU +VFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRU +VFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRU +VFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRU +VFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRU +VFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRU +VFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRU +VFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRU +VFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVMBVVFRUVFRUVFRUVFRU +VFRUVFRUVFRUVFRUVFRUVFRUVFTCwCkEAAEIAF0Fgmgu3EMJEKNNfhjCDDG7NRQA +AAAAABwAEHNhbHRAbm90YXRpb25zLm9wZW5wZ3Bqcy5vcmdpoIeCV9r1q63ur3kQ +tmDhFiEEX7dLHQOx48sxvC+Ko01+GMIMMbsAAGXDA/4+XjyYlcW5AQlK4bP71f4J +dmmg+ijVY48OvALy35BE/W68t+vrWmCaSsXwg1NIPqaHQGNkL6I1qjSkQAxe8K90 +Z9JTerD46t0XuA6/v+W0j4uG5frNxfZ2D2AoNBW0yu6wgo5MM4IA+PH0xhtj0xa3 +e9Jwn5aNHhCQpFB3y/FDXAEAAP//nNYTdw== +=ETAD +-----END PGP MESSAGE-----` diff --git a/openpgp/v2/subkeys.go b/openpgp/v2/subkeys.go index 9dc70899e..f056ae7d4 100644 --- a/openpgp/v2/subkeys.go +++ b/openpgp/v2/subkeys.go @@ -187,7 +187,7 @@ func (s *Subkey) LatestValidBindingSignature(date time.Time, config *packet.Conf if sig.Valid == nil { err := s.Primary.PrimaryKey.VerifyKeySignature(s.PublicKey, sig.Packet) if err == nil { - err = checkSignatureDetails(s.PublicKey, sig.Packet, date, config) + err = checkSignatureDetails(sig.Packet, date, config) } valid := err == nil sig.Valid = &valid @@ -206,5 +206,5 @@ func (s *Subkey) LatestValidBindingSignature(date time.Time, config *packet.Conf if selectedSig == nil { return nil, errors.StructuralError("no valid binding signature found for subkey") } - return + return selectedSig, nil } diff --git a/openpgp/v2/user.go b/openpgp/v2/user.go index 3da03bd7c..8af5cac94 100644 --- a/openpgp/v2/user.go +++ b/openpgp/v2/user.go @@ -121,7 +121,7 @@ func (i *Identity) Revoked(selfCertification *packet.Signature, date time.Time, // Verify revocation signature (not verified yet). err := i.Primary.PrimaryKey.VerifyUserIdSignature(i.Name, i.Primary.PrimaryKey, revocation.Packet) if err == nil { - err = checkSignatureDetails(i.Primary.PrimaryKey, revocation.Packet, date, config) + err = checkSignatureDetails(revocation.Packet, date, config) } valid := err == nil revocation.Valid = &valid @@ -206,7 +206,7 @@ func (i *Identity) LatestValidSelfCertification(date time.Time, config *packet.C // Verify revocation signature (not verified yet). err = i.Primary.PrimaryKey.VerifyUserIdSignature(i.Name, i.Primary.PrimaryKey, sig.Packet) if err == nil { - err = checkSignatureDetails(i.Primary.PrimaryKey, sig.Packet, date, config) + err = checkSignatureDetails(sig.Packet, date, config) } valid := err == nil sig.Valid = &valid @@ -219,5 +219,5 @@ func (i *Identity) LatestValidSelfCertification(date time.Time, config *packet.C if selectedSig == nil { return nil, errors.StructuralError("no valid certification signature found for identity") } - return + return selectedSig, nil } diff --git a/openpgp/v2/write.go b/openpgp/v2/write.go index 5146607c7..0692cec6c 100644 --- a/openpgp/v2/write.go +++ b/openpgp/v2/write.go @@ -594,8 +594,8 @@ func encrypt( encryptKeys := make([]Key, len(to)+len(toHidden)) config := params.Config - // AEAD is used only if config enables it and every key supports it - aeadSupported := config.AEAD() != nil + // AEAD is used if every key supports it + aeadSupported := true var intendedRecipients []*packet.Recipient // Intended Recipient Fingerprint subpacket SHOULD be used when creating a signed and encrypted message @@ -611,10 +611,8 @@ func encrypt( timeForEncryptionKey = *params.EncryptionTime } for i, recipient := range append(to, toHidden...) { - var ok bool - encryptKeys[i], ok = recipient.EncryptionKey(timeForEncryptionKey, config) - if !ok { - return nil, errors.InvalidArgumentError("cannot encrypt a message to key id " + strconv.FormatUint(to[i].PrimaryKey.KeyId, 16) + " because it has no valid encryption keys") + if encryptKeys[i], err = recipient.EncryptionKeyWithError(timeForEncryptionKey, config); err != nil { + return nil, err } primarySelfSignature, _ := recipient.PrimarySelfSignature(timeForEncryptionKey, config) @@ -691,7 +689,7 @@ func encrypt( } for _, password := range params.Passwords { - if err = packet.SerializeSymmetricKeyEncryptedReuseKey(params.KeyWriter, params.SessionKey, password, params.Config); err != nil { + if err = packet.SerializeSymmetricKeyEncryptedAEADReuseKey(params.KeyWriter, params.SessionKey, password, aeadSupported, params.Config); err != nil { return nil, err } } @@ -882,7 +880,7 @@ func (s signatureWriter) Close() error { return s.encryptedData.Close() } -func adaptHashToSigningKey(config *packet.Config, primary *packet.PublicKey) crypto.Hash { +func selectHashForSigningKey(config *packet.Config, primary *packet.PublicKey) crypto.Hash { acceptableHashes := acceptableHashesToWrite(primary) hash, ok := algorithm.HashToHashId(config.Hash()) if !ok { @@ -895,17 +893,16 @@ func adaptHashToSigningKey(config *packet.Config, primary *packet.PublicKey) cry } if len(acceptableHashes) > 0 { defaultAcceptedHash, ok := algorithm.HashIdToHash(acceptableHashes[0]) - if !ok { - return config.Hash() + if ok { + return defaultAcceptedHash } - return defaultAcceptedHash } return config.Hash() } func createSignaturePacket(signer *packet.PublicKey, sigType packet.SignatureType, config *packet.Config) *packet.Signature { sigLifetimeSecs := config.SigLifetime() - hash := adaptHashToSigningKey(config, signer) + hash := selectHashForSigningKey(config, signer) return &packet.Signature{ Version: signer.Version, SigType: sigType, diff --git a/openpgp/write.go b/openpgp/write.go index b0f6ef7b0..84bc27d83 100644 --- a/openpgp/write.go +++ b/openpgp/write.go @@ -253,34 +253,12 @@ func writeAndSign(payload io.WriteCloser, candidateHashes []uint8, signed *Entit } var hash crypto.Hash - for _, hashId := range candidateHashes { - if h, ok := algorithm.HashIdToHash(hashId); ok && h.Available() { - hash = h - break - } - } - - // If the hash specified by config is a candidate, we'll use that. - if configuredHash := config.Hash(); configuredHash.Available() { - for _, hashId := range candidateHashes { - if h, ok := algorithm.HashIdToHash(hashId); ok && h == configuredHash { - hash = h - break - } - } - } - - if hash == 0 { - hashId := candidateHashes[0] - name, ok := algorithm.HashIdToString(hashId) - if !ok { - name = "#" + strconv.Itoa(int(hashId)) - } - return nil, errors.InvalidArgumentError("cannot encrypt because no candidate hash functions are compiled in. (Wanted " + name + " in this case.)") - } - var salt []byte if signer != nil { + if hash, err = selectHash(candidateHashes, config.Hash(), signer); err != nil { + return nil, err + } + var opsVersion = 3 if signer.Version == 6 { opsVersion = signer.Version @@ -558,13 +536,34 @@ func (s signatureWriter) Close() error { return s.encryptedData.Close() } +func selectHashForSigningKey(config *packet.Config, signer *packet.PublicKey) crypto.Hash { + acceptableHashes := acceptableHashesToWrite(signer) + hash, ok := algorithm.HashToHashId(config.Hash()) + if !ok { + return config.Hash() + } + for _, acceptableHashes := range acceptableHashes { + if acceptableHashes == hash { + return config.Hash() + } + } + if len(acceptableHashes) > 0 { + defaultAcceptedHash, ok := algorithm.HashIdToHash(acceptableHashes[0]) + if ok { + return defaultAcceptedHash + } + } + return config.Hash() +} + func createSignaturePacket(signer *packet.PublicKey, sigType packet.SignatureType, config *packet.Config) *packet.Signature { sigLifetimeSecs := config.SigLifetime() + hash := selectHashForSigningKey(config, signer) return &packet.Signature{ Version: signer.Version, SigType: sigType, PubKeyAlgo: signer.PubKeyAlgo, - Hash: config.Hash(), + Hash: hash, CreationTime: config.Now(), IssuerKeyId: &signer.KeyId, IssuerFingerprint: signer.Fingerprint, @@ -618,3 +617,74 @@ func handleCompression(compressed io.WriteCloser, candidateCompression []uint8, } return data, nil } + +// selectHash selects the preferred hash given the candidateHashes and the configuredHash +func selectHash(candidateHashes []byte, configuredHash crypto.Hash, signer *packet.PrivateKey) (hash crypto.Hash, err error) { + acceptableHashes := acceptableHashesToWrite(&signer.PublicKey) + candidateHashes = intersectPreferences(acceptableHashes, candidateHashes) + + for _, hashId := range candidateHashes { + if h, ok := algorithm.HashIdToHash(hashId); ok && h.Available() { + hash = h + break + } + } + + // If the hash specified by config is a candidate, we'll use that. + if configuredHash.Available() { + for _, hashId := range candidateHashes { + if h, ok := algorithm.HashIdToHash(hashId); ok && h == configuredHash { + hash = h + break + } + } + } + + if hash == 0 { + if len(acceptableHashes) > 0 { + if h, ok := algorithm.HashIdToHash(acceptableHashes[0]); ok { + hash = h + } else { + return 0, errors.UnsupportedError("no candidate hash functions are compiled in.") + } + } else { + return 0, errors.UnsupportedError("no candidate hash functions are compiled in.") + } + } + return +} + +func acceptableHashesToWrite(singingKey *packet.PublicKey) []uint8 { + switch singingKey.PubKeyAlgo { + case packet.PubKeyAlgoEd448: + return []uint8{ + hashToHashId(crypto.SHA512), + hashToHashId(crypto.SHA3_512), + } + case packet.PubKeyAlgoECDSA, packet.PubKeyAlgoEdDSA: + if curve, err := singingKey.Curve(); err == nil { + if curve == packet.Curve448 || + curve == packet.CurveNistP521 || + curve == packet.CurveBrainpoolP512 { + return []uint8{ + hashToHashId(crypto.SHA512), + hashToHashId(crypto.SHA3_512), + } + } else if curve == packet.CurveBrainpoolP384 || + curve == packet.CurveNistP384 { + return []uint8{ + hashToHashId(crypto.SHA384), + hashToHashId(crypto.SHA512), + hashToHashId(crypto.SHA3_512), + } + } + } + } + return []uint8{ + hashToHashId(crypto.SHA256), + hashToHashId(crypto.SHA384), + hashToHashId(crypto.SHA512), + hashToHashId(crypto.SHA3_256), + hashToHashId(crypto.SHA3_512), + } +}