mhaeusler commented on code in PR #438:
URL: https://github.com/apache/ws-wss4j/pull/438#discussion_r1934395804


##########
ws-security-dom/src/main/java/org/apache/wss4j/dom/message/WSSecEncryptedKey.java:
##########
@@ -277,127 +277,144 @@ protected void 
createEncryptedKeyElement(X509Certificate remoteCert, Crypto cryp
         if (customEKKeyInfoElement != null) {
             
encryptedKeyElement.appendChild(getDocument().adoptNode(customEKKeyInfoElement));
         } else {
+            Element keyInfoElement =
+                    getDocument().createElementNS(
+                            WSConstants.SIG_NS, WSConstants.SIG_PREFIX + ":" + 
WSConstants.KEYINFO_LN
+                    );
+            keyInfoElement.setAttributeNS(
+                    WSConstants.XMLNS_NS, "xmlns:" + WSConstants.SIG_PREFIX, 
WSConstants.SIG_NS
+            );
+
+            Element keyInfoChildElement;
+
             SecurityTokenReference secToken = new 
SecurityTokenReference(getDocument());
             if (addWSUNamespace) {
                 secToken.addWSUNamespace();
             }
 
             switch (keyIdentifierType) {
-            case WSConstants.X509_KEY_IDENTIFIER:
-                secToken.setKeyIdentifier(remoteCert);
-                break;
+                case WSConstants.X509_KEY_IDENTIFIER:
+                    secToken.setKeyIdentifier(remoteCert);
+                    keyInfoChildElement = secToken.getElement();
+                    break;
 
-            case WSConstants.SKI_KEY_IDENTIFIER:
-                secToken.setKeyIdentifierSKI(remoteCert, crypto);
+                case WSConstants.SKI_KEY_IDENTIFIER:
+                    secToken.setKeyIdentifierSKI(remoteCert, crypto);
 
-                if (includeEncryptionToken) {
-                    addBST(remoteCert);
-                }
-                break;
-
-            case WSConstants.THUMBPRINT_IDENTIFIER:
-            case WSConstants.ENCRYPTED_KEY_SHA1_IDENTIFIER:
-                //
-                // This identifier is not applicable for this case, so fall 
back to
-                // ThumbprintRSA.
-                //
-                secToken.setKeyIdentifierThumb(remoteCert);
-
-                if (includeEncryptionToken) {
-                    addBST(remoteCert);
-                }
-                break;
-
-            case WSConstants.ISSUER_SERIAL:
-                addIssuerSerial(remoteCert, secToken, false);
-                break;
-
-            case WSConstants.ISSUER_SERIAL_QUOTE_FORMAT:
-                addIssuerSerial(remoteCert, secToken,true);
-                break;
-
-            case WSConstants.BST_DIRECT_REFERENCE:
-                Reference ref = new Reference(getDocument());
-                String certUri = IDGenerator.generateID(null);
-                ref.setURI("#" + certUri);
-                bstToken = new X509Security(getDocument());
-                ((X509Security) bstToken).setX509Certificate(remoteCert);
-                bstToken.setID(certUri);
-                ref.setValueType(bstToken.getValueType());
-                secToken.setReference(ref);
-                break;
-
-            case WSConstants.CUSTOM_SYMM_SIGNING :
-                Reference refCust = new Reference(getDocument());
-                if 
(WSConstants.WSS_SAML_KI_VALUE_TYPE.equals(customEKTokenValueType)) {
-                    secToken.addTokenType(WSConstants.WSS_SAML_TOKEN_TYPE);
-                    refCust.setValueType(customEKTokenValueType);
-                } else if 
(WSConstants.WSS_SAML2_KI_VALUE_TYPE.equals(customEKTokenValueType)) {
-                    secToken.addTokenType(WSConstants.WSS_SAML2_TOKEN_TYPE);
-                } else if 
(WSConstants.WSS_ENC_KEY_VALUE_TYPE.equals(customEKTokenValueType)) {
-                    secToken.addTokenType(WSConstants.WSS_ENC_KEY_VALUE_TYPE);
-                    refCust.setValueType(customEKTokenValueType);
-                } else {
-                    refCust.setValueType(customEKTokenValueType);
-                }
-                refCust.setURI("#" + customEKTokenId);
-                secToken.setReference(refCust);
-                break;
-
-            case WSConstants.CUSTOM_SYMM_SIGNING_DIRECT :
-                Reference refCustd = new Reference(getDocument());
-                if 
(WSConstants.WSS_SAML_KI_VALUE_TYPE.equals(customEKTokenValueType)) {
-                    secToken.addTokenType(WSConstants.WSS_SAML_TOKEN_TYPE);
-                    refCustd.setValueType(customEKTokenValueType);
-                } else if 
(WSConstants.WSS_SAML2_KI_VALUE_TYPE.equals(customEKTokenValueType)) {
-                    secToken.addTokenType(WSConstants.WSS_SAML2_TOKEN_TYPE);
-                } else if 
(WSConstants.WSS_ENC_KEY_VALUE_TYPE.equals(customEKTokenValueType)) {
-                    secToken.addTokenType(WSConstants.WSS_ENC_KEY_VALUE_TYPE);
-                    refCustd.setValueType(customEKTokenValueType);
-                } else {
-                    refCustd.setValueType(customEKTokenValueType);
-                }
-                refCustd.setURI(customEKTokenId);
-                secToken.setReference(refCustd);
-                break;
-
-            case WSConstants.CUSTOM_KEY_IDENTIFIER:
-                secToken.setKeyIdentifier(customEKTokenValueType, 
customEKTokenId);
-                if 
(WSConstants.WSS_SAML_KI_VALUE_TYPE.equals(customEKTokenValueType)) {
-                    secToken.addTokenType(WSConstants.WSS_SAML_TOKEN_TYPE);
-                } else if 
(WSConstants.WSS_SAML2_KI_VALUE_TYPE.equals(customEKTokenValueType)) {
-                    secToken.addTokenType(WSConstants.WSS_SAML2_TOKEN_TYPE);
-                } else if 
(WSConstants.WSS_ENC_KEY_VALUE_TYPE.equals(customEKTokenValueType)) {
-                    secToken.addTokenType(WSConstants.WSS_ENC_KEY_VALUE_TYPE);
-                } else if 
(SecurityTokenReference.ENC_KEY_SHA1_URI.equals(customEKTokenValueType)) {
-                    secToken.addTokenType(WSConstants.WSS_ENC_KEY_VALUE_TYPE);
-                }
-                break;
+                    if (includeEncryptionToken) {
+                        addBST(remoteCert);
+                    }
+                    keyInfoChildElement = secToken.getElement();
+                    break;
 
-            default:
-                throw new 
WSSecurityException(WSSecurityException.ErrorCode.FAILURE, "unsupportedKeyId",
-                                              new Object[] 
{keyIdentifierType});
+                case WSConstants.THUMBPRINT_IDENTIFIER:
+                case WSConstants.ENCRYPTED_KEY_SHA1_IDENTIFIER:
+                    //
+                    // This identifier is not applicable for this case, so 
fall back to
+                    // ThumbprintRSA.
+                    //
+                    secToken.setKeyIdentifierThumb(remoteCert);
+
+                    if (includeEncryptionToken) {
+                        addBST(remoteCert);
+                    }
+                    keyInfoChildElement = secToken.getElement();
+                    break;
+
+                case WSConstants.ISSUER_SERIAL:
+                    addIssuerSerial(remoteCert, secToken, false);
+                    keyInfoChildElement = secToken.getElement();
+                    break;
+
+                case WSConstants.ISSUER_SERIAL_QUOTE_FORMAT:
+                    addIssuerSerial(remoteCert, secToken,true);
+                    keyInfoChildElement = secToken.getElement();
+                    break;
+
+                case WSConstants.BST_DIRECT_REFERENCE:
+                    Reference ref = new Reference(getDocument());
+                    String certUri = IDGenerator.generateID(null);
+                    ref.setURI("#" + certUri);
+                    bstToken = new X509Security(getDocument());
+                    ((X509Security) bstToken).setX509Certificate(remoteCert);
+                    bstToken.setID(certUri);
+                    ref.setValueType(bstToken.getValueType());
+                    secToken.setReference(ref);
+                    keyInfoChildElement = secToken.getElement();
+                    break;
+
+                case WSConstants.CUSTOM_SYMM_SIGNING :
+                    Reference refCust = new Reference(getDocument());
+                    if 
(WSConstants.WSS_SAML_KI_VALUE_TYPE.equals(customEKTokenValueType)) {
+                        secToken.addTokenType(WSConstants.WSS_SAML_TOKEN_TYPE);
+                        refCust.setValueType(customEKTokenValueType);
+                    } else if 
(WSConstants.WSS_SAML2_KI_VALUE_TYPE.equals(customEKTokenValueType)) {
+                        
secToken.addTokenType(WSConstants.WSS_SAML2_TOKEN_TYPE);
+                    } else if 
(WSConstants.WSS_ENC_KEY_VALUE_TYPE.equals(customEKTokenValueType)) {
+                        
secToken.addTokenType(WSConstants.WSS_ENC_KEY_VALUE_TYPE);
+                        refCust.setValueType(customEKTokenValueType);
+                    } else {
+                        refCust.setValueType(customEKTokenValueType);
+                    }
+                    refCust.setURI("#" + customEKTokenId);
+                    secToken.setReference(refCust);
+                    keyInfoChildElement = secToken.getElement();
+                    break;
+
+                case WSConstants.CUSTOM_SYMM_SIGNING_DIRECT :
+                    Reference refCustd = new Reference(getDocument());
+                    if 
(WSConstants.WSS_SAML_KI_VALUE_TYPE.equals(customEKTokenValueType)) {
+                        secToken.addTokenType(WSConstants.WSS_SAML_TOKEN_TYPE);
+                        refCustd.setValueType(customEKTokenValueType);
+                    } else if 
(WSConstants.WSS_SAML2_KI_VALUE_TYPE.equals(customEKTokenValueType)) {
+                        
secToken.addTokenType(WSConstants.WSS_SAML2_TOKEN_TYPE);
+                    } else if 
(WSConstants.WSS_ENC_KEY_VALUE_TYPE.equals(customEKTokenValueType)) {
+                        
secToken.addTokenType(WSConstants.WSS_ENC_KEY_VALUE_TYPE);
+                        refCustd.setValueType(customEKTokenValueType);
+                    } else {
+                        refCustd.setValueType(customEKTokenValueType);
+                    }
+                    refCustd.setURI(customEKTokenId);
+                    secToken.setReference(refCustd);
+                    keyInfoChildElement = secToken.getElement();
+                    break;
+
+                case WSConstants.CUSTOM_KEY_IDENTIFIER:
+                    secToken.setKeyIdentifier(customEKTokenValueType, 
customEKTokenId);
+                    if 
(WSConstants.WSS_SAML_KI_VALUE_TYPE.equals(customEKTokenValueType)) {
+                        secToken.addTokenType(WSConstants.WSS_SAML_TOKEN_TYPE);
+                    } else if 
(WSConstants.WSS_SAML2_KI_VALUE_TYPE.equals(customEKTokenValueType)) {
+                        
secToken.addTokenType(WSConstants.WSS_SAML2_TOKEN_TYPE);
+                    } else if 
(WSConstants.WSS_ENC_KEY_VALUE_TYPE.equals(customEKTokenValueType)) {
+                        
secToken.addTokenType(WSConstants.WSS_ENC_KEY_VALUE_TYPE);
+                    } else if 
(SecurityTokenReference.ENC_KEY_SHA1_URI.equals(customEKTokenValueType)) {
+                        
secToken.addTokenType(WSConstants.WSS_ENC_KEY_VALUE_TYPE);
+                    }
+                    keyInfoChildElement = secToken.getElement();
+                    break;
+                case WSConstants.X509_SKI:

Review Comment:
   I think it makes more sense to have a flag that decides if secTokens should 
be created at all, and handle that case accordingly for all possible 
keyIdentifierTypes, not just the SKI one.
   
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@ws.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@ws.apache.org
For additional commands, e-mail: dev-h...@ws.apache.org

Reply via email to