Feat[MQB]: add authentication with basic logic #746

emelialei88 · 2025-05-13T14:39:54Z

Handing authentication logic on the broker side:

Added authentication protocol.
Introduced a new AuthenticationEvent event type.
Implemented the authenticator component to handle authentication logic, using AuthenticationContext to encapsulate relevant context data.
Updated InitialConnectionHandler to handle the authentication flow.

At this stage, we only handle incoming authentication requests and always allow them to succeed. Outbound, reverse authentication requests initiated by a broker, and re-authentication are not supported.

We also updated the unit test m_bmqstoragetool_cslprinter.t to align with changes made by bascodegen.

src/groups/bmq/bmqp/bmqp_ctrlmsg.xsd

src/groups/mqb/mqba/mqba_initialconnectionhandler.cpp

src/groups/bmq/bmqp/bmqp_ctrlmsg.xsd

src/groups/mqb/mqba/mqba_authenticator.cpp

pniedzielski · 2025-05-14T19:15:10Z

src/groups/mqb/mqba/mqba_authenticator.cpp

+    const AuthenticationContextSp& context)
+{
+    // PRECONDITIONS
+    BSLS_ASSERT_SAFE(


Fine to have preconditions, but they aren't preconditions if they're not documented in the function contract: The behavior of this function is undefined unless .... Otherwise, how would a caller of this function know it will crash if !context->d_initialConnectionContext_p->isIncoming() && !context->d_isReversed? We'd be lying to them.

Second concern: are these preconditions checked or enforced by the caller? I think they are, but the second one below has me worried. Is there any way we could get a bad message over the wire from a client and crash?

I'm not sure I understand the second concern.
isIncoming is set by us when TcpSessionFactory receives an incoming call (true) or sending an outbound message (false).
For negotiation, (and it should be the same for authentication),d_isReversed is default to be false unless we receive an reversed connection request (type bmqp_ctrlmsg::NegotiationMessage ::SELECTION_INDEX_REVERSE_CONNECTION_REQUEST) or we're about to send out a reverse connection request.

pniedzielski · 2025-05-14T19:18:57Z

src/groups/mqb/mqba/mqba_authenticator.cpp

+    bmqp_ctrlmsg::AuthenticateRequest& authenticateRequest =
+        context->d_authenticationMessage.authenticateRequest();
+
+    BALL_LOG_DEBUG


I am a little concerned about logging arbitrary data that we get from the network, but I think the operator<< for authenticateRequest as generated is fine with this. This is another place where base64 may help compress the logs.

I could get rid of printing the data, but sometime maybe we just want to debug and see what it is. Is setting it to debug mode still not okay?

src/groups/mqb/mqba/mqba_authenticator.cpp

src/groups/mqb/mqba/mqba_authenticator.h

src/groups/mqb/mqbnet/mqbnet_authenticator.h

dorjesinpo

We need to at least document/comment how are we going to enforce authentication. Is AuthenticationContext going to have a state/status that someone (InitialConnectionHandler? ClientSession?) is going to check?

src/groups/mqb/mqba/mqba_authenticator.h

dorjesinpo · 2025-05-14T15:20:44Z

src/groups/mqb/mqba/mqba_initialconnectionhandler.cpp

+    else if (event.isControlEvent()) {
+        const int rc = event.loadControlEvent(&negotiationMessage);
+        if (rc != 0) {
+            BALL_LOG_ERROR << "Invalid response from broker [reason: 'control "


Is this a response from broker (used to be negotiation message)?
And we do need to log/dump the blob

And we do need to log/dump the blob

Is this a question? Do you mean we need to keep the error log of event or I should log even there's not an error?

And we do need to log/dump the blob

Is this a question? Do you mean we need to keep the error log of event or I should log even there's not an error?

I think, it would be helpful; to log/dump the blob when there is an error (rc != 0). Seems like we had this in the previous version of the code

src/groups/mqb/mqbnet/mqbnet_authenticationcontext.h

src/groups/mqb/mqba/mqba_initialconnectionhandler.cpp

src/groups/mqb/mqbnet/mqbnet_authenticator.h

src/groups/mqb/mqba/mqba_authenticator.h

Signed-off-by: Emelia Lei <wlei29@bloomberg.net>

dorjesinpo

looks good as is and we could merge it. but would be good to outline how we move forward

dorjesinpo · 2025-05-23T19:28:10Z

src/groups/mqb/mqba/mqba_authenticator.cpp

+    response.status().code()     = 0;
+    response.lifetimeMs()        = 10 * 60 * 1000;
+
+    authenticationContext->state().testAndSwap(


checking return value would look good

dorjesinpo · 2025-05-23T19:30:48Z

src/groups/mqb/mqba/mqba_authenticator.cpp

+    } break;  // BREAK
+    case bmqp_ctrlmsg::AuthenticationMessage::
+        SELECTION_ID_AUTHENTICATE_RESPONSE: {
+        rc = onAuthenticationResponse(errStream, authenticationMsg, context);


Will there be a case when brokers receive AuthenticationResponse? If so, need a comment.

dorjesinpo · 2025-05-23T19:37:37Z

src/groups/mqb/mqba/mqba_authenticator.cpp

+                                       authenticationResponse,
+                                       authenticationContext);
+
+    return rc;


making a note. when it becomes asynchronous, we need to

handle authentication failure in the auth thread (how? by disconnecting the channel?)

to make sure there is an authentication timeout and somebody enforces it

dorjesinpo · 2025-05-23T19:44:31Z

src/groups/mqb/mqba/mqba_initialconnectionhandler.cpp

    }

-    if (negotiationMsg.has_value()) {
+    // Authentication or Negotiation based on the type of message received.


We need a check to reject negotiation message in between authentication request and authentication response.
Also a check to reject (another) authentication request in between authentication request and authentication response.
This could be checking InitialConnectionContext::d_authenticationCtxSp although explicit state would be better (we may need more checks or a mini FSM even)

dorjesinpo · 2025-05-23T19:50:58Z

src/groups/mqb/mqba/mqba_authenticator.cpp

+            State::e_AUTHENTICATING  // state
+        );
+
+    context->setAuthenticationContext(authenticationContext);


What would we give to Client/Admin/ClusterNode session for future authorization(s) if Authenticator::handleAuthentication was never called? What's the plan? Can we document it?

pniedzielski

Because this is going in the integration branch, let's merge. We'll eventually make a PR into main with somethings from the integration branch, so it's not a big issue if this isn't fully fleshed out yet.

Signed-off-by: Emelia Lei <wlei29@bloomberg.net>

emelialei88 requested a review from a team as a code owner May 13, 2025 14:39

emelialei88 requested a review from dorjesinpo May 13, 2025 15:19

emelialei88 assigned dorjesinpo May 13, 2025

emelialei88 requested a review from 678098 May 13, 2025 15:40

emelialei88 force-pushed the feat-authn-protocol-mqb branch from f71824d to aa65195 Compare May 13, 2025 19:57

chrisbeard reviewed May 13, 2025

View reviewed changes

src/groups/bmq/bmqp/bmqp_ctrlmsg.xsd Outdated Show resolved Hide resolved

chrisbeard reviewed May 13, 2025

View reviewed changes

src/groups/mqb/mqba/mqba_initialconnectionhandler.cpp Outdated Show resolved Hide resolved

pniedzielski reviewed May 14, 2025

View reviewed changes

dorjesinpo reviewed May 15, 2025

View reviewed changes

dorjesinpo assigned emelialei88 and unassigned dorjesinpo May 16, 2025

emelialei88 force-pushed the feat-authn-protocol-mqb branch 9 times, most recently from e1fe5b9 to d8123e8 Compare May 20, 2025 21:34

emelialei88 assigned pniedzielski and dorjesinpo and unassigned emelialei88 May 20, 2025

emelialei88 force-pushed the feat-authn-protocol-mqb branch 2 times, most recently from 501f217 to 95f9480 Compare May 20, 2025 21:44

emelialei88 requested review from dorjesinpo and pniedzielski May 20, 2025 21:56

emelialei88 force-pushed the feat-authn-protocol-mqb branch 3 times, most recently from ce8e150 to fa1fdd7 Compare May 22, 2025 18:15

authn functions take InitialConnectionContextSp

02917f1

Signed-off-by: Emelia Lei <wlei29@bloomberg.net>

emelialei88 force-pushed the feat-authn-protocol-mqb branch from b20015d to 02917f1 Compare May 23, 2025 17:53

dorjesinpo requested changes May 23, 2025

View reviewed changes

dorjesinpo assigned emelialei88 and unassigned dorjesinpo May 26, 2025

pniedzielski approved these changes May 28, 2025

View reviewed changes

changed m_bmqstoragetool_cslprinter.t due to bascodegen change

3678933

Signed-off-by: Emelia Lei <wlei29@bloomberg.net>

emelialei88 merged commit a401707 into bloomberg:integration/authn-authz May 29, 2025
33 checks passed

emelialei88 added a commit to emelialei88/blazingmq that referenced this pull request May 29, 2025

Feat[MQB]: add authentication with basic logic (bloomberg#746)

438adc1

Signed-off-by: Emelia Lei <wlei29@bloomberg.net>

emelialei88 added a commit that referenced this pull request Jun 12, 2025

Feat[MQB]: add authentication with basic logic (#746)

01cce76

Signed-off-by: Emelia Lei <wlei29@bloomberg.net>

emelialei88 added a commit that referenced this pull request Jun 18, 2025

Feat[MQB]: add authentication with basic logic (#746)

7666ad4

Signed-off-by: Emelia Lei <wlei29@bloomberg.net>

emelialei88 added a commit that referenced this pull request Jun 20, 2025

Feat[MQB]: add authentication with basic logic (#746)

0e18d65

Signed-off-by: Emelia Lei <wlei29@bloomberg.net>

emelialei88 added a commit that referenced this pull request Jun 20, 2025

Feat[MQB]: add authentication with basic logic (#746)

6dd12f5

Signed-off-by: Emelia Lei <wlei29@bloomberg.net>

emelialei88 added a commit to emelialei88/blazingmq that referenced this pull request Jun 20, 2025

Feat[MQB]: add authentication with basic logic (bloomberg#746)

c980d41

Signed-off-by: Emelia Lei <wlei29@bloomberg.net>

emelialei88 added a commit that referenced this pull request Jun 23, 2025

Feat[MQB]: add authentication with basic logic (#746)

c1291bb

Signed-off-by: Emelia Lei <wlei29@bloomberg.net>

emelialei88 added a commit that referenced this pull request Jun 25, 2025

Feat[MQB]: add authentication with basic logic (#746)

10a9bbf

Signed-off-by: Emelia Lei <wlei29@bloomberg.net>

emelialei88 added a commit to emelialei88/blazingmq that referenced this pull request Jun 25, 2025

Feat[MQB]: add authentication with basic logic (bloomberg#746)

f175d40

Signed-off-by: Emelia Lei <wlei29@bloomberg.net>

emelialei88 added a commit that referenced this pull request Jul 10, 2025

Feat[MQB]: add authentication with basic logic (#746)

b50f4eb

Signed-off-by: Emelia Lei <wlei29@bloomberg.net>

emelialei88 added a commit that referenced this pull request Jul 25, 2025

Feat[MQB]: add authentication with basic logic (#746)

3970732

Signed-off-by: Emelia Lei <wlei29@bloomberg.net>

emelialei88 added a commit that referenced this pull request Jul 28, 2025

Feat[MQB]: add authentication with basic logic (#746)

d9b91f8

Signed-off-by: Emelia Lei <wlei29@bloomberg.net>

emelialei88 added a commit that referenced this pull request Jul 30, 2025

Feat[MQB]: add authentication with basic logic (#746)

46917ff

Signed-off-by: Emelia Lei <wlei29@bloomberg.net>

emelialei88 added a commit that referenced this pull request Aug 11, 2025

Feat[MQB]: add authentication with basic logic (#746)

13d92ca

Signed-off-by: Emelia Lei <wlei29@bloomberg.net>

emelialei88 added a commit that referenced this pull request Aug 15, 2025

Feat[MQB]: add authentication with basic logic (#746)

61ef11e

Signed-off-by: Emelia Lei <wlei29@bloomberg.net>

emelialei88 added a commit that referenced this pull request Aug 22, 2025

Feat[MQB]: add authentication with basic logic (#746)

68c5a93

Signed-off-by: Emelia Lei <wlei29@bloomberg.net>

emelialei88 added a commit that referenced this pull request Aug 29, 2025

Feat[MQB]: add authentication with basic logic (#746)

65c3c6b

Signed-off-by: Emelia Lei <wlei29@bloomberg.net>

emelialei88 added a commit that referenced this pull request Sep 8, 2025

Feat[MQB]: add authentication with basic logic (#746)

124b308

Signed-off-by: Emelia Lei <wlei29@bloomberg.net>

emelialei88 added a commit that referenced this pull request Sep 11, 2025

Feat[MQB]: add authentication with basic logic (#746)

7c0c099

Signed-off-by: Emelia Lei <wlei29@bloomberg.net>

emelialei88 added a commit that referenced this pull request Sep 22, 2025

Feat[MQB]: add authentication with basic logic (#746)

02dce13

Signed-off-by: Emelia Lei <wlei29@bloomberg.net>

emelialei88 added a commit that referenced this pull request Sep 23, 2025

Feat[MQB]: add authentication with basic logic (#746)

eaa1b2e

Signed-off-by: Emelia Lei <wlei29@bloomberg.net>

emelialei88 added a commit that referenced this pull request Oct 3, 2025

Feat[MQB]: add authentication with basic logic (#746)

7e5823c

Signed-off-by: Emelia Lei <wlei29@bloomberg.net>

Feat[MQB]: add authentication with basic logic #746

Feat[MQB]: add authentication with basic logic #746

Uh oh!

Conversation

emelialei88 commented May 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

emelialei88 May 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dorjesinpo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

emelialei88 May 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dorjesinpo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pniedzielski left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

emelialei88 commented May 13, 2025 •

edited

Loading

emelialei88 May 16, 2025 •

edited

Loading

emelialei88 May 16, 2025 •

edited

Loading