Fix ssl_context_info.c to correctly check for EOF - type-limit bug #3449

naynajain · 2020-06-23T14:01:16Z

Notes:

Description

Reference: Pull Request 3431
Separated bug fixes from PKCS7 feature PR

Status

READY

Requires Backporting

It is a bug fix, but the offending file is not present in any of the LTS branches, so no backporting required [edited by mpg].

Migrations

If there is any API change, what's the incentive and logic for it.
NO

Additional Comments:
Changelog is updated.

gilles-peskine-arm · 2020-06-23T14:34:24Z

ChangeLog.d/bugfix_1.txt

@@ -0,0 +1,3 @@
+Bugfix:


This needs to be Bugfix without :

gilles-peskine-arm · 2020-06-23T14:36:26Z

ChangeLog.d/bugfix_1.txt

@@ -0,0 +1,3 @@
+Bugfix:
+    * x509: do not include time.h without MBEDTLS_HAVE_TIME.
+    * Fixes type-limit error programs/ssl/ssl_context_info.c.


Please use complete, grammatical sentences in the change log. Also, please write with the library user in mind. Why would the user care about a “type-limit error”? Does the compilation fail on some platforms? Is there a runtime bug?

gilles-peskine-arm · 2020-06-23T14:37:50Z

ChangeLog.d/bugfix_1.txt

@@ -0,0 +1,3 @@
+Bugfix:
+    * x509: do not include time.h without MBEDTLS_HAVE_TIME.


We usually acknowledge contributors in changelog entries. Unless you don't want to be credited, feel free to add

Reported and fix contributed by naynajain in #3449.

You can use your name instead of your GitHub handle if you prefer.

gilles-peskine-arm · 2020-06-23T14:39:03Z

programs/ssl/ssl_context_info.c

@@ -379,7 +379,7 @@ size_t read_next_b64_code( uint8_t **b64, size_t *max_len )
    int valid_balance = 0;  /* balance between valid and invalid characters */
    size_t len = 0;
    char pad = 0;
-    char c = 0;
+    signed char c = 0;


This change doesn't look right. c contains either a character read from a file or EOF, so the correct type is int.

This bug fix should have a non-regression test, with an input file containing the byte '\xff'.

This change doesn't look right. c contains either a character read from a file or EOF, so the correct type is int.

This bug fix should have a non-regression test, with an input file containing the byte '\xff'.

Thanks for the review.

When you say it should have "non-regression test, with an input file... ", where are you expecting this test ? The bug fix is for one of the program file. And it seems that takes command line args for testing different cases. I have not explored ssl code in mbedtls yet, just started looking at this file because of failure with cmake to build it, so I might be missing something.

There are tests for this program in tests/context-info.sh

gilles-peskine-arm · 2020-06-23T14:48:20Z

Apparently there are also build errors in TLS code. This pull request is complementary with #3444. It would be great if you could synchronize with @raoulstrackx to fix the problem everywhere.

naynajain · 2020-08-05T16:52:07Z

Apparently there are also build errors in TLS code. This pull request is complementary with #3444. It would be great if you could synchronize with @raoulstrackx to fix the problem everywhere.

I have messaged to @raoulstrackx to see how he wants to go forward. In the meantime, I was wondering if I should resubmit just another bug in this commit "type-limit" error while we are coordinating "MBEDTLS_HAVE_TIME" issue with @raoulstrackx.

Thanks & Regards,
- Nayna

gilles-peskine-arm · 2020-08-05T21:35:44Z

Sorry, I don't understand what you mean by “type-limit error”.

naynajain · 2020-08-06T11:42:06Z

Sorry, I don't understand what you mean by “type-limit error”.

I meant this commit in PR - "mbedtls: Fixes type-limit error".

Thanks & Regards,
- Nayna

gilles-peskine-arm · 2020-08-06T11:50:03Z

Oh, right. Please make a separate PR for that, or reuse this one to do only that.

As I said before, please fix this correctly instead of ignoring the error, and add a non-regression test.

naynajain · 2020-08-06T12:16:45Z

