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

    https://github.com/apache/activemq-artemis/pull/1961#discussion_r176769932
  
    --- 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 --
    
    I noticed that it fails eventually when returning null, though without 
indication why unless the debug logging is on, but the main thing is it 
shouldn't get to this bit of code without being able to succeed, except through 
malicious intent on a clients part. EXTERNAL can be supported by the broker but 
still not be used/usable by all clients, so it really shouldn't be offered to 
those that cant actually do it.


---

Reply via email to