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

One way key update #314

Merged
merged 4 commits into from
Nov 26, 2018

Conversation

kazu-yamamoto
Copy link
Collaborator

This should fix #311.

@ocheron
Copy link
Contributor

ocheron commented Nov 22, 2018

OK but I still think it would be good to add an argument to updateKey and let QuickCheck test one-way update too.

@kazu-yamamoto
Copy link
Collaborator Author

Done. But would you check runTLSPipeSimpleKeyUpdate? I don't know how to write the test case smartly in this case.

@ocheron
Copy link
Contributor

ocheron commented Nov 25, 2018

With QuickCheck the canonical way is to put Arbitrary instances as parameters to the properties, so that generated values are displayed in case of test failure.

prop_handshake_keyupdate :: KeyUpdateRequest -> KeyUpdateRequest -> Property
prop_handshake_keyupdate reqClient reqServer = monadicIO $ do
    params <- pick arbitraryPairParams
    let possible = isVersionEnabled TLS13 params
    runTLSPipeN 3 params (tlsServer possible) (tlsClient possible)
  where tlsServer possible ctx queue = do
            handshake ctx
            d0 <- recvDataNonNull ctx
            sent <- updateKey ctx reqServer
            possible `assertEq` sent
            d1 <- recvDataNonNull ctx
            d2 <- recvDataNonNull ctx
            writeChan queue [d0,d1,d2]
        tlsClient possible queue ctx = do
            handshake ctx
            d0 <- readChan queue
            sendData ctx (L.fromChunks [d0])
            d1 <- readChan queue
            sendData ctx (L.fromChunks [d1])
            sent <- updateKey ctx reqClient
            possible `assertEq` sent
            d2 <- readChan queue
            sendData ctx (L.fromChunks [d2])
            bye ctx

But we don't use monadicIO like this right now. Something like 549eb76 will be good enough (updating tls-debug too).

@kazu-yamamoto
Copy link
Collaborator Author

@ocheron Thank you for your explanation.

tls-debug: done.

Ready for merging?

@kazu-yamamoto
Copy link
Collaborator Author

I will merget this. Please register another issue if necessary.

kazu-yamamoto added a commit to kazu-yamamoto/hs-tls that referenced this pull request Nov 26, 2018
@kazu-yamamoto kazu-yamamoto merged commit bf83458 into haskell-tls:master Nov 26, 2018
@kazu-yamamoto kazu-yamamoto deleted the one-way-key-update branch November 26, 2018 01:16
@kazu-yamamoto
Copy link
Collaborator Author

Merged.
Thank you for your review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Key update for one direction only
2 participants