Oh, right. Please make a separate PR for that, or reuse this one to do only that.

As I said before, please fix this correctly instead of ignoring the error, and add a non-regression test.

Sure.

Thanks & Regards,
- Nayna

naynajain · 2020-08-14T14:39:47Z

Daniel Axtens from my team is working on MBEDTLS_HAVE_TIME bugfix. He will post his changes as separate PR. I would be using this PR for the fix related to "type-limit" issue. I am renaming this PR to "Fix the EOF check error in ssl_context_info.c " that is identified by gcc warning for type-limit.

read_next_b64_code() function, that parses base64 encoded input doesn't recognize the EOF and returns when "Too many bad symbols are detected". This issue got identified when gcc complained for type-limit error during cmake. This patch fixes the issue by changing the variable type to int and removing type-cast of fgetc() output to 'char'. Signed-off-by: Nayna Jain <[email protected]>

This patch updates the test cases to also check for EOF correctly. Signed-off-by: Nayna Jain <[email protected]>

naynajain · 2020-08-14T15:20:37Z

Before this patch, running ssl_context_info with valid file input was exiting not because of EOF but because of error "Too many bad symbols detected"
$ ./programs/ssl/ssl_context_info -f ./tests/data_files/base64/cli_cid.txt

Deserializing number 1:

Base64 code:
AhUAAH8AAA8AAAQ8AAAAAF6MZUPMqAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAB
h7h8/aprLN1fS0KwLkZzKcsa5LNtDW7sYu7d1z7fNetuRjLJpX/A1mTSqeBY7li8AAAAAAAM7MII
DNzCCAh+gAwIBAgIBAjANBgkqhkiG9w0BAQsFADA7MQswCQYDVQQGEwJOTDERMA8GA1UECgwIUG9
sYXJTU0wxGTAXBgNVBAMMEFBvbGFyU1NMIFRlc3QgQ0EwHhcNMTkwMjEwMTQ0NDA2WhcNMjkwMjE
wMTQ0NDA2WjA0MQswCQYDVQQGEwJOTDERMA8GA1UECgwIUG9sYXJTU0wxEjAQBgNVBAMMCWxvY2F
saG9zdDCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAMFNo93nzR3RBNdJcriZrA545Do
8Ss86ExbQWuTNowCIp+4ea5anUrSQ7y1yej4kmvy2NKwk9XfgJmSMnLAofaHa6ozmyRyWvP7BBFK
zNtSj+uGxdtiQwWG0ZlI2oiZTqqt0Xgd9GYLbKtgfoNkNHC1JZvdbJXNG6AuKT2kMtQCQ4dqCEGZ
9rlQri2V5kaHiYcPNQEkI7mgM8YuG0ka/0LiqEQMef1aoGh5EGA8PhYvai0Re4hjGYi/HZo36Xdh
98yeJKQHFkA4/J/EwyEoO79bex8cna8cFPXrEAjyaHT4P6DSYW8tzS1KW2BGiLICIaTla0w+w3lk
vEcf36hIBMJcCAwEAAaNNMEswCQYDVR0TBAIwADAdBgNVHQ4EFgQUpQXoZLjc32APUBJNYKhkr02
LQ5MwHwYDVR0jBBgwFoAUtFrkpbPe0lL2udWmlQ/rPrzH/f8wDQYJKoZIhvcNAQELBQADggEBAC4
65FJhPqel7zJngHIHJrqj/wVAxGAFOTF396XKATGAp+HRCqJ81Ry60CNK1jDzk8dv6M6UHoS7RIF
iM/9rXQCbJfiPD5xMTejZp5n5UYHAmxsxDaazfA5FuBhkfokKK6jD4Eq91C94xGKb6X4/VkaPF7c
qoBBw/bHxawXc0UEPjqayiBpCYU/rJoVZgLqFVP7Px3sva1nOrNx8rPPI1hJ+ZOg8maiPTxHZnBV
LakSSLQy/sWeWyazO1RnrbxjrbgQtYKz0e3nwGpu1w13vfckFmUSBhHXH7AAS/HpKC4IH7G2GAk3
+n8iSSN71sZzpxonQwVbopMZqLmbBm/7WPLcAAJRZtK1pHRuu/Uw+Y91KCaqMAHKWeVJvuqjiTaE
lrahsx+HYoZ1+8i5BMY1NOL/y4TR9qZdxY+7NvNrEdEoFgcI/DqUN0aKs0zAIPmk92pFnjnbro5L
xWRm3JbtIFcG6PdN+9aAbISrewt6EERIPhS45aH+Si08NLrvM+CcEBfqBBqOD+4LCZqT8nDBtALJ
yRqiykibsAAFRgAAAAF6MZUNak74BhbcgvZ2M8WhZKjQyCix7GJzRs4SqnD7iXoxlQ7YXjsVI0K/
xyMOJPkT9ZcPEi/2jHGIte1ZduW4Cvu8C3q0AAAAAAAAAAAAAAAIAAAAAAAAABwAAAQAAAAAAAwA
AAA==

