Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
8508036:
Initial remoting::protocol::Authenticator interface. (Closed)
|
Created:
9 years, 1 month ago by Sergey Ulanov Modified:
9 years, 1 month ago Reviewers:
Wez CC:
chromium-reviews, jamiewalch+watch_chromium.org, hclam+watch_chromium.org, simonmorris+watch_chromium.org, wez+watch_chromium.org, dmaclach+watch_chromium.org, garykac+watch_chromium.org, lambroslambrou+watch_chromium.org, ajwong+watch_chromium.org, sergeyu+watch_chromium.org Base URL:
http://git.chromium.org/chromium/src.git@master Visibility:
Public. |
DescriptionInitial Authenticator interface.
BUG=None
TEST=None
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=110008
Patch Set 1 #
Total comments: 26
Patch Set 2 : - #Patch Set 3 : - #Patch Set 4 : - #Patch Set 5 : - #Messages
Total messages: 18 (0 generated)
This is basic interface I think we should use for our auth implementations. The idea is that an Authenticator instance is provided to protocol::Session implementation, and Session would use it to authenticate the session. It should be enough to implement current IT2Me auth and EKE-based auth for Me2Me, and later for cert-based auth too.
http://codereview.chromium.org/8508036/diff/1/remoting/protocol/authenticator.h File remoting/protocol/authenticator.h (right): http://codereview.chromium.org/8508036/diff/1/remoting/protocol/authenticator... remoting/protocol/authenticator.h:79: virtual std::string GetSharedSecret() const = 0; Alternatively we can return ChannelAuthenticator here, but it would require some refactoring of the ChannelAuthenticator interface, and I'm not sure if it is necessary right now.
Nice! http://codereview.chromium.org/8508036/diff/1/remoting/protocol/authenticator.h File remoting/protocol/authenticator.h (right): http://codereview.chromium.org/8508036/diff/1/remoting/protocol/authenticator... remoting/protocol/authenticator.h:20: // the connection. I don't think the second sentence is necessary. You might instead explain that the Chromoting client and host will repeatedly call their Authenticators and deliver the messages they generate, until they report successful authentication. http://codereview.chromium.org/8508036/diff/1/remoting/protocol/authenticator... remoting/protocol/authenticator.h:22: // Authenticator instances on connection ends may exchange multiple reword: Authenticators may exchange... http://codereview.chromium.org/8508036/diff/1/remoting/protocol/authenticator... remoting/protocol/authenticator.h:26: // in a session-info message. Explain why it is that some of the exchange uses one scheme, and the rest of it uses another, rather than just always using session-info? Are we really abusing session-info? http://codereview.chromium.org/8508036/diff/1/remoting/protocol/authenticator... remoting/protocol/authenticator.h:76: virtual std::string GetPeerCert() const = 0; Do we need GetLocalCert and GetPeerCert? Isn't certificate verification internal to an Authenticator? e.g. a certificate-verifying Authenticator would be created by calling code and supplied with a store of permitted certificates, and the Authenticator then passed to the main Chromoting code, where it will set up channels and check the certs against the store? http://codereview.chromium.org/8508036/diff/1/remoting/protocol/authenticator... remoting/protocol/authenticator.h:79: virtual std::string GetSharedSecret() const = 0; On 2011/11/10 00:12:51, sergeyu wrote: > Alternatively we can return ChannelAuthenticator here, but it would require some > refactoring of the ChannelAuthenticator interface, and I'm not sure if it is > necessary right now. We'll need to be able to return different kinds of ChannelAuthenticator to implement the EKE-based protocol. http://codereview.chromium.org/8508036/diff/1/remoting/protocol/authenticator... remoting/protocol/authenticator.h:85: // authenticator for the new session. |first_message| specified typo: specified -> specifies (or "provides") http://codereview.chromium.org/8508036/diff/1/remoting/protocol/authenticator... remoting/protocol/authenticator.h:87: // appropriate type of Authenticator can be choosed for the session typo: choosed -> chosen http://codereview.chromium.org/8508036/diff/1/remoting/protocol/authenticator... remoting/protocol/authenticator.h:91: // for the result of this method. Although the last line makes sense, it's an implementation detail not relevant to the caller, I don't think. Should the caller make sure to invoke GetNextMessage() and send whatever it returns to the peer, if the Authenticator is non-NULL, though?
http://codereview.chromium.org/8508036/diff/1/remoting/protocol/authenticator.h File remoting/protocol/authenticator.h (right): http://codereview.chromium.org/8508036/diff/1/remoting/protocol/authenticator... remoting/protocol/authenticator.h:20: // the connection. On 2011/11/10 02:26:59, Wez wrote: > I don't think the second sentence is necessary. > > You might instead explain that the Chromoting client and host will repeatedly > call their Authenticators and deliver the messages they generate, until they > report successful authentication. Done. http://codereview.chromium.org/8508036/diff/1/remoting/protocol/authenticator... remoting/protocol/authenticator.h:22: // Authenticator instances on connection ends may exchange multiple On 2011/11/10 02:26:59, Wez wrote: > reword: Authenticators may exchange... Done. http://codereview.chromium.org/8508036/diff/1/remoting/protocol/authenticator... remoting/protocol/authenticator.h:26: // in a session-info message. On 2011/11/10 02:26:59, Wez wrote: > Explain why it is that some of the exchange uses one scheme, and the rest of it > uses another, rather than just always using session-info? Done. > Are we really abusing session-info? No. Session-info is designed for stuff like this. http://xmpp.org/extensions/xep-0166.html#def-action-session-info : The session-info action is used to send session-level information, such as a session ping or (for Jingle RTP sessions) a ringing message. Not that it is different from transport-info messages. http://codereview.chromium.org/8508036/diff/1/remoting/protocol/authenticator... remoting/protocol/authenticator.h:76: virtual std::string GetPeerCert() const = 0; On 2011/11/10 02:26:59, Wez wrote: > Do we need GetLocalCert and GetPeerCert? Isn't certificate verification > internal to an Authenticator? > > e.g. a certificate-verifying Authenticator would be created by calling code and > supplied with a store of permitted certificates, and the Authenticator then > passed to the main Chromoting code, where it will set up channels and check the > certs against the store? GetPeerCert() and GetLocalCert() return certs that should be used for SSL connections of the channels. protocol::Session will use these to get certs when it starts SSL handshake. I think LocalCert is always specified when authentication is created or not specified at all. Potentially it can be passed directly to Session() when it is created, but I think it is cleaner when we keep cert-management in the authenticator. In the current IT2Me implementation as well as EKE-based auth the host cert is received over signalling channel and it will be returned by GetPeerCert(). On the host side GetPeerCert() would return empty string (i.e. client certs are disabled). For proper cert-based authentication we would have to replace GetPeerCert() with VerifyPeerCert() function - but the problem is that it would not be usable with our current SSL stack because it doesn't allow to use an arbitrary cert verifier - net::CertVerifier is not an abstract interface. I've added TODO for this. http://codereview.chromium.org/8508036/diff/1/remoting/protocol/authenticator... remoting/protocol/authenticator.h:79: virtual std::string GetSharedSecret() const = 0; On 2011/11/10 02:26:59, Wez wrote: > On 2011/11/10 00:12:51, sergeyu wrote: > > Alternatively we can return ChannelAuthenticator here, but it would require > some > > refactoring of the ChannelAuthenticator interface, and I'm not sure if it is > > necessary right now. > > We'll need to be able to return different kinds of ChannelAuthenticator to > implement the EKE-based protocol. Ok I've replaced it with CreateChannelAuthenticator() http://codereview.chromium.org/8508036/diff/1/remoting/protocol/authenticator... remoting/protocol/authenticator.h:85: // authenticator for the new session. |first_message| specified On 2011/11/10 02:26:59, Wez wrote: > typo: specified -> specifies (or "provides") Done. http://codereview.chromium.org/8508036/diff/1/remoting/protocol/authenticator... remoting/protocol/authenticator.h:87: // appropriate type of Authenticator can be choosed for the session On 2011/11/10 02:26:59, Wez wrote: > typo: choosed -> chosen Done. http://codereview.chromium.org/8508036/diff/1/remoting/protocol/authenticator... remoting/protocol/authenticator.h:91: // for the result of this method. On 2011/11/10 02:26:59, Wez wrote: > Although the last line makes sense, it's an implementation detail not relevant > to the caller, I don't think. > > Should the caller make sure to invoke GetNextMessage() and send whatever it > returns to the peer, if the Authenticator is non-NULL, though? The idea is that the caller must call ProcessMessage(), it is not called inside of CreateAuthenticator(). I.e. it is not an implementation detail. Reworder this sentence to make it clearer.
http://codereview.chromium.org/8508036/diff/1/remoting/protocol/authenticator.h File remoting/protocol/authenticator.h (right): http://codereview.chromium.org/8508036/diff/1/remoting/protocol/authenticator... remoting/protocol/authenticator.h:76: virtual std::string GetPeerCert() const = 0; On 2011/11/10 23:17:47, sergeyu wrote: > On 2011/11/10 02:26:59, Wez wrote: > > Do we need GetLocalCert and GetPeerCert? Isn't certificate verification > > internal to an Authenticator? > > > > e.g. a certificate-verifying Authenticator would be created by calling code > and > > supplied with a store of permitted certificates, and the Authenticator then > > passed to the main Chromoting code, where it will set up channels and check > the > > certs against the store? > > GetPeerCert() and GetLocalCert() return certs that should be used for SSL > connections of the channels. protocol::Session will use these to get certs when > it starts SSL handshake. > I think LocalCert is always specified when authentication is created or not > specified at all. Potentially it can be passed directly to Session() when it is > created, but I think it is cleaner when we keep cert-management in the > authenticator. > In the current IT2Me implementation as well as EKE-based auth the host cert is > received over signalling channel and it will be returned by GetPeerCert(). On > the host side GetPeerCert() would return empty string (i.e. client certs are > disabled). > For proper cert-based authentication we would have to replace GetPeerCert() with > VerifyPeerCert() function - but the problem is that it would not be usable with > our current SSL stack because it doesn't allow to use an arbitrary cert verifier > - net::CertVerifier is not an abstract interface. I've added TODO for this. Let's discuss this offline; I think it's important to make it clear _how_ the certs should be used, if we're specifying them here. Note that the EKE-based authentication scheme doesn't require verification of the host certificate. http://codereview.chromium.org/8508036/diff/1/remoting/protocol/authenticator... remoting/protocol/authenticator.h:91: // for the result of this method. On 2011/11/10 23:17:47, sergeyu wrote: > On 2011/11/10 02:26:59, Wez wrote: > > Although the last line makes sense, it's an implementation detail not relevant > > to the caller, I don't think. > > > > Should the caller make sure to invoke GetNextMessage() and send whatever it > > returns to the peer, if the Authenticator is non-NULL, though? > > The idea is that the caller must call ProcessMessage(), it is not called inside > of CreateAuthenticator(). I.e. it is not an implementation detail. Reworder this > sentence to make it clearer. Yes, making it clearer would be good! My reading of the comment is that CreateAuthenticator() should _internally_ make sure to call ProcessMessage() with |first_message|, but it sounds like that's not what you mean. In which case, what is |first_message| used for?
http://codereview.chromium.org/8508036/diff/1/remoting/protocol/authenticator.h File remoting/protocol/authenticator.h (right): http://codereview.chromium.org/8508036/diff/1/remoting/protocol/authenticator... remoting/protocol/authenticator.h:76: virtual std::string GetPeerCert() const = 0; On 2011/11/11 01:17:42, Wez wrote: > On 2011/11/10 23:17:47, sergeyu wrote: > > On 2011/11/10 02:26:59, Wez wrote: > > > Do we need GetLocalCert and GetPeerCert? Isn't certificate verification > > > internal to an Authenticator? > > > > > > e.g. a certificate-verifying Authenticator would be created by calling code > > and > > > supplied with a store of permitted certificates, and the Authenticator then > > > passed to the main Chromoting code, where it will set up channels and check > > the > > > certs against the store? > > > > GetPeerCert() and GetLocalCert() return certs that should be used for SSL > > connections of the channels. protocol::Session will use these to get certs > when > > it starts SSL handshake. > > I think LocalCert is always specified when authentication is created or not > > specified at all. Potentially it can be passed directly to Session() when it > is > > created, but I think it is cleaner when we keep cert-management in the > > authenticator. > > In the current IT2Me implementation as well as EKE-based auth the host cert is > > received over signalling channel and it will be returned by GetPeerCert(). On > > the host side GetPeerCert() would return empty string (i.e. client certs are > > disabled). > > For proper cert-based authentication we would have to replace GetPeerCert() > with > > VerifyPeerCert() function - but the problem is that it would not be usable > with > > our current SSL stack because it doesn't allow to use an arbitrary cert > verifier > > - net::CertVerifier is not an abstract interface. I've added TODO for this. > > Let's discuss this offline; I think it's important to make it clear _how_ the > certs should be used, if we're specifying them here. Removed these methods for now. > Note that the EKE-based authentication scheme doesn't require verification of > the host certificate. It doesn't require it, but I think it is still useful to verify that host cert matches what we receive from directory. http://codereview.chromium.org/8508036/diff/1/remoting/protocol/authenticator... remoting/protocol/authenticator.h:91: // for the result of this method. On 2011/11/11 01:17:42, Wez wrote: > On 2011/11/10 23:17:47, sergeyu wrote: > > On 2011/11/10 02:26:59, Wez wrote: > > > Although the last line makes sense, it's an implementation detail not > relevant > > > to the caller, I don't think. > > > > > > Should the caller make sure to invoke GetNextMessage() and send whatever it > > > returns to the peer, if the Authenticator is non-NULL, though? > > > > The idea is that the caller must call ProcessMessage(), it is not called > inside > > of CreateAuthenticator(). I.e. it is not an implementation detail. Reworder > this > > sentence to make it clearer. > > Yes, making it clearer would be good! My reading of the comment is that > CreateAuthenticator() should _internally_ make sure to call ProcessMessage() > with |first_message|, but it sounds like that's not what you mean. In which > case, what is |first_message| used for? It's used to determine what type of authenticator should be created in case the host supports multiple authentication algorithms. For example, if |first_message| contains <auth-token> then this method will create Authenticator that implements our current IT2Me auth, if there is an eke message in |first_message| then it will create EkeAuthenticator, etc.
http://codereview.chromium.org/8508036/diff/1/remoting/protocol/authenticator.h File remoting/protocol/authenticator.h (right): http://codereview.chromium.org/8508036/diff/1/remoting/protocol/authenticator... remoting/protocol/authenticator.h:60: virtual State ProcessMessage(talk_base::XmlElement* message) = 0; For the existing IT2Me authentication scheme, we know whether we accept the session by the time ProcessMessage() returns, but we still have to send a message back to the peer, and the peer isn't going to send us another message to process. So I think we really need to separate State out from ProcessMessage() or GetNextMessage(); the caller just keeps calling us and fetching another message for ProcessMessage(), or calling GetNextMessage() and sending it, depending on what state we're in, until the State is Accepted or Rejected? http://codereview.chromium.org/8508036/diff/1/remoting/protocol/authenticator... remoting/protocol/authenticator.h:91: // for the result of this method. On 2011/11/11 19:16:41, sergeyu wrote: > On 2011/11/11 01:17:42, Wez wrote: > > On 2011/11/10 23:17:47, sergeyu wrote: > > > On 2011/11/10 02:26:59, Wez wrote: > > > > Although the last line makes sense, it's an implementation detail not > > relevant > > > > to the caller, I don't think. > > > > > > > > Should the caller make sure to invoke GetNextMessage() and send whatever > it > > > > returns to the peer, if the Authenticator is non-NULL, though? > > > > > > The idea is that the caller must call ProcessMessage(), it is not called > > inside > > > of CreateAuthenticator(). I.e. it is not an implementation detail. Reworder > > this > > > sentence to make it clearer. > > > > Yes, making it clearer would be good! My reading of the comment is that > > CreateAuthenticator() should _internally_ make sure to call ProcessMessage() > > with |first_message|, but it sounds like that's not what you mean. In which > > case, what is |first_message| used for? > > It's used to determine what type of authenticator should be created in case the > host supports multiple authentication algorithms. For example, if > |first_message| contains <auth-token> then this method will create Authenticator > that implements our current IT2Me auth, if there is an eke message in > |first_message| then it will create EkeAuthenticator, etc. So is the purpose of your comment to say that even though you've passed the message in when creating the thing, you still have to call ProcessMessage() afterwards, with the same message?
http://codereview.chromium.org/8508036/diff/1/remoting/protocol/authenticator.h File remoting/protocol/authenticator.h (right): http://codereview.chromium.org/8508036/diff/1/remoting/protocol/authenticator... remoting/protocol/authenticator.h:60: virtual State ProcessMessage(talk_base::XmlElement* message) = 0; On 2011/11/11 22:12:00, Wez wrote: > For the existing IT2Me authentication scheme, we know whether we accept the > session by the time ProcessMessage() returns, but we still have to send a > message back to the peer, and the peer isn't going to send us another message to > process. I don't think there is a problem for the existing scheme. On the client side Authenticator will go from MESSAGE_READY to ACCEPTED after first auth message is sent with session-initiate. On the host side the Authenticator receives the message and goes from WAITING_MESSAGE to ACCEPTED or REJECTED. Authenticator doesn't need to worry about sending the session-accept message - Session will need to do it even if the AUTHENTICATOR is in the ACCEPTED state. > > So I think we really need to separate State out from ProcessMessage() or > GetNextMessage(); the caller just keeps calling us and fetching another message > for ProcessMessage(), or calling GetNextMessage() and sending it, depending on > what state we're in, until the State is Accepted or Rejected? http://codereview.chromium.org/8508036/diff/1/remoting/protocol/authenticator... remoting/protocol/authenticator.h:91: // for the result of this method. On 2011/11/11 22:12:00, Wez wrote: > On 2011/11/11 19:16:41, sergeyu wrote: > > On 2011/11/11 01:17:42, Wez wrote: > > > On 2011/11/10 23:17:47, sergeyu wrote: > > > > On 2011/11/10 02:26:59, Wez wrote: > > > > > Although the last line makes sense, it's an implementation detail not > > > relevant > > > > > to the caller, I don't think. > > > > > > > > > > Should the caller make sure to invoke GetNextMessage() and send whatever > > it > > > > > returns to the peer, if the Authenticator is non-NULL, though? > > > > > > > > The idea is that the caller must call ProcessMessage(), it is not called > > > inside > > > > of CreateAuthenticator(). I.e. it is not an implementation detail. > Reworder > > > this > > > > sentence to make it clearer. > > > > > > Yes, making it clearer would be good! My reading of the comment is that > > > CreateAuthenticator() should _internally_ make sure to call ProcessMessage() > > > with |first_message|, but it sounds like that's not what you mean. In which > > > case, what is |first_message| used for? > > > > It's used to determine what type of authenticator should be created in case > the > > host supports multiple authentication algorithms. For example, if > > |first_message| contains <auth-token> then this method will create > Authenticator > > that implements our current IT2Me auth, if there is an eke message in > > |first_message| then it will create EkeAuthenticator, etc. > > So is the purpose of your comment to say that even though you've passed the > message in when creating the thing, you still have to call ProcessMessage() > afterwards, with the same message? Yes.
http://codereview.chromium.org/8508036/diff/1/remoting/protocol/authenticator.h File remoting/protocol/authenticator.h (right): http://codereview.chromium.org/8508036/diff/1/remoting/protocol/authenticator... remoting/protocol/authenticator.h:60: virtual State ProcessMessage(talk_base::XmlElement* message) = 0; On 2011/11/12 00:12:49, sergeyu wrote: > On 2011/11/11 22:12:00, Wez wrote: > > For the existing IT2Me authentication scheme, we know whether we accept the > > session by the time ProcessMessage() returns, but we still have to send a > > message back to the peer, and the peer isn't going to send us another message > to > > process. > > I don't think there is a problem for the existing scheme. On the client side > Authenticator will go from MESSAGE_READY to ACCEPTED after first auth message is > sent with session-initiate. On the host side the Authenticator receives the > message and goes from WAITING_MESSAGE to ACCEPTED or REJECTED. Authenticator > doesn't need to worry about sending the session-accept message - Session will > need to do it even if the AUTHENTICATOR is in the ACCEPTED state. Not sure I understand the last comment; do you mean that the Session will *always* send a session-accept message? I thought the SPAKE2-based auth was going to use initiate->accept->info->info to exchange the messages, so if the Session then sends an accept when it sees we've authenticated, won't that be a duplicate?
On 2011/11/12 00:25:35, Wez wrote: > http://codereview.chromium.org/8508036/diff/1/remoting/protocol/authenticator.h > File remoting/protocol/authenticator.h (right): > > http://codereview.chromium.org/8508036/diff/1/remoting/protocol/authenticator... > remoting/protocol/authenticator.h:60: virtual State > ProcessMessage(talk_base::XmlElement* message) = 0; > On 2011/11/12 00:12:49, sergeyu wrote: > > On 2011/11/11 22:12:00, Wez wrote: > > > For the existing IT2Me authentication scheme, we know whether we accept the > > > session by the time ProcessMessage() returns, but we still have to send a > > > message back to the peer, and the peer isn't going to send us another > message > > to > > > process. > > > > I don't think there is a problem for the existing scheme. On the client side > > Authenticator will go from MESSAGE_READY to ACCEPTED after first auth message > is > > sent with session-initiate. On the host side the Authenticator receives the > > message and goes from WAITING_MESSAGE to ACCEPTED or REJECTED. Authenticator > > doesn't need to worry about sending the session-accept message - Session will > > need to do it even if the AUTHENTICATOR is in the ACCEPTED state. > > Not sure I understand the last comment; do you mean that the Session will > *always* send a session-accept message? I thought the SPAKE2-based auth was > going to use initiate->accept->info->info to exchange the messages, so if the > Session then sends an accept when it sees we've authenticated, won't that be a > duplicate? For EKE exhange the sequence of the events will be as follows (only one accept message): 1. client->host: initiate + 1st EKE message 2. host->client: accept + 2nd EKE message 3. client->host: 3rd EKE message in session-info 4. host->client: 4th EKE message in session-info For the current scheme: 1. client->host: initiate + auth token 2. host->client: accept
On 2011/11/14 19:55:27, sergeyu wrote: > For EKE exhange the sequence of the events will be as follows (only one accept > message): > 1. client->host: initiate + 1st EKE message > 2. host->client: accept + 2nd EKE message > 3. client->host: 3rd EKE message in session-info > 4. host->client: 4th EKE message in session-info On receipt of the 3rd message, the host calls ProcessMessage and gets ACCEPTED, so how does the caller know that it needs to send the 4th message? Alternatively, the host calls ProcessMessage and gets MESSAGE_READY, so calls GetNextMessage() to get the next message to send. It's not going to see another message, so it won't call ProcessMessage(), so it will never get to ACCEPTED? > For the current scheme: > 1. client->host: initiate + auth token > 2. host->client: accept This differs from the new scheme in that the Host has nothing to send initially; how does it determine that?
On 2011/11/14 22:32:02, Wez wrote: > On 2011/11/14 19:55:27, sergeyu wrote: > > For EKE exhange the sequence of the events will be as follows (only one accept > > message): > > 1. client->host: initiate + 1st EKE message > > 2. host->client: accept + 2nd EKE message > > 3. client->host: 3rd EKE message in session-info > > 4. host->client: 4th EKE message in session-info > On receipt of the 3rd message, the host calls ProcessMessage and gets > ACCEPTED, so how does the caller know that it needs to send the 4th message? > Alternatively, the host calls ProcessMessage and gets MESSAGE_READY, so calls > GetNextMessage() to get the next message to send. It's not going to see another > message, so it won't call ProcessMessage(), so it will never get to ACCEPTED? GetNextMessage() can also change state. So when GetNexMessage() is called on the host side for the last message the authenticator goes from MESSAGE_READY to ACCEPTED state - this is mentioned in the list of allowed state transitions. > > > For the current scheme: > > 1. client->host: initiate + auth token > > 2. host->client: accept > > This differs from the new scheme in that the Host has nothing to send initially; > how does it determine that? On the client side the authenticator is created in MESSAGE_READY state. Session message gets the first message message by calling GetNextMessage() and attaches it to session-initiate. This GetNextMessage() call changes state to ACCEPTED. When host authenticator is created on the host side upon receipt of the session-initiate stanza it is initially in the WAITING_MESSAGE state. Then, Session calls ProcessMessage() after which the authenticator goes to ACCEPTED or REJECTED state. If the new state is ACCEPTED the session sends session-accept.
On 2011/11/14 23:15:49, sergeyu wrote: > On 2011/11/14 22:32:02, Wez wrote: > > On 2011/11/14 19:55:27, sergeyu wrote: > > > For EKE exhange the sequence of the events will be as follows (only one > accept > > > message): > > > 1. client->host: initiate + 1st EKE message > > > 2. host->client: accept + 2nd EKE message > > > 3. client->host: 3rd EKE message in session-info > > > 4. host->client: 4th EKE message in session-info > > On receipt of the 3rd message, the host calls ProcessMessage and gets > > ACCEPTED, so how does the caller know that it needs to send the 4th message? > > Alternatively, the host calls ProcessMessage and gets MESSAGE_READY, so > calls > > GetNextMessage() to get the next message to send. It's not going to see > another > > message, so it won't call ProcessMessage(), so it will never get to ACCEPTED? > > GetNextMessage() can also change state. So when GetNexMessage() is called on the > host side for the last message the authenticator goes from MESSAGE_READY to > ACCEPTED state - this is mentioned in the list of allowed state transitions. > > > > > > > For the current scheme: > > > 1. client->host: initiate + auth token > > > 2. host->client: accept > > > > This differs from the new scheme in that the Host has nothing to send > initially; > > how does it determine that? > > On the client side the authenticator is created in MESSAGE_READY state. Session > message gets the first message message by calling GetNextMessage() and attaches Here I meant "Session gets the first message ..." > it to session-initiate. This GetNextMessage() call changes state to ACCEPTED. > When host authenticator is created on the host side upon receipt of the > session-initiate stanza it is initially in the WAITING_MESSAGE state. Then, > Session calls ProcessMessage() after which the authenticator goes to ACCEPTED or > REJECTED state. If the new state is ACCEPTED the session sends session-accept.
On 2011/11/14 23:15:49, sergeyu wrote: > On 2011/11/14 22:32:02, Wez wrote: > > On 2011/11/14 19:55:27, sergeyu wrote: > > > For EKE exhange the sequence of the events will be as follows (only one > accept > > > message): > > > 1. client->host: initiate + 1st EKE message > > > 2. host->client: accept + 2nd EKE message > > > 3. client->host: 3rd EKE message in session-info > > > 4. host->client: 4th EKE message in session-info > > On receipt of the 3rd message, the host calls ProcessMessage and gets > > ACCEPTED, so how does the caller know that it needs to send the 4th message? > > Alternatively, the host calls ProcessMessage and gets MESSAGE_READY, so > calls > > GetNextMessage() to get the next message to send. It's not going to see > another > > message, so it won't call ProcessMessage(), so it will never get to ACCEPTED? > > GetNextMessage() can also change state. So when GetNexMessage() is called on the > host side for the last message the authenticator goes from MESSAGE_READY to > ACCEPTED state - this is mentioned in the list of allowed state transitions. Right - so the caller basically has to call state() and switch depending on the result, to fetch more data for ProcessMessage(), write some data from GetNextMessage(), finish, or fail. That sounds like a straightforward while/switch loop, so I don't think ProcessMessage() returning the new state is very helpful?
lgtm
On 2011/11/15 00:05:19, Wez wrote: > On 2011/11/14 23:15:49, sergeyu wrote: > > On 2011/11/14 22:32:02, Wez wrote: > > > On 2011/11/14 19:55:27, sergeyu wrote: > > > > For EKE exhange the sequence of the events will be as follows (only one > > accept > > > > message): > > > > 1. client->host: initiate + 1st EKE message > > > > 2. host->client: accept + 2nd EKE message > > > > 3. client->host: 3rd EKE message in session-info > > > > 4. host->client: 4th EKE message in session-info > > > On receipt of the 3rd message, the host calls ProcessMessage and gets > > > ACCEPTED, so how does the caller know that it needs to send the 4th message? > > > Alternatively, the host calls ProcessMessage and gets MESSAGE_READY, so > > calls > > > GetNextMessage() to get the next message to send. It's not going to see > > another > > > message, so it won't call ProcessMessage(), so it will never get to > ACCEPTED? > > > > GetNextMessage() can also change state. So when GetNexMessage() is called on > the > > host side for the last message the authenticator goes from MESSAGE_READY to > > ACCEPTED state - this is mentioned in the list of allowed state transitions. > > Right - so the caller basically has to call state() and switch depending on the > result, to fetch more data for ProcessMessage(), write some data from > GetNextMessage(), finish, or fail. That sounds like a straightforward > while/switch loop, so I don't think ProcessMessage() returning the new state is > very helpful? Changed ProcessMessage() result type to void.
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sergeyu@chromium.org/8508036/20001
Change committed as 110008 |
