Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: update TLS tests for OpenSSL 3.2 #53384

Merged
merged 1 commit into from
Jun 9, 2024

Conversation

richardlau
Copy link
Member

@richardlau richardlau commented Jun 7, 2024

Update the following TLS tests to account for error code changes in OpenSSL 3.2 and later.

  • parallel/test-tls-empty-sni-context
  • parallel/test-tls-psk-circuit

Refs: #53382
Refs: openssl/openssl#19950

cc @nodejs/crypto


Addresses these failures with OpenSSL 3.2:

not ok 3051 parallel/test-tls-empty-sni-context
  ---
  duration_ms: 107.38800
  severity: fail
  exitcode: 1
  stack: |-
    node:assert:126
      throw new AssertionError(obj);
      ^
    
    AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:
    + actual - expected
    
    + 'ERR_SSL_SSL/TLS_ALERT_HANDSHAKE_FAILURE'
    - 'ERR_SSL_SSLV3_ALERT_HANDSHAKE_FAILURE'
        at TLSSocket.<anonymous> (/home/nodejs/node/test/parallel/test-tls-empty-sni-context.js:29:12)
        at TLSSocket.<anonymous> (/home/nodejs/node/test/common/index.js:466:15)
        at TLSSocket.emit (node:events:520:28)
        at emitErrorNT (node:internal/streams/destroy:170:8)
        at emitErrorCloseNT (node:internal/streams/destroy:129:3)
        at process.processTicksAndRejections (node:internal/process/task_queues:82:21) {
      generatedMessage: true,
      code: 'ERR_ASSERTION',
      actual: 'ERR_SSL_SSL/TLS_ALERT_HANDSHAKE_FAILURE',
      expected: 'ERR_SSL_SSLV3_ALERT_HANDSHAKE_FAILURE',
      operator: 'strictEqual'
    }
    
    Node.js v23.0.0-pre
  ...
not ok 3135 parallel/test-tls-psk-circuit
  ---
  duration_ms: 130.16900
  severity: fail
  exitcode: 1
  stack: |-
    node:assert:126
      throw new AssertionError(obj);
      ^
    
    AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:
    + actual - expected
    
    + 'ERR_SSL_SSL/TLS_ALERT_HANDSHAKE_FAILURE'
    - 'ERR_SSL_SSLV3_ALERT_HANDSHAKE_FAILURE'
        at TLSSocket.<anonymous> (/home/nodejs/node/test/parallel/test-tls-psk-circuit.js:52:16)
        at TLSSocket.<anonymous> (/home/nodejs/node/test/common/index.js:466:15)
        at TLSSocket.emit (node:events:520:28)
        at emitErrorNT (node:internal/streams/destroy:170:8)
        at emitErrorCloseNT (node:internal/streams/destroy:129:3)
        at process.processTicksAndRejections (node:internal/process/task_queues:82:21) {
      generatedMessage: true,
      code: 'ERR_ASSERTION',
      actual: 'ERR_SSL_SSL/TLS_ALERT_HANDSHAKE_FAILURE',
      expected: 'ERR_SSL_SSLV3_ALERT_HANDSHAKE_FAILURE',
      operator: 'strictEqual'
    }
    
    Node.js v23.0.0-pre
  ...

Update the following TLS tests to account for error code changes
in OpenSSL 3.2 and later.
- `parallel/test-tls-empty-sni-context`
- `parallel/test-tls-psk-circuit`
@richardlau richardlau added test Issues and PRs related to the tests. openssl Issues and PRs related to the OpenSSL dependency. labels Jun 7, 2024
@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label Jun 7, 2024
@nodejs-github-bot

This comment was marked as outdated.

@richardlau richardlau added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jun 7, 2024
@nodejs-github-bot
Copy link
Collaborator

@richardlau richardlau added the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 9, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 9, 2024
@nodejs-github-bot nodejs-github-bot merged commit ce531af into nodejs:main Jun 9, 2024
64 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in ce531af

@richardlau richardlau deleted the openssl3.2tests branch June 9, 2024 17:58
@sebastianas
Copy link

The failure in test/parallel/test-http2-https-fallback.js is due to openssl/openssl#24338
It looks like node was testing an error behavior with error code but now an error is returned.

@sebastianas
Copy link

One question: For the tests, OpenSSL 3.0,3.1,3.2,3.3 have the same ABI. Would it be possible to compare the version against the version as returned by OpenSSL_version_num() (current library version) instead the version of openssl it was compiled against (constants.OPENSSL_VERSION_NUMBER)?

@richardlau
Copy link
Member Author

I think it should be possible, but we'll run into the same issue as #51007 (comment) where the values are being captured in the snapshot and somehow need to be reset (I'm not entirely sure how).

targos pushed a commit that referenced this pull request Jun 20, 2024
Update the following TLS tests to account for error code changes
in OpenSSL 3.2 and later.
- `parallel/test-tls-empty-sni-context`
- `parallel/test-tls-psk-circuit`

PR-URL: #53384
Refs: #53382
Refs: openssl/openssl#19950
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
EliphazBouye pushed a commit to EliphazBouye/node that referenced this pull request Jun 20, 2024
Update the following TLS tests to account for error code changes
in OpenSSL 3.2 and later.
- `parallel/test-tls-empty-sni-context`
- `parallel/test-tls-psk-circuit`