Mbed TLS version:
major 2
minor 21
path 0

Enabled session and context configuration:
MBEDTLS_HAVE_TIME
MBEDTLS_X509_CRT_PARSE_C
MBEDTLS_SSL_MAX_FRAGMENT_LENGTH
MBEDTLS_SSL_TRUNCATED_HMAC
MBEDTLS_SSL_ENCRYPT_THEN_MAC
MBEDTLS_SSL_SESSION_TICKETS
MBEDTLS_SSL_SESSION_TICKETS and client
MBEDTLS_SSL_DTLS_CONNECTION_ID
MBEDTLS_SSL_DTLS_BADMAC_LIMIT
MBEDTLS_SSL_DTLS_ANTI_REPLAY
MBEDTLS_SSL_ALPN

Session info:
start time : 2020-04-07 11:34:27
ciphersuite : TLS-ECDHE-RSA-WITH-CHACHA20-POLY1305-SHA256
cipher flags : 0x00
cipher : CHACHA20-POLY1305
Message-Digest : SHA256
compression : disabled
session ID : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
master secret : 61 EE 1F 3F 6A 9A CB 37 57 D2 D0 AC 0B 91 9C CA
72 C6 B9 2C DB 43 5B BB 18 BB B7 75 CF B7 CD 7A
DB 91 8C B2 69 5F F0 35 99 34 AA 78 16 3B 96 2F
verify result : 0x00000000

Certificate:
cert. version : 3
serial number : 02
issuer name : C=NL, O=PolarSSL, CN=PolarSSL Test CA
subject name : C=NL, O=PolarSSL, CN=localhost
issued on : 2019-02-10 14:44:06
expires on : 2029-02-10 14:44:06
signed using : RSA with SHA-256
RSA key size : 2048 bits
basic constraints : CA=false

Ticket:
59 B4 AD 69 1D 1B AE FD 4C 3E 63 DD 4A 09 AA 8C 00 72 96 79 52 6F
BA A8 E2 4D A1 25 AD A8 6C C7 E1 D8 A1 9D 7E F2 2E 41 31 8D 4D 38
BF F2 E1 34 7D A9 97 71 63 EE CD BC DA C4 74 4A 05 81 C2 3F 0E A5
0D D1 A2 AC D3 30 08 3E 69 3D DA 91 67 8E 76 EB A3 92 F1 59 19 B7
25 BB 48 15 C1 BA 3D D3 7E F5 A0 1B 21 2A DE C2 DE 84 11 12 0F 85
2E 39 68 7F 92 8B 4F 0D 2E BB CC F8 27 04 05 FA 81 06 A3 83 FB 82
C2 66 A4 FC 9C 30 6D 00 B2 72 46 A8 B2 92 26 EC

lifetime : 86400 sec.

Session others:
MFL : none
negotiate truncated HMAC : disabled
Encrypt-then-MAC : disabled

