Uploaded image for project: 'Keycloak'
  1. Keycloak
  2. KEYCLOAK-5644

Advice tag not recognized/supported in SAML2 responses

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Closed (View Workflow)
    • Priority: Major
    • Resolution: Done
    • Affects Version/s: 3.2.1.Final
    • Fix Version/s: 3.4.2.Final
    • Component/s: SAML
    • Labels:
      None
    • Docs QE Status:
      NEW
    • QE Status:
      VERIFIED

      Description

      Keycloak 3.2.1 brokering to a Shibboleth via saml2. Please note that the Shibboleth act as a proxy to another idp (maybe another Shibboleth, i dont' know). Due to this (afaik) saml2 response returned to keycloak has the "Advice" tag compiled inside "Assertion" tag.

      This is not supported in kc 3.2.1 that fails on response parsing.
      I've temporary solved patching class SAMLAssertionParser in saml-core module. The parse method was extended to accept (consume and do nothing) the tag "Advice".

      Btw this patch enforcing me to disable response sign check to make it work. This is odd.
      Please give me a solution.

          /**
           * @see {@link ParserNamespaceSupport#parse(XMLEventReader)}
           */
          public Object parse(XMLEventReader xmlEventReader) throws ParsingException {
              StartElement startElement = StaxParserUtil.peekNextStartElement(xmlEventReader);
              String startElementName = StaxParserUtil.getStartElementName(startElement);
              if (startElementName.equals(JBossSAMLConstants.ENCRYPTED_ASSERTION.get())) {
                  Element domElement = StaxParserUtil.getDOMElement(xmlEventReader);
      
                  EncryptedAssertionType encryptedAssertion = new EncryptedAssertionType();
                  encryptedAssertion.setEncryptedElement(domElement);
                  return encryptedAssertion;
              }
      
              startElement = StaxParserUtil.getNextStartElement(xmlEventReader);
      
              // Special case: Encrypted Assertion
              StaxParserUtil.validate(startElement, ASSERTION);
              AssertionType assertion = parseBaseAttributes(startElement);
      
              // Peek at the next event
              while (xmlEventReader.hasNext()) {
                  XMLEvent xmlEvent = StaxParserUtil.peek(xmlEventReader);
                  if (xmlEvent == null)
                      break;
      
                  if (xmlEvent instanceof EndElement) {
                      xmlEvent = StaxParserUtil.getNextEvent(xmlEventReader);
                      EndElement endElement = (EndElement) xmlEvent;
                      String endElementTag = StaxParserUtil.getEndElementName(endElement);
                      if (endElementTag.equals(JBossSAMLConstants.ASSERTION.get()))
                          break;
                      else
                          throw new RuntimeException(ErrorCodes.UNKNOWN_END_ELEMENT + endElementTag);
                  }
      
                  StartElement peekedElement = null;
      
                  if (xmlEvent instanceof StartElement) {
                      peekedElement = (StartElement) xmlEvent;
                  } else {
                      peekedElement = StaxParserUtil.peekNextStartElement(xmlEventReader);
                  }
                  if (peekedElement == null)
                      break;
      
                  String tag = StaxParserUtil.getStartElementName(peekedElement);
      
                  if (tag.equals(JBossSAMLConstants.SIGNATURE.get())) {
                      assertion.setSignature(StaxParserUtil.getDOMElement(xmlEventReader));
                      continue;
                  }
      
                  if (JBossSAMLConstants.ISSUER.get().equalsIgnoreCase(tag)) {
                      startElement = StaxParserUtil.getNextStartElement(xmlEventReader);
                      String issuerValue = StaxParserUtil.getElementText(xmlEventReader);
                      NameIDType issuer = new NameIDType();
                      issuer.setValue(issuerValue);
      
                      assertion.setIssuer(issuer);
                  } else if (JBossSAMLConstants.SUBJECT.get().equalsIgnoreCase(tag)) {
                      SAMLSubjectParser subjectParser = new SAMLSubjectParser();
                      assertion.setSubject((SubjectType) subjectParser.parse(xmlEventReader));
                  } else if (JBossSAMLConstants.CONDITIONS.get().equalsIgnoreCase(tag)) {
                      SAMLConditionsParser conditionsParser = new SAMLConditionsParser();
                      ConditionsType conditions = (ConditionsType) conditionsParser.parse(xmlEventReader);
      
                      assertion.setConditions(conditions);
                  } else if (JBossSAMLConstants.AUTHN_STATEMENT.get().equalsIgnoreCase(tag)) {
                      AuthnStatementType authnStatementType = SAMLParserUtil.parseAuthnStatement(xmlEventReader);
                      assertion.addStatement(authnStatementType);
                  } else if (JBossSAMLConstants.ATTRIBUTE_STATEMENT.get().equalsIgnoreCase(tag)) {
                      AttributeStatementType attributeStatementType = SAMLParserUtil.parseAttributeStatement(xmlEventReader);
                      assertion.addStatement(attributeStatementType);
                  } else if (JBossSAMLConstants.STATEMENT.get().equalsIgnoreCase(tag)) {
                      startElement = StaxParserUtil.getNextStartElement(xmlEventReader);
      
                      String xsiTypeValue = StaxParserUtil.getXSITypeValue(startElement);
                      throw new RuntimeException(ErrorCodes.UNKNOWN_XSI + xsiTypeValue);
                  }
                  // PATCH BEGIN - Denis
                  else if ("Advice".equals(tag)) {
                      // Just ignore Advice that just contains forwarding info. Not supported in kc 3.2.1
                      // Consume tag and do nothing. Have to consume all nested until tag closing due to STAX streaming mode
                      while (xmlEventReader.hasNext()) {
                              XMLEvent innerXmlEvent  = StaxParserUtil.peek(xmlEventReader);
                              try {
                                  // On inner end element skip tag. If is </Advice> exit from cycle
                                  if (innerXmlEvent != null && innerXmlEvent.isEndElement()){
                                      EndElement ee=(EndElement)innerXmlEvent;
                                      if (StaxParserUtil.matches(ee,"Advice")) {
                                          xmlEventReader.nextEvent();
                                          break;
                                      }
                                  }
                                  xmlEventReader.nextEvent();
                              } catch (XMLStreamException e) {
                                  throw new RuntimeException(ErrorCodes.PARSING_ERROR + tag + "::location=" + innerXmlEvent.getLocation());
                              }
                      }
                      continue;
                  }
                  // PATCH END - Denis
                  else
                      throw new RuntimeException(ErrorCodes.UNKNOWN_TAG + tag + "::location=" + peekedElement.getLocation());
              }
              return assertion;
          }
      
      ......    
      	<saml2:Assertion ID="s25505f5e858daf1cfea4b53f7576a15fa287bc8"
                           IssueInstant="2017-10-03T11:41:36.747Z"
                           Version="2.0"
                           xmlns:saml2="urn:oasis:names:tc:SAML:2.0:assertion"
                           xmlns:xs="http://www.w3.org/2001/XMLSchema"
                           >
              <saml2:Issuer xmlns:saml2="urn:oasis:names:tc:SAML:2.0:assertion">https://xxxxxxxxxx/xxxxx/metadata</saml2:Issuer>
              <ds:Signature xmlns:ds="http://www.w3.org/2000/09/xmldsig#">
                  .....
              </ds:Signature>
              <saml:Subject xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion">
      			....
              </saml:Subject>
              <saml2:Conditions NotBefore="2017-10-03T11:41:36.747Z"
                                NotOnOrAfter="2017-10-03T11:46:36.747Z"
                                xmlns:saml2="urn:oasis:names:tc:SAML:2.0:assertion"
                                >
                  ....
              </saml2:Conditions>
       *       <saml2:Advice xmlns:saml2="urn:oasis:names:tc:SAML:2.0:assertion">
      			....
              </saml2:Advice>*
              <saml:AuthnStatement AuthnInstant="2017-10-03T11:41:36.000Z"
                                   SessionIndex="nlat5IPYLwMpWdzkKq-NYgxF"
                                   xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion"
                                   >
                  ....
              </saml:AuthnStatement>
              <saml2:AttributeStatement xmlns:saml2="urn:oasis:names:tc:SAML:2.0:assertion">
                  ....
              </saml2:AttributeStatement>
          </saml2:Assertion>
      .....	
      

        Attachments

          Activity

            People

            Assignee:
            hmlnarik Hynek Mlnařík
            Reporter:
            denis.miorandi Denis Miorandi (Inactive)
            Tester:
            Michal Hajas Michal Hajas
            Votes:
            2 Vote for this issue
            Watchers:
            4 Start watching this issue

              Dates

              Created:
              Updated:
              Resolved: