mirror of
				https://github.com/apache/cloudstack.git
				synced 2025-10-26 08:42:29 +01:00 
			
		
		
		
	api: client verification in servlet
This introduces new global settings to handle how client address checks
are handled by the API layer:
proxy.header.verify: enables/disables checking of ipaddresses from a
                     proxy set header
proxy.header.names: a list of names to check for allowed ipaddresses
                    from a proxy set header.
proxy.cidr: a list of cidrs for which \"proxy.header.names\" are
            honoured if the \"Remote_Addr\" is in this list.
(cherry picked from commit b65546636d84a5790e0297b1b0ca8e5a67a48dbc)
Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
			
			
This commit is contained in:
		
							parent
							
								
									72b2eb0087
								
							
						
					
					
						commit
						00f687db1b
					
				| @ -31,7 +31,6 @@ import com.cloud.utils.server.ServerProperties; | ||||
| import org.apache.commons.daemon.Daemon; | ||||
| import org.apache.commons.daemon.DaemonContext; | ||||
| import org.eclipse.jetty.jmx.MBeanContainer; | ||||
| import org.eclipse.jetty.server.ForwardedRequestCustomizer; | ||||
| import org.eclipse.jetty.server.HttpConfiguration; | ||||
| import org.eclipse.jetty.server.HttpConnectionFactory; | ||||
| import org.eclipse.jetty.server.NCSARequestLog; | ||||
| @ -167,7 +166,7 @@ public class ServerDaemon implements Daemon { | ||||
| 
 | ||||
|         // HTTP config | ||||
|         final HttpConfiguration httpConfig = new HttpConfiguration(); | ||||
|         httpConfig.addCustomizer( new ForwardedRequestCustomizer() ); | ||||
| // it would be nice to make this dynamic but we take care of this ourselves for now: httpConfig.addCustomizer( new ForwardedRequestCustomizer() ); | ||||
|         httpConfig.setSecureScheme("https"); | ||||
|         httpConfig.setSecurePort(httpsPort); | ||||
|         httpConfig.setOutputBufferSize(32768); | ||||
|  | ||||
| @ -34,6 +34,7 @@ public class ConfigKey<T> { | ||||
| 
 | ||||
|     public static final String CATEGORY_ADVANCED = "Advanced"; | ||||
|     public static final String CATEGORY_ALERT = "Alert"; | ||||
|     public static final String CATEGORY_NETWORK = "Network"; | ||||
| 
 | ||||
|     public enum Scope { | ||||
|         Global, Zone, Cluster, StoragePool, Account, ManagementServer, ImageStore, Domain | ||||
|  | ||||
| @ -234,42 +234,42 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer | ||||
|     @Inject | ||||
|     private MessageBus messageBus; | ||||
| 
 | ||||
|     private static final ConfigKey<Integer> IntegrationAPIPort = new ConfigKey<Integer>("Advanced" | ||||
|     private static final ConfigKey<Integer> IntegrationAPIPort = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED | ||||
|             , Integer.class | ||||
|             , "integration.api.port" | ||||
|             , "0" | ||||
|             , "Integration (unauthenticated) API port. To disable set it to 0 or negative." | ||||
|             , false | ||||
|             , ConfigKey.Scope.Global); | ||||
|     private static final ConfigKey<Long> ConcurrentSnapshotsThresholdPerHost = new ConfigKey<Long>("Advanced" | ||||
|     private static final ConfigKey<Long> ConcurrentSnapshotsThresholdPerHost = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED | ||||
|             , Long.class | ||||
|             , "concurrent.snapshots.threshold.perhost" | ||||
|             , null | ||||
|             , "Limits number of snapshots that can be handled by the host concurrently; default is NULL - unlimited" | ||||
|             , true // not sure if this is to be dynamic | ||||
|             , ConfigKey.Scope.Global); | ||||
|     private static final ConfigKey<Boolean> EncodeApiResponse = new ConfigKey<Boolean>("Advanced" | ||||
|     private static final ConfigKey<Boolean> EncodeApiResponse = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED | ||||
|             , Boolean.class | ||||
|             , "encode.api.response" | ||||
|             , "false" | ||||
|             , "Do URL encoding for the api response, false by default" | ||||
|             , false | ||||
|             , ConfigKey.Scope.Global); | ||||
|     static final ConfigKey<String> JSONcontentType = new ConfigKey<String>( "Advanced" | ||||
|     static final ConfigKey<String> JSONcontentType = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED | ||||
|             , String.class | ||||
|             , "json.content.type" | ||||
|             , "application/json; charset=UTF-8" | ||||
|             , "Http response content type for .js files (default is text/javascript)" | ||||
|             , false | ||||
|             , ConfigKey.Scope.Global); | ||||
|     static final ConfigKey<Boolean> EnableSecureSessionCookie = new ConfigKey<Boolean>("Advanced" | ||||
|     static final ConfigKey<Boolean> EnableSecureSessionCookie = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED | ||||
|             , Boolean.class | ||||
|             , "enable.secure.session.cookie" | ||||
|             , "false" | ||||
|             , "Session cookie is marked as secure if this is enabled. Secure cookies only work when HTTPS is used." | ||||
|             , false | ||||
|             , ConfigKey.Scope.Global); | ||||
|     private static final ConfigKey<String> JSONDefaultContentType = new ConfigKey<String> ("Advanced" | ||||
|     private static final ConfigKey<String> JSONDefaultContentType = new ConfigKey<> (ConfigKey.CATEGORY_ADVANCED | ||||
|             , String.class | ||||
|             , "json.content.type" | ||||
|             , "application/json; charset=UTF-8" | ||||
| @ -277,13 +277,34 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer | ||||
|             , false | ||||
|             , ConfigKey.Scope.Global); | ||||
| 
 | ||||
|     private static final ConfigKey<Boolean> UseEventAccountInfo = new ConfigKey<Boolean>( "advanced" | ||||
|     private static final ConfigKey<Boolean> UseEventAccountInfo = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED | ||||
|             , Boolean.class | ||||
|             , "event.accountinfo" | ||||
|             , "false" | ||||
|             , "use account info in event logging" | ||||
|             , true | ||||
|             , ConfigKey.Scope.Global); | ||||
|     static final ConfigKey<Boolean> useForwardHeader = new ConfigKey<>(ConfigKey.CATEGORY_NETWORK | ||||
|             , Boolean.class | ||||
|             , "proxy.header.verify" | ||||
|             , "false" | ||||
|             , "enables/disables checking of ipaddresses from a proxy set header. See \"proxy.header.names\" for the headers to allow." | ||||
|             , true | ||||
|             , ConfigKey.Scope.Global); | ||||
|     static final ConfigKey<String> listOfForwardHeaders = new ConfigKey<>(ConfigKey.CATEGORY_NETWORK | ||||
|             , String.class | ||||
|             , "proxy.header.names" | ||||
|             , "X-Forwarded-For,HTTP_CLIENT_IP,HTTP_X_FORWARDED_FOR" | ||||
|             , "a list of names to check for allowed ipaddresses from a proxy set header. See \"proxy.cidr\" for the proxies allowed to set these headers." | ||||
|             , true | ||||
|             , ConfigKey.Scope.Global); | ||||
|     static final ConfigKey<String> proxyForwardList = new ConfigKey<>(ConfigKey.CATEGORY_NETWORK | ||||
|             , String.class | ||||
|             , "proxy.cidr" | ||||
|             , "" | ||||
|             , "a list of cidrs for which \"proxy.header.names\" are honoured if the \"Remote_Addr\" is in this list." | ||||
|             , true | ||||
|             , ConfigKey.Scope.Global); | ||||
| 
 | ||||
|     @Override | ||||
|     public boolean configure(final String name, final Map<String, Object> params) throws ConfigurationException { | ||||
| @ -1499,7 +1520,10 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer | ||||
|                 ConcurrentSnapshotsThresholdPerHost, | ||||
|                 EncodeApiResponse, | ||||
|                 EnableSecureSessionCookie, | ||||
|                 JSONDefaultContentType | ||||
|                 JSONDefaultContentType, | ||||
|                 proxyForwardList, | ||||
|                 useForwardHeader, | ||||
|                 listOfForwardHeaders | ||||
|         }; | ||||
|     } | ||||
| } | ||||
|  | ||||
| @ -21,7 +21,6 @@ import java.net.InetAddress; | ||||
| import java.net.URLDecoder; | ||||
| import java.net.UnknownHostException; | ||||
| import java.util.Arrays; | ||||
| import java.util.Collections; | ||||
| import java.util.HashMap; | ||||
| import java.util.List; | ||||
| import java.util.Map; | ||||
| @ -69,13 +68,9 @@ import com.cloud.utils.db.EntityManager; | ||||
| import com.cloud.utils.net.NetUtils; | ||||
| 
 | ||||
| @Component("apiServlet") | ||||
| @SuppressWarnings("serial") | ||||
| public class ApiServlet extends HttpServlet { | ||||
|     public static final Logger s_logger = Logger.getLogger(ApiServlet.class.getName()); | ||||
|     private static final Logger s_accessLogger = Logger.getLogger("apiserver." + ApiServlet.class.getName()); | ||||
|     private final static List<String> s_clientAddressHeaders = Collections | ||||
|             .unmodifiableList(Arrays.asList("X-Forwarded-For", | ||||
|                     "HTTP_CLIENT_IP", "HTTP_X_FORWARDED_FOR", "Remote_Addr")); | ||||
|     private static final String REPLACEMENT = "_"; | ||||
|     private static final String LOG_REPLACEMENTS = "[\n\r\t]"; | ||||
| 
 | ||||
| @ -570,17 +565,39 @@ public class ApiServlet extends HttpServlet { | ||||
|         } | ||||
|         return false; | ||||
|     } | ||||
|     boolean doUseForwardHeaders() { | ||||
|         return Boolean.TRUE.equals(ApiServer.useForwardHeader.value()); | ||||
|     } | ||||
| 
 | ||||
|     String[] proxyNets() { | ||||
|         return ApiServer.proxyForwardList.value().split(","); | ||||
|     } | ||||
|     //This method will try to get login IP of user even if servlet is behind reverseProxy or loadBalancer | ||||
|     public static InetAddress getClientAddress(final HttpServletRequest request) throws UnknownHostException { | ||||
|         for(final String header : s_clientAddressHeaders) { | ||||
|             final String ip = getCorrectIPAddress(request.getHeader(header)); | ||||
|             if (ip != null) { | ||||
|     public InetAddress getClientAddress(final HttpServletRequest request) throws UnknownHostException { | ||||
|         String ip = null; | ||||
|         InetAddress pretender = InetAddress.getByName(request.getRemoteAddr()); | ||||
|         if(doUseForwardHeaders()) { | ||||
|             if (NetUtils.isIpInCidrList(pretender, proxyNets())) { | ||||
|                 for (String header : getClientAddressHeaders()) { | ||||
|                     header = header.trim(); | ||||
|                     ip = getCorrectIPAddress(request.getHeader(header)); | ||||
|                     if (StringUtils.isNotBlank(ip)) { | ||||
|                         s_logger.debug(String.format("found ip %s in header %s ", ip, header)); | ||||
|                         break; | ||||
|                     } | ||||
|                 } // no address found in header so ip is blank and use remote addr | ||||
|             } // else not an allowed proxy address, ip is blank and use remote addr | ||||
|         } | ||||
|         if (StringUtils.isBlank(ip)) { | ||||
|             s_logger.trace(String.format("no ip found in headers, returning remote address %s.", pretender.getHostAddress())); | ||||
|             return pretender; | ||||
|         } | ||||
| 
 | ||||
|         return InetAddress.getByName(ip); | ||||
|     } | ||||
|         } | ||||
| 
 | ||||
|         return InetAddress.getByName(request.getRemoteAddr()); | ||||
|     private String[] getClientAddressHeaders() { | ||||
|         return ApiServer.listOfForwardHeaders.value().split(","); | ||||
|     } | ||||
| 
 | ||||
|     private static String getCorrectIPAddress(String ip) { | ||||
|  | ||||
| @ -99,15 +99,17 @@ public class ApiServletTest { | ||||
|     @Mock | ||||
|     AccountService accountMgr; | ||||
| 
 | ||||
|     @Mock ConfigKey<Boolean> useForwardHeader; | ||||
|     StringWriter responseWriter; | ||||
| 
 | ||||
|     ApiServlet servlet; | ||||
| 
 | ||||
|     ApiServlet spyServlet; | ||||
|     @SuppressWarnings("unchecked") | ||||
|     @Before | ||||
|     public void setup() throws SecurityException, NoSuchFieldException, | ||||
|     IllegalArgumentException, IllegalAccessException, IOException, UnknownHostException { | ||||
|         servlet = new ApiServlet(); | ||||
|         spyServlet = Mockito.spy(servlet); | ||||
|         responseWriter = new StringWriter(); | ||||
|         Mockito.when(response.getWriter()).thenReturn( | ||||
|                 new PrintWriter(responseWriter)); | ||||
| @ -261,32 +263,43 @@ public class ApiServletTest { | ||||
| 
 | ||||
|     @Test | ||||
|     public void getClientAddressWithXForwardedFor() throws UnknownHostException { | ||||
|         String[] proxynet = {"127.0.0.0/8"}; | ||||
|         Mockito.when(spyServlet.proxyNets()).thenReturn(proxynet); | ||||
|         Mockito.when(spyServlet.doUseForwardHeaders()).thenReturn(true); | ||||
|         Mockito.when(request.getHeader(Mockito.eq("X-Forwarded-For"))).thenReturn("192.168.1.1"); | ||||
|         Assert.assertEquals(InetAddress.getByName("192.168.1.1"), ApiServlet.getClientAddress(request)); | ||||
|         Assert.assertEquals(InetAddress.getByName("192.168.1.1"), spyServlet.getClientAddress(request)); | ||||
|     } | ||||
| 
 | ||||
|     @Test | ||||
|     public void getClientAddressWithHttpXForwardedFor() throws UnknownHostException { | ||||
|         String[] proxynet = {"127.0.0.0/8"}; | ||||
|         Mockito.when(spyServlet.proxyNets()).thenReturn(proxynet); | ||||
|         Mockito.when(spyServlet.doUseForwardHeaders()).thenReturn(true); | ||||
|         Mockito.when(request.getHeader(Mockito.eq("HTTP_X_FORWARDED_FOR"))).thenReturn("192.168.1.1"); | ||||
|         Assert.assertEquals(InetAddress.getByName("192.168.1.1"), ApiServlet.getClientAddress(request)); | ||||
|         Assert.assertEquals(InetAddress.getByName("192.168.1.1"), spyServlet.getClientAddress(request)); | ||||
|     } | ||||
| 
 | ||||
|     @Test | ||||
|     public void getClientAddressWithXRemoteAddr() throws UnknownHostException { | ||||
|         Mockito.when(request.getHeader(Mockito.eq("Remote_Addr"))).thenReturn("192.168.1.1"); | ||||
|         Assert.assertEquals(InetAddress.getByName("192.168.1.1"), ApiServlet.getClientAddress(request)); | ||||
|     public void getClientAddressWithRemoteAddr() throws UnknownHostException { | ||||
|         String[] proxynet = {"127.0.0.0/8"}; | ||||
|         Mockito.when(spyServlet.proxyNets()).thenReturn(proxynet); | ||||
|         Mockito.when(spyServlet.doUseForwardHeaders()).thenReturn(true); | ||||
|         Assert.assertEquals(InetAddress.getByName("127.0.0.1"), spyServlet.getClientAddress(request)); | ||||
|     } | ||||
| 
 | ||||
|     @Test | ||||
|     public void getClientAddressWithHttpClientIp() throws UnknownHostException { | ||||
|         String[] proxynet = {"127.0.0.0/8"}; | ||||
|         Mockito.when(spyServlet.proxyNets()).thenReturn(proxynet); | ||||
|         Mockito.when(spyServlet.doUseForwardHeaders()).thenReturn(true); | ||||
|         Mockito.when(request.getHeader(Mockito.eq("HTTP_CLIENT_IP"))).thenReturn("192.168.1.1"); | ||||
|         Assert.assertEquals(InetAddress.getByName("192.168.1.1"), ApiServlet.getClientAddress(request)); | ||||
|         Assert.assertEquals(InetAddress.getByName("192.168.1.1"), spyServlet.getClientAddress(request)); | ||||
|     } | ||||
| 
 | ||||
|     @Test | ||||
|     public void getClientAddressDefault() throws UnknownHostException { | ||||
|         Mockito.when(request.getRemoteAddr()).thenReturn("127.0.0.1"); | ||||
|         Assert.assertEquals(InetAddress.getByName("127.0.0.1"), ApiServlet.getClientAddress(request)); | ||||
|         Assert.assertEquals(InetAddress.getByName("127.0.0.1"), spyServlet.getClientAddress(request)); | ||||
|     } | ||||
| 
 | ||||
|     @Test | ||||
|  | ||||
| @ -27,7 +27,7 @@ import java.util.Map; | ||||
| import java.util.regex.Matcher; | ||||
| import java.util.regex.Pattern; | ||||
| 
 | ||||
| public class StringUtils { | ||||
| public class StringUtils extends org.apache.commons.lang3.StringUtils { | ||||
|     private static final char[] hexChar = {'0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'A', 'B', 'C', 'D', 'E', 'F'}; | ||||
| 
 | ||||
|     private static Charset preferredACSCharset; | ||||
|  | ||||
		Loading…
	
	
			
			x
			
			
		
	
		Reference in New Issue
	
	Block a user