Random bytes:
5E 8C 65 43 5A 93 BE 01 85 B7 20 BD 9D 8C F1 68 59 2A 34 32 0A 2C
7B 18 9C D1 B3 84 AA 9C 3E E2 5E 8C 65 43 B6 17 8E C5 48 D0 AF F1
C8 C3 89 3E 44 FD 65 C3 C4 8B FD A3 1C 62 2D 7B 56 5D B9 6E

Context others:
in CID : BE EF
out CID : DE AD
bad MAC seen number : 0
last validated record sequence no. : 00 00 00 00 00 00 00 02
bitmask for replay detection : 00 00 00 00 00 00 00 07
DTLS datagram packing : enabled
outgoing record sequence no. : 00 01 00 00 00 00 00 03
MTU : 0
ALPN negotiation : not selected

ERROR: Too many bad symbols detected. File check aborted.

After the fix, it is exiting correctly recognizing EOF as below. No error is reported.

$ ./programs/ssl/ssl_context_info -f ./tests/data_files/base64/cli_cid.txt

Deserializing number 1:

Base64 code:
AhUAAH8AAA8AAAQ8AAAAAF6MZUPMqAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAB
h7h8/aprLN1fS0KwLkZzKcsa5LNtDW7sYu7d1z7fNetuRjLJpX/A1mTSqeBY7li8AAAAAAAM7MII
DNzCCAh+gAwIBAgIBAjANBgkqhkiG9w0BAQsFADA7MQswCQYDVQQGEwJOTDERMA8GA1UECgwIUG9
sYXJTU0wxGTAXBgNVBAMMEFBvbGFyU1NMIFRlc3QgQ0EwHhcNMTkwMjEwMTQ0NDA2WhcNMjkwMjE
wMTQ0NDA2WjA0MQswCQYDVQQGEwJOTDERMA8GA1UECgwIUG9sYXJTU0wxEjAQBgNVBAMMCWxvY2F
saG9zdDCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAMFNo93nzR3RBNdJcriZrA545Do
8Ss86ExbQWuTNowCIp+4ea5anUrSQ7y1yej4kmvy2NKwk9XfgJmSMnLAofaHa6ozmyRyWvP7BBFK
zNtSj+uGxdtiQwWG0ZlI2oiZTqqt0Xgd9GYLbKtgfoNkNHC1JZvdbJXNG6AuKT2kMtQCQ4dqCEGZ
9rlQri2V5kaHiYcPNQEkI7mgM8YuG0ka/0LiqEQMef1aoGh5EGA8PhYvai0Re4hjGYi/HZo36Xdh
98yeJKQHFkA4/J/EwyEoO79bex8cna8cFPXrEAjyaHT4P6DSYW8tzS1KW2BGiLICIaTla0w+w3lk
vEcf36hIBMJcCAwEAAaNNMEswCQYDVR0TBAIwADAdBgNVHQ4EFgQUpQXoZLjc32APUBJNYKhkr02
LQ5MwHwYDVR0jBBgwFoAUtFrkpbPe0lL2udWmlQ/rPrzH/f8wDQYJKoZIhvcNAQELBQADggEBAC4
65FJhPqel7zJngHIHJrqj/wVAxGAFOTF396XKATGAp+HRCqJ81Ry60CNK1jDzk8dv6M6UHoS7RIF
iM/9rXQCbJfiPD5xMTejZp5n5UYHAmxsxDaazfA5FuBhkfokKK6jD4Eq91C94xGKb6X4/VkaPF7c
qoBBw/bHxawXc0UEPjqayiBpCYU/rJoVZgLqFVP7Px3sva1nOrNx8rPPI1hJ+ZOg8maiPTxHZnBV
LakSSLQy/sWeWyazO1RnrbxjrbgQtYKz0e3nwGpu1w13vfckFmUSBhHXH7AAS/HpKC4IH7G2GAk3
+n8iSSN71sZzpxonQwVbopMZqLmbBm/7WPLcAAJRZtK1pHRuu/Uw+Y91KCaqMAHKWeVJvuqjiTaE
lrahsx+HYoZ1+8i5BMY1NOL/y4TR9qZdxY+7NvNrEdEoFgcI/DqUN0aKs0zAIPmk92pFnjnbro5L
xWRm3JbtIFcG6PdN+9aAbISrewt6EERIPhS45aH+Si08NLrvM+CcEBfqBBqOD+4LCZqT8nDBtALJ
yRqiykibsAAFRgAAAAF6MZUNak74BhbcgvZ2M8WhZKjQyCix7GJzRs4SqnD7iXoxlQ7YXjsVI0K/
xyMOJPkT9ZcPEi/2jHGIte1ZduW4Cvu8C3q0AAAAAAAAAAAAAAAIAAAAAAAAABwAAAQAAAAAAAwA
AAA==

