You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@guacamole.apache.org by GitBox <gi...@apache.org> on 2022/08/23 23:37:35 UTC
[GitHub] [guacamole-server] jmuehlner opened a new pull request, #391: GUACAMOLE-1669: Prefer FIPS compliant ciphers and algorithms when FIPS mode is enabled.
jmuehlner opened a new pull request, #391:
URL: https://github.com/apache/guacamole-server/pull/391
As discussed in https://issues.apache.org/jira/browse/GUACAMOLE-1669, this change prefers FIPS-compliant ciphers and algorithms if FIPS mode is enabled on the guacd host.
--
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-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-server] necouchman commented on a diff in pull request #391: GUACAMOLE-1669: Prefer FIPS compliant ciphers and algorithms when FIPS mode is enabled.
Posted by GitBox <gi...@apache.org>.
necouchman commented on code in PR #391:
URL: https://github.com/apache/guacamole-server/pull/391#discussion_r953206985
##########
src/libguac/fips.c:
##########
@@ -0,0 +1,68 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include "config.h"
+#include "guacamole/fips.h"
+
+#include <stdio.h>
+
+#ifdef ENABLE_SSL
+#include <openssl/opensslv.h>
+
+/* OpenSSL versions prior to 0.9.7e did not have FIPS support */
+#if !defined(OPENSSL_VERSION_NUMBER) || (OPENSSL_VERSION_NUMBER < 0x00090705f)
+
+int guac_fips_enabled() {
+
+ return 0;
+
+}
+
+/* OpenSSL 3+ uses EVP_default_properties_is_fips_enabled() */
+#elif defined(OPENSSL_VERSION_MAJOR) && (OPENSSL_VERSION_MAJOR >= 3)
+
+#include <openssl/evp.h>
+
+int guac_fips_enabled() {;
Review Comment:
Is that syntax correct, though, with the `;` at the end of the line?
--
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-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-server] mike-jumper commented on a diff in pull request #391: GUACAMOLE-1669: Prefer FIPS compliant ciphers and algorithms when FIPS mode is enabled.
Posted by GitBox <gi...@apache.org>.
mike-jumper commented on code in PR #391:
URL: https://github.com/apache/guacamole-server/pull/391#discussion_r954335057
##########
src/libguac/guacamole/fips.h:
##########
@@ -0,0 +1,33 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#ifndef GUAC_FIPS_H
+#define GUAC_FIPS_H
+
+/**
+ * Returns a non-zero value if FIPS mode is enabled for OpenSSL, or zero if
+ * FIPS mode is not enabled.
+ *
+ * @return
+ * A non-zero value if FIPS mode is enabled for OpenSSL, or zero if FIPS
+ * mode is not enabled.
Review Comment:
Thoughts on decoupling the API definition of this from OpenSSL, and simply stating that it returns non-zero if the local system has FIPS mode enabled?
--
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-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-server] jmuehlner commented on pull request #391: GUACAMOLE-1669: Prefer FIPS compliant ciphers and algorithms when FIPS mode is enabled.
Posted by GitBox <gi...@apache.org>.
jmuehlner commented on PR #391:
URL: https://github.com/apache/guacamole-server/pull/391#issuecomment-1226032494
Ah, gotcha. Yeah I don't think I see a use case where you'd want to manually be turning this off and on.
> I should clarify that I wasn't necessarily saying that we should allow configuration of individual cipher/key algorithms, just whether FIPS mode is enabled or not. But, I'll go with whatever you guys think on that - it was an honest question, not a leading 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-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-server] jmuehlner commented on a diff in pull request #391: GUACAMOLE-1669: Prefer FIPS compliant ciphers and algorithms when FIPS mode is enabled.
Posted by GitBox <gi...@apache.org>.
jmuehlner commented on code in PR #391:
URL: https://github.com/apache/guacamole-server/pull/391#discussion_r953209448
##########
src/libguac/fips.c:
##########
@@ -0,0 +1,68 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include "config.h"
+#include "guacamole/fips.h"
+
+#include <stdio.h>
+
+#ifdef ENABLE_SSL
+#include <openssl/opensslv.h>
+
+/* OpenSSL versions prior to 0.9.7e did not have FIPS support */
+#if !defined(OPENSSL_VERSION_NUMBER) || (OPENSSL_VERSION_NUMBER < 0x00090705f)
+
+int guac_fips_enabled() {
+
+ return 0;
+
+}
+
+/* OpenSSL 3+ uses EVP_default_properties_is_fips_enabled() */
+#elif defined(OPENSSL_VERSION_MAJOR) && (OPENSSL_VERSION_MAJOR >= 3)
+
+#include <openssl/evp.h>
+
+int guac_fips_enabled() {;
Review Comment:
Definitely not! fixed.
--
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-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-server] jmuehlner commented on pull request #391: GUACAMOLE-1669: Prefer FIPS compliant ciphers and algorithms when FIPS mode is enabled.
Posted by GitBox <gi...@apache.org>.
jmuehlner commented on PR #391:
URL: https://github.com/apache/guacamole-server/pull/391#issuecomment-1225235336
Yeah, I kinda feel like it's probably not worth it. In the general case, we don't really want to be telling libssh2 what it should be using at all - it's job is to negotiate a set of algorithms and ciphers that both server and client can use. Unfortunately it doesn't seem to work in the case of FIPS.
Luckily for FIPS, there's a pretty small set of options that are both FIPS-compliant, AND libssh2-supported. I just listed those from biggest key to smallest. I guess you could imagine somebody wanting to prefer smaller key sizes for performance reasons, but I'd guess that nobody would ever use such an option ¯\_(ツ)_/¯
> Cool, looks okay to me as long as it tests okay. My only other question would be if it's worth making it configurable or not? I'm guessing not - since i'ts just setting an order of preferred cipher and key algorithms, I would guess if it supports it you want it. Just throwing it out, though.
--
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-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-server] necouchman commented on pull request #391: GUACAMOLE-1669: Prefer FIPS compliant ciphers and algorithms when FIPS mode is enabled.
Posted by GitBox <gi...@apache.org>.
necouchman commented on PR #391:
URL: https://github.com/apache/guacamole-server/pull/391#issuecomment-1225590315
I should clarify that I wasn't necessarily saying that we should allow configuration of individual cipher/key algorithms, just whether FIPS mode is enabled or not. But, I'll go with whatever you guys think on that - it was an honest question, not a leading 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-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-server] mike-jumper merged pull request #391: GUACAMOLE-1669: Prefer FIPS compliant ciphers and algorithms when FIPS mode is enabled.
Posted by GitBox <gi...@apache.org>.
mike-jumper merged PR #391:
URL: https://github.com/apache/guacamole-server/pull/391
--
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-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-server] necouchman commented on pull request #391: GUACAMOLE-1669: Prefer FIPS compliant ciphers and algorithms when FIPS mode is enabled.
Posted by GitBox <gi...@apache.org>.
necouchman commented on PR #391:
URL: https://github.com/apache/guacamole-server/pull/391#issuecomment-1225029921
Cool, looks okay to me as long as it tests okay. My only other question would be if it's worth making it configurable or not? I'm guessing not - since i'ts just setting an order of preferred cipher and key algorithms, I would guess if it supports it you want it. Just throwing it out, though.
--
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-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-server] mike-jumper commented on a diff in pull request #391: GUACAMOLE-1669: Prefer FIPS compliant ciphers and algorithms when FIPS mode is enabled.
Posted by GitBox <gi...@apache.org>.
mike-jumper commented on code in PR #391:
URL: https://github.com/apache/guacamole-server/pull/391#discussion_r953408359
##########
src/libguac/fips.c:
##########
@@ -0,0 +1,67 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include "config.h"
+#include "guacamole/fips.h"
+
+#ifdef ENABLE_SSL
+
+#include <openssl/opensslv.h>
+
+/* OpenSSL versions prior to 0.9.7e did not have FIPS support */
+#if !defined(OPENSSL_VERSION_NUMBER) || (OPENSSL_VERSION_NUMBER < 0x00090705f)
+
+int guac_fips_enabled() {
+
+ return 0;
+
+}
+
+/* OpenSSL 3+ uses EVP_default_properties_is_fips_enabled() */
+#elif defined(OPENSSL_VERSION_MAJOR) && (OPENSSL_VERSION_MAJOR >= 3)
+
+#include <openssl/evp.h>
+
+int guac_fips_enabled() {
+
+ return EVP_default_properties_is_fips_enabled(NULL);
+
+}
+
+/* For OpenSSL versions between 0.9.7e and 3.0, use FIPS_mode() */
+#else
Review Comment:
I'm finding this somewhat hard to read, and I think that's in part because the frequency of empty lines makes this effectively double-spaced.
I have some ideas that might clean things up a smidge:
1. Nix the extra spacing within the one-line func:
```c
int foo() {
return 0;
}
```
or ...
2. ... Leverage macros to avoid having to redeclare everything. For example:
```c
#if thing1
#include <foo.h>
#define GUAC_FIPS_ENABLED EVP_default_properties_is_fips_enabled(NULL)
#elif thing2
#include <bar.h>
#define GUAC_FIPS_ENABLED FIPS_mode()
#else
#define GUAC_FIPS_ENABLED 0
#endif
int guac_fips_enabled() {
return GUAC_FIPS_ENABLED;
}
```
##########
src/libguac/fips.c:
##########
@@ -0,0 +1,67 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include "config.h"
+#include "guacamole/fips.h"
+
+#ifdef ENABLE_SSL
+
+#include <openssl/opensslv.h>
+
+/* OpenSSL versions prior to 0.9.7e did not have FIPS support */
+#if !defined(OPENSSL_VERSION_NUMBER) || (OPENSSL_VERSION_NUMBER < 0x00090705f)
+
+int guac_fips_enabled() {
+
+ return 0;
+
+}
+
+/* OpenSSL 3+ uses EVP_default_properties_is_fips_enabled() */
+#elif defined(OPENSSL_VERSION_MAJOR) && (OPENSSL_VERSION_MAJOR >= 3)
+
+#include <openssl/evp.h>
+
+int guac_fips_enabled() {
+
+ return EVP_default_properties_is_fips_enabled(NULL);
Review Comment:
Beware tabs have snuck in.
--
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-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-server] mike-jumper commented on a diff in pull request #391: GUACAMOLE-1669: Prefer FIPS compliant ciphers and algorithms when FIPS mode is enabled.
Posted by GitBox <gi...@apache.org>.
mike-jumper commented on code in PR #391:
URL: https://github.com/apache/guacamole-server/pull/391#discussion_r954023932
##########
src/libguac/guacamole/fips.h:
##########
@@ -0,0 +1,33 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#ifndef GUAC_FIPS_H
+#define GUAC_FIPS_H
+
+/**
+ * Returns a positive value if FIPS mode is enabled for OpenSSL, or zero if
+ * FIPS mode is not enabled.
+ *
+ * @return
+ * A positive value if FIPS mode is enabled for OpenSSL, or zero if FIPS
+ * mode is not enabled.
Review Comment:
Rather than bake into the API that this value must be positive when true, I suggest instead documenting the value as non-zero.
--
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-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-server] mike-jumper commented on pull request #391: GUACAMOLE-1669: Prefer FIPS compliant ciphers and algorithms when FIPS mode is enabled.
Posted by GitBox <gi...@apache.org>.
mike-jumper commented on PR #391:
URL: https://github.com/apache/guacamole-server/pull/391#issuecomment-1225259186
> ... In the general case, we don't really want to be telling libssh2 what it should be using at all - it's job is to negotiate a set of algorithms and ciphers that both server and client can use. ...
I would echo that. We might need to do this now for the sake of having thins work in a FIPS environment _now_, but in the general case it's one of the things that should just work within the underlying library.
--
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-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-server] jmuehlner commented on a diff in pull request #391: GUACAMOLE-1669: Prefer FIPS compliant ciphers and algorithms when FIPS mode is enabled.
Posted by GitBox <gi...@apache.org>.
jmuehlner commented on code in PR #391:
URL: https://github.com/apache/guacamole-server/pull/391#discussion_r953191763
##########
src/libguac/fips.c:
##########
@@ -0,0 +1,68 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include "config.h"
+#include "guacamole/fips.h"
+
+#include <stdio.h>
+
+#ifdef ENABLE_SSL
+#include <openssl/opensslv.h>
+
+/* OpenSSL versions prior to 0.9.7e did not have FIPS support */
+#if !defined(OPENSSL_VERSION_NUMBER) || (OPENSSL_VERSION_NUMBER < 0x00090705f)
+
+int guac_fips_enabled() {
+
+ return 0;
+
+}
+
+/* OpenSSL 3+ uses EVP_default_properties_is_fips_enabled() */
+#elif defined(OPENSSL_VERSION_MAJOR) && (OPENSSL_VERSION_MAJOR >= 3)
+
+#include <openssl/evp.h>
+
+int guac_fips_enabled() {;
Review Comment:
I'm repeating the function definition here because these different FIPS checks require different headers to be included, and this seemed like the most straightforward way to do it.
--
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-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-server] jmuehlner commented on a diff in pull request #391: GUACAMOLE-1669: Prefer FIPS compliant ciphers and algorithms when FIPS mode is enabled.
Posted by GitBox <gi...@apache.org>.
jmuehlner commented on code in PR #391:
URL: https://github.com/apache/guacamole-server/pull/391#discussion_r954013186
##########
src/libguac/fips.c:
##########
@@ -0,0 +1,67 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include "config.h"
+#include "guacamole/fips.h"
+
+#ifdef ENABLE_SSL
+
+#include <openssl/opensslv.h>
+
+/* OpenSSL versions prior to 0.9.7e did not have FIPS support */
+#if !defined(OPENSSL_VERSION_NUMBER) || (OPENSSL_VERSION_NUMBER < 0x00090705f)
+
+int guac_fips_enabled() {
+
+ return 0;
+
+}
+
+/* OpenSSL 3+ uses EVP_default_properties_is_fips_enabled() */
+#elif defined(OPENSSL_VERSION_MAJOR) && (OPENSSL_VERSION_MAJOR >= 3)
+
+#include <openssl/evp.h>
+
+int guac_fips_enabled() {
+
+ return EVP_default_properties_is_fips_enabled(NULL);
+
+}
+
+/* For OpenSSL versions between 0.9.7e and 3.0, use FIPS_mode() */
+#else
Review Comment:
Ah, good call! That is much better, thanks. My C preprocessor skills are apparently quite rusty so this is a good learning experience.
--
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-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-server] jmuehlner commented on a diff in pull request #391: GUACAMOLE-1669: Prefer FIPS compliant ciphers and algorithms when FIPS mode is enabled.
Posted by GitBox <gi...@apache.org>.
jmuehlner commented on code in PR #391:
URL: https://github.com/apache/guacamole-server/pull/391#discussion_r954013466
##########
src/libguac/fips.c:
##########
@@ -0,0 +1,67 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include "config.h"
+#include "guacamole/fips.h"
+
+#ifdef ENABLE_SSL
+
+#include <openssl/opensslv.h>
+
+/* OpenSSL versions prior to 0.9.7e did not have FIPS support */
+#if !defined(OPENSSL_VERSION_NUMBER) || (OPENSSL_VERSION_NUMBER < 0x00090705f)
+
+int guac_fips_enabled() {
+
+ return 0;
+
+}
+
+/* OpenSSL 3+ uses EVP_default_properties_is_fips_enabled() */
+#elif defined(OPENSSL_VERSION_MAJOR) && (OPENSSL_VERSION_MAJOR >= 3)
+
+#include <openssl/evp.h>
+
+int guac_fips_enabled() {
+
+ return EVP_default_properties_is_fips_enabled(NULL);
Review Comment:
I'll get em.
--
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-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-server] jmuehlner commented on a diff in pull request #391: GUACAMOLE-1669: Prefer FIPS compliant ciphers and algorithms when FIPS mode is enabled.
Posted by GitBox <gi...@apache.org>.
jmuehlner commented on code in PR #391:
URL: https://github.com/apache/guacamole-server/pull/391#discussion_r953210440
##########
src/libguac/fips.c:
##########
@@ -0,0 +1,68 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include "config.h"
+#include "guacamole/fips.h"
+
+#include <stdio.h>
+
+#ifdef ENABLE_SSL
+#include <openssl/opensslv.h>
+
+/* OpenSSL versions prior to 0.9.7e did not have FIPS support */
+#if !defined(OPENSSL_VERSION_NUMBER) || (OPENSSL_VERSION_NUMBER < 0x00090705f)
+
+int guac_fips_enabled() {
+
+ return 0;
+
+}
+
+/* OpenSSL 3+ uses EVP_default_properties_is_fips_enabled() */
+#elif defined(OPENSSL_VERSION_MAJOR) && (OPENSSL_VERSION_MAJOR >= 3)
+
+#include <openssl/evp.h>
+
+int guac_fips_enabled() {;
Review Comment:
Let me test this again before I un-draft it.
--
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-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-server] jmuehlner commented on pull request #391: GUACAMOLE-1669: Prefer FIPS compliant ciphers and algorithms when FIPS mode is enabled.
Posted by GitBox <gi...@apache.org>.
jmuehlner commented on PR #391:
URL: https://github.com/apache/guacamole-server/pull/391#issuecomment-1226002655
>
> > Luckily for FIPS, there's a pretty small set of options that are both FIPS-compliant, AND libssh2-supported. I just listed those from biggest key sizes to smallest. I guess you could imagine somebody wanting to prefer smaller key sizes for performance reasons, but I'd guess that nobody would ever use such an option ¯_(ツ)_/¯
>
> Maybe there's a way we can check what OpenSSH considers its order of priority in a FIPS environment?
>
> Googling around, I found https://techhub.hpe.com/eginfolib/networking/docs/switches/5130ei/5200-3946_security_cg/content/485048549.htm which states:
>
> > ... In FIPS mode ... SSH2 uses the aes128-ctr, aes192-ctr, aes256-ctr, aes128-gcm, aes256-gcm, aes128-cbc, and aes256-cbc encryption algorithms in descending order of priority for algorithm negotiation.
>
> Which seems to indicate:
>
> * Prefer CTR, then GCM, then finally CBC.
> * Use as small a key as the SSH server will allow.
>
> There might be good reason or documentation for that.
> > Luckily for FIPS, there's a pretty small set of options that are both FIPS-compliant, AND libssh2-supported. I just listed those from biggest key sizes to smallest. I guess you could imagine somebody wanting to prefer smaller key sizes for performance reasons, but I'd guess that nobody would ever use such an option ¯_(ツ)_/¯
>
> Maybe there's a way we can check what OpenSSH considers its order of priority in a FIPS environment?
>
> Googling around, I found https://techhub.hpe.com/eginfolib/networking/docs/switches/5130ei/5200-3946_security_cg/content/485048549.htm which states:
>
> > ... In FIPS mode ... SSH2 uses the aes128-ctr, aes192-ctr, aes256-ctr, aes128-gcm, aes256-gcm, aes128-cbc, and aes256-cbc encryption algorithms in descending order of priority for algorithm negotiation.
>
> Which seems to indicate:
>
> * Prefer CTR, then GCM, then finally CBC.
> * Use as small a key as the SSH server will allow.
>
> There might be good reason or documentation for that.
Ah yeah, that's a good call. I'll switch the order to lowest keysize first. That's what libssh2 was doing with the other algorithms first anyway, so it's a good call. FWIW, the GCM algorithms don't seem to be supported by libssh2, so I just left those out https://ssh-comparison.quendi.de/comparison/cipher.html
--
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-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-server] jmuehlner commented on a diff in pull request #391: GUACAMOLE-1669: Prefer FIPS compliant ciphers and algorithms when FIPS mode is enabled.
Posted by GitBox <gi...@apache.org>.
jmuehlner commented on code in PR #391:
URL: https://github.com/apache/guacamole-server/pull/391#discussion_r953377942
##########
src/libguac/fips.c:
##########
@@ -0,0 +1,68 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include "config.h"
+#include "guacamole/fips.h"
+
+#include <stdio.h>
+
+#ifdef ENABLE_SSL
+#include <openssl/opensslv.h>
+
+/* OpenSSL versions prior to 0.9.7e did not have FIPS support */
+#if !defined(OPENSSL_VERSION_NUMBER) || (OPENSSL_VERSION_NUMBER < 0x00090705f)
+
+int guac_fips_enabled() {
+
+ return 0;
+
+}
+
+/* OpenSSL 3+ uses EVP_default_properties_is_fips_enabled() */
+#elif defined(OPENSSL_VERSION_MAJOR) && (OPENSSL_VERSION_MAJOR >= 3)
+
+#include <openssl/evp.h>
+
+int guac_fips_enabled() {;
Review Comment:
Ooooops, I somehow lost my Makefile changes. Definitely didn't work without those.
--
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-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-server] jmuehlner commented on a diff in pull request #391: GUACAMOLE-1669: Prefer FIPS compliant ciphers and algorithms when FIPS mode is enabled.
Posted by GitBox <gi...@apache.org>.
jmuehlner commented on code in PR #391:
URL: https://github.com/apache/guacamole-server/pull/391#discussion_r954348505
##########
src/libguac/guacamole/fips.h:
##########
@@ -0,0 +1,33 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#ifndef GUAC_FIPS_H
+#define GUAC_FIPS_H
+
+/**
+ * Returns a non-zero value if FIPS mode is enabled for OpenSSL, or zero if
+ * FIPS mode is not enabled.
+ *
+ * @return
+ * A non-zero value if FIPS mode is enabled for OpenSSL, or zero if FIPS
+ * mode is not enabled.
Review Comment:
Yeah, that's reasonable. I got rid of the OpenSSL stuff.
--
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-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-server] mike-jumper commented on pull request #391: GUACAMOLE-1669: Prefer FIPS compliant ciphers and algorithms when FIPS mode is enabled.
Posted by GitBox <gi...@apache.org>.
mike-jumper commented on PR #391:
URL: https://github.com/apache/guacamole-server/pull/391#issuecomment-1225264823
> Luckily for FIPS, there's a pretty small set of options that are both FIPS-compliant, AND libssh2-supported. I just listed those from biggest key sizes to smallest. I guess you could imagine somebody wanting to prefer smaller key sizes for performance reasons, but I'd guess that nobody would ever use such an option ¯_(ツ)_/¯
Maybe there's a way we can check what OpenSSH considers its order of priority in a FIPS environment?
Googling around, I found [this page](https://techhub.hpe.com/eginfolib/networking/docs/switches/5130ei/5200-3946_security_cg/content/485048549.htm) which states:
> ... In FIPS mode ... SSH2 uses the aes128-ctr, aes192-ctr, aes256-ctr, aes128-gcm, aes256-gcm, aes128-cbc, and aes256-cbc encryption algorithms in descending order of priority for algorithm negotiation.
Which seems to indicate:
* Prefer CTR, then GCM, then finally CBC.
* Use as small a key as the SSH server will allow.
There might be good reason or documentation for that.
--
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-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org