TLSv1.3: add optional integrity-only ciphers [RFC 9150] #22903

rajeev-0 · 2023-12-01T15:57:31Z

This implements the integrity-only ciphers for TLS v1.3 defined in RFC 9150 as requested in #18343.

Tests for the new cipher suites are implicitly added by evp_libctx_test.c.

Fixes #18343

Checklist

documentation is added or updated
tests are added or updated

slontis · 2023-12-04T20:46:40Z

Just wondering, was this written from scratch, or is it a copy of someones else's work?

rajeev-0 · 2023-12-04T21:22:37Z

As mentioned in #18343, Some work was done for openssl 1.1.1.
But it was not as described in RFC 9150 and was too much effort to adopt it. So I have to start from scratch. But if you know some work which is already done for openssl 3.0 and is as per RFC 9150, I will be happy to check it.

slontis · 2023-12-04T22:16:52Z

No I dont.. Just checking that we didnt need to get permission from another author, if it was derived from somewhere..

rajeev-0 · 2023-12-04T23:05:26Z

Need help #22902

ssl/tls13_enc.c

test/evp_libctx_test.c

DDvO

One bug, a few memory leaks on error, several questions,
and various suggestions for small style, name, and text improvements

CHANGES.md

doc/man1/openssl-ciphers.pod.in

include/openssl/evp.h

Configure

test/quicapitest.c

test/sslapitest.c

rajeev-0 · 2024-01-16T09:37:49Z

@DDvO- Thankyou for the review. I did the required changes.

DDvO · 2024-01-16T17:43:12Z

@DDvO- Thank you for the review. I did the required changes.

Fine. Just a few follow-up points - see above the the comments that I did not mark as resolved.

DDvO · 2024-01-16T17:45:45Z

A general question, maybe to the OpenSSL team:

Do the tests that are referenced here include the test case that a TLS record is rejected in case its integrity gets violated in transit?

DDvO

Tests appear incomplete.

providers/implementations/ciphers/cipher_enull_hmac_hw.c

rajeev-0 · 2024-01-19T15:26:12Z

@DDvO Thankyou for the review and pointing out incomplete test case.
I hope this time I covered all the test cases.

ssl/record/methods/tls13_meth.c

DDvO

LGTM

DDvO · 2024-05-10T09:58:39Z

@mattcaswell could you please re-approve.

t8m

Why the new empty test/recipes/30-test_evp_data/evpciph_null_hmac.txt

CHANGES.md

Configure

doc/man1/openssl-ciphers.pod.in

doc/man3/SSL_CTX_set_cipher_list.pod

providers/implementations/ciphers/cipher_null.c

ssl/record/methods/tls13_meth.c

rajeev-0 · 2024-05-13T10:22:40Z

Why the new empty test/recipes/30-test_evp_data/evpciph_null_hmac.txt

I removed the file. It was leftover from previous implementation.

.github/workflows/run-checker-merge.yml

DDvO

Reconfirmed

openssl-machine · 2024-05-14T12:00:31Z

This pull request is ready to merge

t8m · 2024-05-14T13:47:03Z

Merged to the master branch with slightly reworded commit message. Thank you for your contribution.

Good work @rajeev-0!

Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: David von Oheimb <david.von.oheimb@siemens.com> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from #22903)

- add test vectors for tls1_3 integrity-only ciphers - recmethod_local.h: add new member for MAC - tls13_meth.c: add MAC only to tls 1.3 - tls13_enc.c: extend function to add MAC only - ssl_local.h: add ssl_cipher_get_evp_md_mac() - s3_lib.c: add the new ciphers and add #ifndef OPENSSL_NO_INTEGRITY_ONLY_CIPHERS - ssl_ciph.c : add ssl_cipher_get_evp_md_mac() and use it - tls13secretstest.c: add dummy test function - Configure: add integrity-only-ciphers option - document the new ciphers Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: David von Oheimb <david.von.oheimb@siemens.com> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from #22903)

github-actions bot added the severity: fips change The pull request changes FIPS provider sources label Dec 1, 2023

t8m added branch: master Merge to master branch triaged: feature The issue/pr requests/adds a feature labels Dec 1, 2023

slontis reviewed Dec 5, 2023

View reviewed changes

ssl/tls13_enc.c Outdated Show resolved Hide resolved

github-actions bot added the severity: ABI change This pull request contains ABI changes label Dec 6, 2023

slontis reviewed Dec 6, 2023

View reviewed changes

test/evp_libctx_test.c Outdated Show resolved Hide resolved

rajeev-0 force-pushed the add_IntegrityOnlyCipher_TLS13 branch 2 times, most recently from 58e3fff to 3c87345 Compare December 11, 2023 10:05

rajeev-0 marked this pull request as ready for review December 11, 2023 10:34

rajeev-0 requested a review from slontis December 12, 2023 10:03

rajeev-0 marked this pull request as draft December 12, 2023 16:17

rajeev-0 force-pushed the add_IntegrityOnlyCipher_TLS13 branch from baebdaa to aa201ba Compare December 13, 2023 22:47

rajeev-0 marked this pull request as ready for review December 13, 2023 23:32