Mbed TLS version:
major 2
minor 21
path 0

Enabled session and context configuration:
MBEDTLS_HAVE_TIME
MBEDTLS_X509_CRT_PARSE_C
MBEDTLS_SSL_MAX_FRAGMENT_LENGTH
MBEDTLS_SSL_TRUNCATED_HMAC
MBEDTLS_SSL_ENCRYPT_THEN_MAC
MBEDTLS_SSL_SESSION_TICKETS
MBEDTLS_SSL_SESSION_TICKETS and client
MBEDTLS_SSL_DTLS_CONNECTION_ID
MBEDTLS_SSL_DTLS_BADMAC_LIMIT
MBEDTLS_SSL_DTLS_ANTI_REPLAY
MBEDTLS_SSL_ALPN

Session info:
start time : 2020-04-07 11:34:27
ciphersuite : TLS-ECDHE-RSA-WITH-CHACHA20-POLY1305-SHA256
cipher flags : 0x00
cipher : CHACHA20-POLY1305
Message-Digest : SHA256
compression : disabled
session ID : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
master secret : 61 EE 1F 3F 6A 9A CB 37 57 D2 D0 AC 0B 91 9C CA
72 C6 B9 2C DB 43 5B BB 18 BB B7 75 CF B7 CD 7A
DB 91 8C B2 69 5F F0 35 99 34 AA 78 16 3B 96 2F
verify result : 0x00000000

Certificate:
cert. version : 3
serial number : 02
issuer name : C=NL, O=PolarSSL, CN=PolarSSL Test CA
subject name : C=NL, O=PolarSSL, CN=localhost
issued on : 2019-02-10 14:44:06
expires on : 2029-02-10 14:44:06
signed using : RSA with SHA-256
RSA key size : 2048 bits
basic constraints : CA=false

Ticket:
59 B4 AD 69 1D 1B AE FD 4C 3E 63 DD 4A 09 AA 8C 00 72 96 79 52 6F
BA A8 E2 4D A1 25 AD A8 6C C7 E1 D8 A1 9D 7E F2 2E 41 31 8D 4D 38
BF F2 E1 34 7D A9 97 71 63 EE CD BC DA C4 74 4A 05 81 C2 3F 0E A5
0D D1 A2 AC D3 30 08 3E 69 3D DA 91 67 8E 76 EB A3 92 F1 59 19 B7
25 BB 48 15 C1 BA 3D D3 7E F5 A0 1B 21 2A DE C2 DE 84 11 12 0F 85
2E 39 68 7F 92 8B 4F 0D 2E BB CC F8 27 04 05 FA 81 06 A3 83 FB 82
C2 66 A4 FC 9C 30 6D 00 B2 72 46 A8 B2 92 26 EC

lifetime : 86400 sec.

Session others:
MFL : none
negotiate truncated HMAC : disabled
Encrypt-then-MAC : disabled

Random bytes:
5E 8C 65 43 5A 93 BE 01 85 B7 20 BD 9D 8C F1 68 59 2A 34 32 0A 2C
7B 18 9C D1 B3 84 AA 9C 3E E2 5E 8C 65 43 B6 17 8E C5 48 D0 AF F1
C8 C3 89 3E 44 FD 65 C3 C4 8B FD A3 1C 62 2D 7B 56 5D B9 6E