PR-URL: nodejs#53384
Refs: nodejs#53382
Refs: openssl/openssl#19950
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
bmeck pushed a commit to bmeck/node that referenced this pull request Jun 22, 2024
Update the following TLS tests to account for error code changes
in OpenSSL 3.2 and later.
- `parallel/test-tls-empty-sni-context`
- `parallel/test-tls-psk-circuit`

PR-URL: nodejs#53384
Refs: nodejs#53382
Refs: openssl/openssl#19950
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
marco-ippolito pushed a commit that referenced this pull request Jul 19, 2024
Update the following TLS tests to account for error code changes
in OpenSSL 3.2 and later.
- `parallel/test-tls-empty-sni-context`
- `parallel/test-tls-psk-circuit`

PR-URL: #53384
Refs: #53382
Refs: openssl/openssl#19950
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
marco-ippolito pushed a commit that referenced this pull request Jul 19, 2024
Update the following TLS tests to account for error code changes
in OpenSSL 3.2 and later.
- `parallel/test-tls-empty-sni-context`
- `parallel/test-tls-psk-circuit`

PR-URL: #53384
Refs: #53382
Refs: openssl/openssl#19950
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
mhdawson added a commit to mhdawson/io.js that referenced this pull request Aug 28, 2024
Refs: nodejs#53382
Refs: nodejs#53384

Same change as in 53384 where OpenSSL32 returns a slightly
different error but for a different test.

Signed-off-by: Michael Dawson <midawson@redhat.com>
mhdawson added a commit to mhdawson/io.js that referenced this pull request Aug 29, 2024
Refs: nodejs#53382
Refs: nodejs#53384

Same change as in 53384 where OpenSSL32 returns a slightly
different error but for a different test.

Signed-off-by: Michael Dawson <midawson@redhat.com>
mhdawson added a commit that referenced this pull request Aug 30, 2024
Refs: #53382
Refs: #53384

Same change as in 53384 where OpenSSL32 returns a slightly
different error but for a different test.

Signed-off-by: Michael Dawson <midawson@redhat.com>
PR-URL: #54610
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
dnalborczyk pushed a commit to dnalborczyk/node that referenced this pull request Aug 30, 2024
Update `parallel/test-tls-set-sigalgs` to account for error code changes
in OpenSSL 3.2 and later.

PR-URL: nodejs#54612
Refs: nodejs#53384
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
RafaelGSS pushed a commit that referenced this pull request Aug 30, 2024
Update `parallel/test-tls-set-sigalgs` to account for error code changes
in OpenSSL 3.2 and later.

PR-URL: #54612
Refs: #53384
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
RafaelGSS pushed a commit that referenced this pull request Aug 30, 2024
Refs: #53382
Refs: #53384

Same change as in 53384 where OpenSSL32 returns a slightly
different error but for a different test.

Signed-off-by: Michael Dawson <midawson@redhat.com>
PR-URL: #54610
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
RafaelGSS pushed a commit that referenced this pull request Aug 30, 2024
Update `parallel/test-tls-set-sigalgs` to account for error code changes
in OpenSSL 3.2 and later.

PR-URL: #54612
Refs: #53384
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
RafaelGSS pushed a commit that referenced this pull request Aug 30, 2024
Refs: #53382
Refs: #53384

Same change as in 53384 where OpenSSL32 returns a slightly
different error but for a different test.

Signed-off-by: Michael Dawson <midawson@redhat.com>
PR-URL: #54610
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
RafaelGSS pushed a commit that referenced this pull request Aug 30, 2024
Update `parallel/test-tls-set-sigalgs` to account for error code changes
in OpenSSL 3.2 and later.

PR-URL: #54612
Refs: #53384
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
RafaelGSS pushed a commit that referenced this pull request Aug 30, 2024
Refs: #53382
Refs: #53384

Same change as in 53384 where OpenSSL32 returns a slightly
different error but for a different test.

Signed-off-by: Michael Dawson <midawson@redhat.com>
PR-URL: #54610
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
sendoru pushed a commit to sendoru/node that referenced this pull request Sep 1, 2024
Update `parallel/test-tls-set-sigalgs` to account for error code changes
in OpenSSL 3.2 and later.

PR-URL: nodejs#54612
Refs: nodejs#53384
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
sendoru pushed a commit to sendoru/node that referenced this pull request Sep 1, 2024
Refs: nodejs#53382
Refs: nodejs#53384

Same change as in 53384 where OpenSSL32 returns a slightly
different error but for a different test.

Signed-off-by: Michael Dawson <midawson@redhat.com>
PR-URL: nodejs#54610
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
RafaelGSS pushed a commit that referenced this pull request Sep 1, 2024
Update `parallel/test-tls-set-sigalgs` to account for error code changes
in OpenSSL 3.2 and later.

PR-URL: #54612
Refs: #53384
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
RafaelGSS pushed a commit that referenced this pull request Sep 1, 2024
Refs: #53382
Refs: #53384

Same change as in 53384 where OpenSSL32 returns a slightly
different error but for a different test.

Signed-off-by: Michael Dawson <midawson@redhat.com>
PR-URL: #54610
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. needs-ci PRs that need a full CI run. openssl Issues and PRs related to the OpenSSL dependency. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants