Tentatively marking as sec issue.

It seems that our assumption that the user behaved[1] is invalid.
`iv_required_len` is retrieved via `EVP_CIPHER_iv_length()`, which
returns 12 for aes-128-ccm, while its default is actually 7.
Since we're bailing out for a given length of 12, the actually
desired length is not set, and therefore `openssl_encrypt()`
behaves as if an IV of length 7 would have been passed.

Or is our assumption correct, and the values would have to match
in OpenSSL?

Jakub, what do you think?

[1] <https://github.com/php/php-src/blob/php-7.3.18/ext/openssl/openssl.c#L6444-L6447>

Some additional information: 

The PHP bug has been revealed in the context of this issue [1] on StackOverflow: openssl_encrypt returns a different ciphertext/tag for AES CCM on a 12 bytes IV than Python with the Cryptography library. A check with Java/BouncyCastle confirms the Python result. 

The same applies to the NIST test vector posted here: Python and Java fulfill the test vector, openssl_encrypt does not. 

OpenSSL itself also satisfies the test vector (verified using this code [2] adapted for the test case).

[1]: https://stackoverflow.com/a/61797909
[2]: https://wiki.openssl.org/index.php/EVP_Authenticated_Encryption_and_Decryption#Authenticated_Encryption_using_CCM_mode

Ah I see yeah that condition is wrong for CCM. I will fix this.

Is this really a security issue? If think so, can you please detail how the security is impacted by using a shorter IV in CCM mode?

I am not sure it can cause security problem like revealing credentials etc. but I think producing broken encryption is security-relevant...

IMO the following example illustrates a scenario where the bug severely affects the security of CCM:

For CCM the IV consists of a nonce (e.g. 12 bytes) and a counter (e.g. 3 bytes) [RFC3610, 2.3. Encryption, L = 3][1]. The counter is incremented with each block and ensures that different IVs are used within the same message. As nonce a random nonce for each message is generated or alternatively a random nonce that is incremented for each further message [RFC3610, 5. Nonce Sugesstions][2]. This reliably ensures that different IVs are also used across different messages.

A 12 bytes nonce that is incremented is converted by the bug into a 7 bytes nonce in which the last 5 bytes are truncated so that the nonce is identical for the first 0x100^5 = 2^40 (different) messages (most-significant-byte first order assumed). I.e. for all these messages, the i-th block of each message would be encrypted with the same key/IV(i) pair.

The crucial condition for CCM is that no key/IV pair may be used more than once, as this would destroy the security of CCM, [CCM mode][3], [RFC4309, 2. AES CCM Mode, last section][4].  

In case of a random nonce for each message (instead of an incremented one) the issue would not occur in this form. This means that the bug may have no or serious security implications depending on the implementation details.

[1]: https://tools.ietf.org/html/rfc3610#section-2.3
[2]: https://tools.ietf.org/html/rfc3610#section-5
[3]: https://en.wikipedia.org/wiki/CCM_mode
[4]: https://tools.ietf.org/html/rfc4309#section-2

Ok I think we should treat it as a security issue then.

Just for the interest how can this have a serious security implications (not trying to say that it doesn't impact on security - just wondering why serious). I might be not understanding it correctly but with nonce being 7 bytes (it means L = 8), the counter should be 8 bytes so one can use larger messages for the price of having shorter nonce. With nonce 12 bytes, the pre computation attack will be more difficult so it's probably better for shorter messages but shouldn't the difficulty for 7 bytes nonce be still high enough when it's default?

The problem is not whether the 7 bytes nonce is in principle less secure than the 12 bytes nonce. CCM allows a nounce size of 7 to 13 bytes with regard to different requirements and the security should be comparable.

I don't see the problem with a new implementation either, because if the bug is known to the developers, the implementation can be designed so that the bug does not become a serious security risk. 

I see the problem with a part of already existing applications that were designed without knowledge of this bug. E.g. in the case of the scenario described in my last comment, the developers assume that a 12 bytes nonce is used, which is incremented for each message (at the end), and do not suspect that the nounce is truncated due to the bug, so that the same nounce and thus the same IV sequence is applied for practically all messages. For a mode related to the CTR mode this is the worst case, e.g.: "The reuse of an AES-CCM or AES-GCM nonce/key combination destroys the security guarantees" [1] or "For ... CTR, reusing an IV completely destroys security" [2]. Such a scenario most likely meets the criteria for a serious security problem. 

But of course, there will also be already existing applications that are designed in such a way that the bug has no impact.

[1]: https://tools.ietf.org/html/rfc5084#section-2 
[2]: https://en.wikipedia.org/wiki/Block_cipher_mode_of_operation#Initialization_vector_(IV)

Ok, the patch that applies fine to PHP 7.3 is attached. Will need to check if it applies to later version as there has been some code changes.

As this has been already exposed and the security issue can be treated in PHP code if the user has got a knowledge about the issue, I'm not really sure if we need to backport it to 7.2 and do a security release.

If everyone is fine with that, I can just push it to 7.3+.

I think unless it requires an extraordinary effort it should be backported. The patch doesn't seem to be too complicated...

In fact, 7.2 code looks the same as 7.3, and I just checked the patch seems to fix the issue for 7.2 too.

@stas why did you change the author of the commit? I think you applied it incorrectly. You should be really using `git am`. Please can you re-apply it?

I think you will need to revert and then apply it again. I spent a bit of time on this so I would really appriciate credit for this work. Thanks.

Or maybe better could someone give me access so I can push to security branches so I don't bother Stas and others with things like this. I think it's not really productive and don't really see reason why we are doing this kind of blocking. At least the commiters that are aware of the process should be given access to commit to those branches as well because it seems like a waste of our time to provide patches and let others handle conflicts and other bits.

Jakub, is only the author wrong or something else too?

As discussed in the commit on GitHub - it's fine to leave it as it is.