Context others:
in CID : BE EF
out CID : DE AD
bad MAC seen number : 0
last validated record sequence no. : 00 00 00 00 00 00 00 02
bitmask for replay detection : 00 00 00 00 00 00 00 07
DTLS datagram packing : enabled
outgoing record sequence no. : 00 01 00 00 00 00 00 03
MTU : 0
ALPN negotiation : not selected

mpg · 2020-08-17T08:15:54Z

Before this patch, running ssl_context_info with valid file input was exiting not because of EOF but because of error "Too many bad symbols detected"
$ ./programs/ssl/ssl_context_info -f ./tests/data_files/base64/cli_cid.txt
[...]
ERROR: Too many bad symbols detected. File check aborted.

I can't reproduce that on my system (Ubuntu 20.04 running on x86-64, development branch). Perhaps this is platform-specific?

naynajain · 2020-08-17T15:21:42Z

Before this patch, running ssl_context_info with valid file input was exiting not because of EOF but because of error "Too many bad symbols detected"
$ ./programs/ssl/ssl_context_info -f ./tests/data_files/base64/cli_cid.txt
[...]
ERROR: Too many bad symbols detected. File check aborted.

I can't reproduce that on my system (Ubuntu 20.04 running on x86-64, development branch). Perhaps this is platform-specific?

Hmm.. so I retried on x86 with RHEL, I didn't get this error.
But when I ran it on Raspberry which has Ubuntu 20.04, I did get this error

Even if this error doesn't appear, there is a valid issue of EOF not getting interpreted correctly. So, I assume the PR and the fix is still applicable.

The difference in tests behaviour.. I will try to look at separately. Sounds ok ?

Thanks & Regards,
- Nayna

mpg · 2020-08-18T07:39:59Z

Ok, thanks for confirming your environment. I think the difference comes from the fact that char is unsigned by default with GCC on Arm targets, while it's signed by default on most other targets, at least on x86. Actually, as I think you noted earlier, just building the program for an Arm target with GCC gives a warning:

ssl/ssl_context_info.c: In function ‘read_next_b64_code’:
ssl/ssl_context_info.c:384:16: warning: comparison is always true due to
limited range of data type [-Wtype-limits]
     while( EOF != c )
                ^

We should have at least one build of the programs targeting Arm in our CI. We already have an issue open tracking that (and more): #3299

So, I assume the PR and the fix is still applicable.

Yes, absolutely. As a matter of principle when a new test is added I always try to run the test without the fix to make sure the test indeed fails when it's supposed to. Here I should have done that on a Arm platform. That's not an issue with the PR, just an issue with how I tested it!

I'll finish reviewing the PR soon, then.

mpg

Looks good to me.

gilles-peskine-arm

The bug fix is ok. The changelog entry needs work. The added test isn't a non-regression test.

gilles-peskine-arm · 2020-08-18T22:20:26Z

tests/context-info.sh

@@ -400,7 +400,12 @@ run_test "Wrong base64 format" \
         "def_bad_b64.txt" \
         -m "ERROR" \
         -u "The length of the base64 code found should be a multiple of 4" \
-         -n "bytes left to analyze from context"
+         -n "bytes left to analyze from context" \


There shouldn't be a trailing backslash here since there is no continuation line.

gilles-peskine-arm · 2020-08-18T22:20:30Z

tests/context-info.sh

+run_test "EOF base64 format" \
+         "def_bad_b64_eof.txt" \
+         -m "Finished. No valid base64 code found" \
+         -n "ERROR" \


There shouldn't be a trailing backslash here since there is no continuation line.

gilles-peskine-arm · 2020-08-18T22:22:46Z

tests/data_files/base64/def_bad_b64_eof.txt

@@ -0,0 +1 @@
+/w==


There's no “EOFish” character (byte 0xff) here. This is a well-formed file containing Base64 data. The encoded data isn't a valid SSL context.

There's no “EOFish” character (byte 0xff) here. This is a well-formed file containing Base64 data. The encoded data isn't a valid SSL context.

Thanks for your review.
It is actually base64 encoding of 0xFF. So, not sure what do you mean by "no "EOFish""