rajeev-0 requested review from t8m and mattcaswell December 13, 2023 23:33

DDvO requested changes Jan 8, 2024

View reviewed changes

rajeev-0 requested a review from DDvO January 16, 2024 09:18

rajeev-0 force-pushed the add_IntegrityOnlyCipher_TLS13 branch from 9af3d51 to 7e33e58 Compare January 16, 2024 22:47

DDvO requested changes Jan 17, 2024

View reviewed changes

providers/implementations/ciphers/cipher_enull_hmac_hw.c Outdated Show resolved Hide resolved

DDvO changed the title ~~Add integrity only cipher tls13~~ TLSv1.3: add optional integrity-only ciphers [RFC 9150] Jan 17, 2024

rajeev-0 force-pushed the add_IntegrityOnlyCipher_TLS13 branch from b16f2e8 to 6825013 Compare January 19, 2024 14:49

rajeev-0 requested a review from DDvO January 19, 2024 15:26

fixup! Add support for integrity-only cipher suites for TLS v1.3

5189582

DDvO reviewed May 10, 2024

View reviewed changes

ssl/record/methods/tls13_meth.c Show resolved Hide resolved

DDvO approved these changes May 10, 2024

View reviewed changes

DDvO added approval: otc review pending This pull request needs review by an OTC member and removed approval: review pending This pull request needs review by a committer labels May 10, 2024

t8m requested a review from mattcaswell May 10, 2024 13:02

t8m requested changes May 10, 2024

View reviewed changes

rajeev-0 force-pushed the add_IntegrityOnlyCipher_TLS13 branch 2 times, most recently from 9f020b0 to a5241b0 Compare May 12, 2024 23:20

rajeev-0 requested a review from t8m May 13, 2024 08:31

fixup! Add support for integrity-only cipher suites for TLS v1.3

efa76a8

rajeev-0 force-pushed the add_IntegrityOnlyCipher_TLS13 branch from a5241b0 to efa76a8 Compare May 13, 2024 10:21

mattcaswell requested changes May 13, 2024

View reviewed changes

.github/workflows/run-checker-merge.yml Show resolved Hide resolved

fixup! Add support for integrity-only cipher suites for TLS v1.3

3bc4581

rajeev-0 requested a review from mattcaswell May 13, 2024 11:18

mattcaswell approved these changes May 13, 2024

View reviewed changes

t8m approved these changes May 13, 2024

View reviewed changes

t8m requested a review from DDvO May 13, 2024 11:27

t8m added approval: done This pull request has the required number of approvals and removed approval: otc review pending This pull request needs review by an OTC member labels May 13, 2024

DDvO approved these changes May 13, 2024

View reviewed changes

openssl-machine added approval: ready to merge The 24 hour grace period has passed, ready to merge and removed approval: done This pull request has the required number of approvals labels May 14, 2024

t8m closed this May 14, 2024

openssl-machine pushed a commit that referenced this pull request May 14, 2024

cipher_null.c: add NULL check

61f3239

Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: David von Oheimb <david.von.oheimb@siemens.com> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from #22903)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TLSv1.3: add optional integrity-only ciphers [RFC 9150] #22903

TLSv1.3: add optional integrity-only ciphers [RFC 9150] #22903

rajeev-0 commented Dec 1, 2023 •

edited by DDvO

slontis commented Dec 4, 2023

rajeev-0 commented Dec 4, 2023 •

edited by DDvO

slontis commented Dec 4, 2023

rajeev-0 commented Dec 4, 2023

DDvO left a comment

rajeev-0 commented Jan 16, 2024

DDvO commented Jan 16, 2024

DDvO commented Jan 16, 2024

DDvO left a comment

rajeev-0 commented Jan 19, 2024

DDvO left a comment

DDvO commented May 10, 2024

t8m left a comment

rajeev-0 commented May 13, 2024

DDvO left a comment

openssl-machine commented May 14, 2024

t8m commented May 14, 2024

TLSv1.3: add optional integrity-only ciphers [RFC 9150] #22903

TLSv1.3: add optional integrity-only ciphers [RFC 9150] #22903

Conversation

rajeev-0 commented Dec 1, 2023 • edited by DDvO

Checklist

slontis commented Dec 4, 2023

rajeev-0 commented Dec 4, 2023 • edited by DDvO

slontis commented Dec 4, 2023

rajeev-0 commented Dec 4, 2023

DDvO left a comment

Choose a reason for hiding this comment

rajeev-0 commented Jan 16, 2024

DDvO commented Jan 16, 2024

DDvO commented Jan 16, 2024

DDvO left a comment

Choose a reason for hiding this comment

rajeev-0 commented Jan 19, 2024

DDvO left a comment

Choose a reason for hiding this comment

DDvO commented May 10, 2024

t8m left a comment

Choose a reason for hiding this comment

rajeev-0 commented May 13, 2024

DDvO left a comment

Choose a reason for hiding this comment

openssl-machine commented May 14, 2024

t8m commented May 14, 2024

rajeev-0 commented Dec 1, 2023 •

edited by DDvO

rajeev-0 commented Dec 4, 2023 •

edited by DDvO