Github user gemmellr commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1961#discussion_r176719439
  
    --- Diff: 
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/broker/AMQPConnectionCallback.java
 ---
    @@ -113,7 +116,20 @@ public ServerSASL getServerSASL(final String 
mechanism) {
                    result = gssapiServerSASL;
                    break;
     
    +            case ExternalServerSASL.NAME:
    +               // validate ssl cert present
    +               Principal principal = 
CertificateUtil.getPeerPrincipalFromConnection(protonConnectionDelegate);
    +               if (principal != null) {
    +                  ExternalServerSASL externalServerSASL = new 
ExternalServerSASL();
    +                  externalServerSASL.setPrincipal(principal);
    +                  result = externalServerSASL;
    +               } else {
    +                  logger.debug("SASL EXTERNAL mechanism requires a TLS 
peer principal");
    --- End diff --
    
    This feels like it should be an exception rather than just a log message. 
Things shouldn't get here if the connection cant actually do EXTERNAL, since 
the server shouldn't offer it in that case (as it can tell before offering that 
it cant work), and should fail before here if the client selected it when it 
wasn't actually offered.


---

Reply via email to