And yes, it is not a valid SSL Context. It is just meant to represent EOF to test that it can be interpreted correctly by read_next_b64_code() function.

Am I missing something ?

Thanks & Regards,
- Nayna

The bug was in the parsing of the base64-encoded, not in the handling of the decoded data. So the fact that the decoded data contains a 0xff is irrelevant. What matters is when the file itself contains a 0xff.

gilles-peskine-arm · 2020-08-18T22:23:10Z

ChangeLog.d/bugfix_type_limit_warning_ssl_context_info.txt

@@ -0,0 +1,3 @@
+Bugfix
+   * read_next_b64_code() isn't correctly checking against EOF because it checks against 'char' rather than 'int'. This is identified via type-limit warning reported by gcc.


Please format changelog entries to 80 columns.

gilles-peskine-arm · 2020-08-18T22:24:33Z

ChangeLog.d/bugfix_type_limit_warning_ssl_context_info.txt

@@ -0,0 +1,3 @@
+Bugfix
+   * read_next_b64_code() isn't correctly checking against EOF because it checks against 'char' rather than 'int'. This is identified via type-limit warning reported by gcc.


Please word changelog entries for users of the libraries, not for maintainers. read_next_b64_code is not meaningful to users, ssl_context_info is. This is a bug in ssl_context_info which, on some platforms, is silently truncating base64 data at character \377 rather than ignoring it.

gilles-peskine-arm · 2020-10-16T20:15:21Z

Thanks for the fix! @d3zd3z has written a non-regression test, so we'll merge this via #3799.

naynajain · 2020-10-19T17:43:26Z

Thanks for the fix! @d3zd3z has written a non-regression test, so we'll merge this via #3799.

Thanks Gilles for accepting the fix with David's regression test. Thank you for closing it.

Thanks & Regards,
- Nayna

gilles-peskine-arm changed the title ~~Bugfix 1~~ Fix build errors without MBEDTLS_HAVE_TIME Jun 23, 2020

gilles-peskine-arm added bug component-platform Portability layer and build scripts needs-backports Backports are missing or are pending review and approval. needs-review Every commit must be reviewed by at least two team members, labels Jun 23, 2020

gilles-peskine-arm reviewed Jun 23, 2020

View reviewed changes

ChangeLog.d/bugfix_1.txt Outdated

@@ -0,0 +1,3 @@

Bugfix:

Copy link

Contributor

gilles-peskine-arm Jun 23, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be Bugfix without :

gilles-peskine-arm reviewed Jun 23, 2020

View reviewed changes

gilles-peskine-arm added needs-work and removed needs-backports Backports are missing or are pending review and approval. needs-review Every commit must be reviewed by at least two team members, labels Jun 23, 2020

gilles-peskine-arm mentioned this pull request Jun 23, 2020

Fix compile errors when MBEDTLS_HAVE_TIME is not defined #3444

Closed

danh-arm assigned gilles-peskine-arm Jun 25, 2020

naynajain changed the title ~~Fix build errors without MBEDTLS_HAVE_TIME~~ Fix ssl_context_info.c to correctly check for EOF - type-limit bug Aug 14, 2020

naynajain added 2 commits August 14, 2020 15:15

programs/ssl: add EOF testcase for ssl_context_info.c

b0aadc4

This patch updates the test cases to also check for EOF correctly. Signed-off-by: Nayna Jain <[email protected]>

naynajain force-pushed the bugfix-1 branch from e66495b to b0aadc4 Compare August 14, 2020 15:17

hanno-becker mentioned this pull request Aug 14, 2020

Fix type conversion bug in ssl_context_info example program. #3567

Closed

mpg mentioned this pull request Aug 18, 2020

Add arm-linux-gnueabi-gcc build #3572

Merged

4 tasks

mpg approved these changes Aug 18, 2020

View reviewed changes

mpg added needs-review Every commit must be reviewed by at least two team members, and removed needs-work labels Aug 18, 2020

mpg added component-tls and removed component-platform Portability layer and build scripts labels Aug 18, 2020

gilles-peskine-arm requested changes Aug 18, 2020

View reviewed changes

