Repository navigation
pppd: Close the link when required MPPE is not negotiated - #651
Merged
paulusmack merged 2 commits intoOct 24, 2026
Merged
Conversation
With require-mppe, require-mppe-40 or require-mppe-128, ccp_reqci() closes the link for a missing or unusable MPPE option only when it rejects the peer's Configure-Request. An empty Configure-Request is not rejected, so pppd acknowledges it. If the peer acknowledges our MPPE request, CCP opens and IPCP comes up, but the peer never asked for MPPE, so no transmit compressor is set up and pppd sends without encryption. README.MPPE says that MPPE must be negotiated successfully in both directions or the link will be disconnected. Close the link whenever MPPE is required for this negotiation (ccp_gotoptions[].mppe) and the peer's request has no acceptable MPPE option, whether the request is rejected or not. The condition dates from when MPPE support was added in 2002, so ppp 2.4.2 and later are affected. Signed-off-by: Semih Baskan <strst.gs@gmail.com>
require-mppe-40 and require-mppe-128 only add a bit to ccp_wantoptions[0].mppe, so later code can clear the last strength bit and the link does not fail. With the radius plugin, an Access-Accept with MS-MPPE-Encryption-Policy 2 (required) and MS-MPPE-Encryption-Types 2 (40-bit only) makes mppe_set_enc_types() clear MPPE_OPT_128. With require-mppe-128 the mask is then 0, ccp_resetci() sees no MPPE at all, and the link comes up without encryption. The same happens with an option list such as "require-mppe-128 nomppe-128". require-mppe and +mppe already set require_mppe. Make require-mppe-40, require-mppe-128 and their + aliases set it too, and let nomppe and -mppe clear it, so that MPPE can still be turned off explicitly. When require_mppe is set but no strength is left, close the link in ccp_resetci() with the message of the existing "both 40-bit and 128-bit disabled" check. Signed-off-by: Semih Baskan <strst.gs@gmail.com>
Member
|
@strrs: Thanks for your work and thanks to @paulusmack for merging! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
With require-mppe, pppd sends without MPPE when the peer acknowledges our MPPE request but sends an empty CCP Configure-Request of its own. ccp_reqci() closes the link for a missing MPPE option only when it rejects the request, and an empty request is acknowledged. The first commit closes the link whenever MPPE is required and the peer's request has no acceptable MPPE option.
The second commit covers a case where the MPPE requirement disappears before CCP starts. With the radius plugin, require-mppe-128 and an MS-MPPE-Encryption-Types reply that allows only 40-bit, mppe_set_enc_types() clears the 128-bit bit and pppd negotiates no MPPE at all. require_mppe now records that MPPE was required (nomppe and -mppe still clear it), and ccp_resetci() closes the link when no strength is left. Option lists that require a strength and then remove it, such as "require-mppe-128 nomppe-128", now fail the same way instead of coming up without MPPE.
Tested on master at 2aef1a8 with the testsuite's network namespace pair. Against a test-only peer that sends an empty CCP Configure-Request, with require-mppe-128 on both sides, the link came up unencrypted before; with the first commit pppd closes it with "MPPE required but peer negotiation failed". With a test-only plugin that makes the radius plugin's mppe_set_enc_types() call for a 40-bit only reply, require-mppe-128 came up without MPPE before; with the second commit it fails with "MPPE required, but both 40-bit and 128-bit disabled.". Normal MPPE links and links with MPPE off come up as before, and make check and runtests pass.