gilles-peskine-arm added needs-work and removed needs-review Every commit must be reviewed by at least two team members, labels Aug 18, 2020

daxtens mentioned this pull request Aug 31, 2020

RFC: Fix builds with MBEDTLS_HAVE_TIME disabled and test #3624

Merged

4 tasks

giordano mentioned this pull request Sep 16, 2020

Bump mbedtls to v2.24.0 JuliaPackaging/Yggdrasil#1674

Merged

daverodgman added the Community label Oct 2, 2020

gilles-peskine-arm mentioned this pull request Oct 15, 2020

programs/ssl/ssl_context_info.c fails to compile if char is unsigned #3794

Closed

d3zd3z mentioned this pull request Oct 16, 2020

Fix ssl_context_info handling of EOF (build failure if char is unsigned) #3799

Merged

4 tasks

gilles-peskine-arm closed this Oct 16, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix ssl_context_info.c to correctly check for EOF - type-limit bug #3449

Fix ssl_context_info.c to correctly check for EOF - type-limit bug #3449

naynajain commented Jun 23, 2020 •

edited by mpg

Loading

gilles-peskine-arm Jun 23, 2020

gilles-peskine-arm Jun 23, 2020

gilles-peskine-arm Jun 23, 2020

gilles-peskine-arm Jun 23, 2020 •

edited

Loading

naynajain Aug 5, 2020

gilles-peskine-arm Aug 6, 2020

gilles-peskine-arm commented Jun 23, 2020

naynajain commented Aug 5, 2020 •

edited

Loading

gilles-peskine-arm commented Aug 5, 2020

naynajain commented Aug 6, 2020

gilles-peskine-arm commented Aug 6, 2020

naynajain commented Aug 6, 2020

naynajain commented Aug 14, 2020

naynajain commented Aug 14, 2020

mpg commented Aug 17, 2020

naynajain commented Aug 17, 2020

mpg commented Aug 18, 2020

mpg left a comment

gilles-peskine-arm left a comment

gilles-peskine-arm Aug 18, 2020

gilles-peskine-arm Aug 18, 2020

gilles-peskine-arm Aug 18, 2020

naynajain Aug 19, 2020

gilles-peskine-arm Aug 19, 2020

gilles-peskine-arm Aug 18, 2020

gilles-peskine-arm Aug 18, 2020

gilles-peskine-arm commented Oct 16, 2020

naynajain commented Oct 19, 2020

		@@ -0,0 +1,3 @@
		Bugfix:
		* x509: do not include time.h without MBEDTLS_HAVE_TIME.

		@@ -0,0 +1,3 @@
		Bugfix
		* read_next_b64_code() isn't correctly checking against EOF because it checks against 'char' rather than 'int'. This is identified via type-limit warning reported by gcc.

		@@ -0,0 +1 @@
		/w==

Fix ssl_context_info.c to correctly check for EOF - type-limit bug #3449

Fix ssl_context_info.c to correctly check for EOF - type-limit bug #3449

Conversation

naynajain commented Jun 23, 2020 • edited by mpg Loading

Description

Status

Requires Backporting

Migrations

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gilles-peskine-arm Jun 23, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gilles-peskine-arm commented Jun 23, 2020

naynajain commented Aug 5, 2020 • edited Loading

gilles-peskine-arm commented Aug 5, 2020

naynajain commented Aug 6, 2020

gilles-peskine-arm commented Aug 6, 2020

naynajain commented Aug 6, 2020

naynajain commented Aug 14, 2020

naynajain commented Aug 14, 2020

mpg commented Aug 17, 2020

naynajain commented Aug 17, 2020

mpg commented Aug 18, 2020

mpg left a comment

Choose a reason for hiding this comment

gilles-peskine-arm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gilles-peskine-arm commented Oct 16, 2020

naynajain commented Oct 19, 2020

naynajain commented Jun 23, 2020 •

edited by mpg

Loading

gilles-peskine-arm Jun 23, 2020 •

edited

Loading

naynajain commented Aug 5, 2020 •

edited

Loading