text:
[FEATURE] Visualization Catalog

Is your feature request related to a problem?
As we develop more integrations using standard SS4O, we will likely encounter a lot of overlap between visualization functionality. In particular, visualizations that may be commonly applicable in multiple contexts. Most integrations involving an http component might get value out of a "Status codes over time" visualization. An example of this is already present in the Nginx integration, which is being reduplicated for the upcoming Apache integration. If we don't have visualizations shared in one place, it can lead to a lot of reduplication of effort as functionality is reinvented.
What solution would you like?
The solution to this problem is to collect visualizations in one central location, which I'll call the "Visualization Catalog". This will organize visualizations within one location, so that integration developers can directly find tools that are useful for their own data, and have a better way to share their tools with others. The main problem with this is organization, and making it discoverable by developers. The catalog itself can just live as a directory on this repository, I don't think it needs to be implemented as a full API, as long as we provide an index and instructions for searching it.
For organizing these visualizations, the most important feature to know is what fields they need access to in order to run. In #31 (implemented via #32), we introduce a tool that is able to look at actual sample data records and use that to determine which fields are required. With some extra work, this tool might be extended to visualizations, to determine what components the visualization requires as well, and to connect the data and visualizations at one common schema in the middle. Adding some level of search functionality to the tool can be very helpful in giving useful results.
A basic implementation plan:

Organize existing SS4O visualizations in a new folder for the visualization catalog. Optionally, structure them by component, to provide a minimal level of immediate searchability.
Introduce documentation on developing visualizations, including what the major types are and how the different fields work.
Extend the CLI tool from #31 to dissect a visualization to determine what types of data it can work with. This can be used to check individual visualizations.
Enhance the CLI with search functionality, that lets users quickly find visualizations in the standard schema.
Enhance the CLI with the ability to automatically compile a list of visualizations into a dashboard. This can be somewhat complex, but the basic principle is to generate the Saved Object file that can be imported into OpenSearch Dashboards.
As a final step, we may also considering adding this functionality to the actual catalog API with more complex functionality, but I'm not sure if this would be productive.

These steps are not intended to all be done at once, I think just the first 3 steps will be enough to get started. But if we find more demand and the project is becoming unwieldy, then steps 4-6 can be natural extensions.
What alternatives have you considered?

We can continue to keep things unorganized, which is easy but does introduce reduplication of effort.
We can introduce minimal organization without tooling, which is hard for developers to use.

Do you have any additional context?

#31 and #32: Reading on the modern CLI tooling for integration development, which can be extended for this functionality.
Integrations Wiki: Reading on the integrations project in general.

Some open questions:

What is the actual structure of visualizations? Is this documented? I'd like to not do everything by reverse-engineering code in the OpenSearch Dashboards project, but I can't find any proper specification.
Versioning and Updates: How will updates and versioning of visualizations be handled within the Visualization Catalog? Will there be a process in place to ensure that outdated or deprecated visualizations are removed or marked as such?
Integration Testing: How will the visualizations in the catalog be tested and validated to ensure their accuracy and compatibility with different data sources and scenarios?

@Swiddis u'r proposition make perfect sense, and they correlate to the idea behind the composition of the fields into dedicated functions:

http
cloud
aws-elb
container
...

Each file may have a corresponding visual representation in a form of a dashboard / visualization / views
These correspondence can be assembled by an integration builder into a meaningful composting that represent the component the integration is observing.
Currently each mapping fields set (component mapping) has a dictionary file which describes the fields and their relationships if such exist:

http
cloud
aws-elb
container
...

Using this perspective, the visualization folder is organized in the same (similar) way the schema folder is arranged to reflect this correspondence.
After this folder is created and each component is associated with its visual elements, it will be possible to construct the integration in the following steps:

Identify the subject for which the integration resource is related with ( cloud / http / k8s / O/s  java /  windows  ...)
Find / Search all the relevant categories from the existing field set categories or labels.
Compose the most appropriate mapping components from the previous step
Assemble the visual corresponding elements of the former step
Review - Validate - Test
Publish

This will remove many obstacles for non UX experts and allow fast and efficient creation of integrations for any type of resources.
In the case the schema is incomplete - the process is much harder since the integration needs to actually define and create a new schema component which must be validated and consolidated with the following:

OTEL protocol
OpenSearch Schema support
ECS schema

I agree with applying this when the visualization is restricted to one component, but I'm not sure it's so simple when the visualization references multiple components. A visualization correlating http and container may try to show something like which containers return the most internal server errors, so it's not as cleanly divided as the components themselves are I think.

authors: Swiddis YANG-DB

repo: opensearch-project/opensearch-catalog


text:
[BUG] AD tests fail on security-enabled cluster when plugins.security.system_indices.enabled:true

What is the bug?
When plugins.security.system_indices.enabled is set to true, in opensearch.yml, security-enabled tests will fail since AD is using a custom cmd to delete system indices to cleanup test resources after running some of the test suites.
When the security setting is enabled, it blocks any DELETE requests against system indices by default. Because of this, the system index doesn't get deleted, there are leftover test resources after running some of the test suites, and the subsequent tests can fail due to unexpected resources still existing in the cluster (test indices, test detectors, etc.).

@ohltyler
do we know what block it blocks any DELETE requests?
it is because permission setup?
i didn't find any error/test fail from https://github.com/opensearch-project/opensearch-dashboards-functional-test/actions/runs/4745708789/jobs/8428403568
would you add share a failed test run job to help understand this issue

Issues started in 2.6.0 to current 2.8.0 release.
  Running:  plugins/anomaly-detection-dashboards-plugin/create_detector_spec.js             (1 of 8)

  Create detector workflow
    1) "after each" hook for "Full creation - based on real index"

  0 passing (26s)
  1 failing

  1) Create detector workflow
       "after each" hook for "Full creation - based on real index":
     CypressError: `cy.request()` failed on:

https://localhost:9200/.opendistro-anomaly*

The response we received from your web server was:

  > 403: Forbidden

This was considered a failure because the status code was not `2xx` or `3xx`.

If you do not want status codes to cause failures pass the option: `failOnStatusCode: false`

-----------------------------------------------------------

The request we sent was:

Method: DELETE
URL: https://localhost:9200/.opendistro-anomaly*
Headers: {
  "Connection": "keep-alive",
  "user-agent": "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) HeadlessChrome/112.0.5615.165 Safari/537.36",
  "accept": "*/*",
  "cookie": "security_authentication=Fe26.2**ee290319a7a5343aded95dff5288e25ab33aca0d61bf654181aa83355e10ca82*Jz6YM52FFExqD3iMztxy3w*i45fhMkm9txtJQI6rKfnWwO96QABH-ypQlTgQFt5YbpaWBaoqnaS7hcAcTyTV31ymuAtwDNMpSWxUfwn4X3ujAdEEZPlzg_Ct_POIXHPZu27dBue3tPdzZYu-3HpdxHk-wCbCdGbgJAWPmJZCvESQ9VfKRDqtsxAOhXWFHenCxNyzvlArF9R9nO4Ws9DazK2MJBTZmZ7oxOAipBbj7Q-Rw**7332c2b2e0a4eb7e6d577d487a8b49071aefa9479d24b981c806809506e07251*KQMth0xKWGjHCUpNHf8Z27_Gw2zYq_YotPschTHMsdY",
  "authorization": "Basic YWRtaW46YWRtaW4=",
  "accept-encoding": "gzip, deflate",
  "content-length": 0
}

-----------------------------------------------------------

The response we got was:

Status: 403 - Forbidden
Headers: {
  "content-type": "application/json; charset=UTF-8",
  "content-length": "297"
}
Body: {
  "error": {
    "root_cause": [
      {
        "type": "security_exception",
        "reason": "no permissions for [] and User [name=admin, backend_roles=[admin], requestedTenant=null]"
      }
    ],
    "type": "security_exception",
    "reason": "no permissions for [] and User [name=admin, backend_roles=[admin], requestedTenant=null]"
  },
  "status": 403
}

https://on.cypress.io/request

Because this error occurred during a `after each` hook we are skipping the remaining tests in the current suite: `Create detector workflow`
      at http://localhost:5601/__cypress/runner/cypress_runner.js:160667:80
      at tryCatcher (http://localhost:5601/__cypress/runner/cypress_runner.js:13022:23)
      at Promise._settlePromiseFromHandler (http://localhost:5601/__cypress/runner/cypress_runner.js:10957:31)
      at Promise._settlePromise (http://localhost:5601/__cypress/runner/cypress_runner.js:11014:18)
      at Promise._settlePromise0 (http://localhost:5601/__cypress/runner/cypress_runner.js:11059:10)
      at Promise._settlePromises (http://localhost:5601/__cypress/runner/cypress_runner.js:11139:18)
      at _drainQueueStep (http://localhost:5601/__cypress/runner/cypress_runner.js:7729:12)
      at _drainQueue (http://localhost:5601/__cypress/runner/cypress_runner.js:7722:9)
      at ../../node_modules/bluebird/js/release/async.js.Async._drainQueues (http://localhost:5601/__cypress/runner/cypress_runner.js:7738:5)
      at Async.drainQueues (http://localhost:5601/__cypress/runner/cypress_runner.js:7608:14)
  From Your Spec Code:
      at Context.eval (http://localhost:5601/__cypress/tests?p=cypress/support/index.js:368:6)

  (Results)

  ┌────────────────────────────────────────────────────────────────────────────────────────────────┐
  │ Tests:        1                                                                                │
  │ Passing:      0                                                                                │
  │ Failing:      1                                                                                │
  │ Pending:      0                                                                                │
  │ Skipped:      0                                                                                │
  │ Screenshots:  1                                                                                │
  │ Video:        true                                                                             │
  │ Duration:     25 seconds                                                                       │
  │ Spec Ran:     plugins/anomaly-detection-dashboards-plugin/create_detector_spec.js              │
  └────────────────────────────────────────────────────────────────────────────────────────────────┘

  (Screenshots)

  -  /tmp/tmpod_59grz/anomalyDetectionDashboards/cypress/screenshots/plugins/anomaly-     (1280x720)
     detection-dashboards-plugin/create_detector_spec.js/Create detector workflow --
     Full creation - based on real index -- after each hook (failed).png

  (Video)

  -  Started processing:  Compressing to 32 CRF
  -  Finished processing: /tmp/tmpod_59grz/anomalyDetectionDashboards/cypress/videos/    (2 seconds)
                          plugins/anomaly-detection-dashboards-plugin/create_detector
                          _spec.js.mp4

Related to this:

https://github.com/opensearch-project/opensearch-dashboards-functional-test/pull/554

source: gharchive/issue

authors: ohltyler peterzhuamazon seraphjiang

license_type: permissive

license_source: github-api


text:
[FEATURE] Reactive (reactor) support

Is your feature request related to a problem?
I would like to be able to use client in a reactive way
What solution would you like?
Provide a module that has a reactive version of the client. Preferably using reactor/WebFlux

Do you want to propose how this will look? Also, have you looked at the async capabilities?

This sounds like a positive direction. Are you up for starting work on a PR? Does anyone else have thoughts?

authors: kkocel wbeckler

repo: opensearch-project/opensearch-java


text:
Build automation to verify/manage drift across repositories

Is your feature request related to a problem? Please describe.
We currently rely on opening GitHub issues and manual intervention to align repositories across this organization for such things as copyright notices. When these change, such as in https://github.com/opensearch-project/.github/pull/24, there are no guarantees that everybody applies the changes in a timely manner.
Describe the solution you'd like
A mechanism that evaluates drift across repositories for such things as common files and opens GitHub issues automatically. This mechanism would support common types of things: files must match 100% (e.g. code of conduct), files must contain text (e.g. copyright notice as is), files must be present (e.g. developer_guide), TOC must be present in README, etc.

[Catch All Triage - 1, 2, 3, 4, 5]

@dblock is this issue still valid, do we need to create a tool similar to audit ?

We can use the automation app to handle a lot of these comparisons over time.

authors: dblock peterzhuamazon prudhvigodithi

repo: opensearch-project/opensearch-metrics


text:
[RFC] Improved session management

What is the problem?
OpenSearch uses cookies for session management. Cookies have a size limit (4Kb). Some extremely large session payloads (think of large SAML assertions) hit this limit and we don't currently have a path forward for those use cases.
Additional problems of the current session management implementation (or feature requests) are captured in the issues below. We should aim to solve for the main problem above, but if in doing so we can kill any of these "birds" with that stone, and they can inform the solution for this one, that would be great. Otherwise, we can tackle those separately:

#159
#214
#516
#759
#810
#828
#840
#865
#969
opensearch-project/security#375
opensearch-project/security#1448

What options could we consider?
There are may ways to skin this cat, from short term stopgaps to larger overhauls of how we manage sessions in the product.
Filter JWT attributes stored in the cookie
Right now, there are a number of attributes coming from the IdP that we store in the cookie, whether they are needed or not. Providing an allow-list of attributes and storing just these in the cookies will make the 4Kb limit go a longer way.
Use compression
Compressing the cookie payloads could relax the constrain of that 4Kb limit, as text attributes from the session are generally highly compressible (are we doing that today already? if so, disregard this option).
Stitch multiple cookies together
Another relatively low effort fix for this would be to split cookie payloads into several chunks, and stitching them together when we receive them. If we have, say, 4 chunks... would 16Kb be a high enough limit to enable even the most extreme SAML assertions?
Server-side session management
Storing the session in the backend (whether in OpenSearch Dashboards' backend or OpenSearch's), will remove the size limit, and could potentially address some of the shortcomings of using cookies such as sessions carrying over from device to device, or the ability to invalidate sessions from server side, but it adds complexity and potential use cases we need to think about carefully:

Where would the session storage be?
If we store the session in OpenSearch indices, what are the undesirable side-effects? (i.e. session info lost if cluster becomes red)
How does this help (or hurt) the decoupling of Dashboards from Core? Especially as Dashboards is adding independent security features

[feature-proposal] Dashboards security out of box #1215
[Epic] Dashboards Metadata Storage Decoupling #1441

Next steps
Let's use this issue to have a discussion of options, trade-offs, and come up with a design for what a new session management strategy should look like.

My 2 cents.
Server side session management
This seems to be the right direction for a long term fix for the limitations like large assertions, excessive roles and opens up the avenue for a lot of other user-behaviour related insights.
In the future different plugins would want to extend this to collect data relevant to their use-case. The design would need to take this into consideration. Time will be a constraint here to solve the pain-point quickly.
JWT Attribute filter
The subject key and roles form the largest portion of the JWT we generate from a SAML Response.
However, this can restrict a user from adopting an approach of having a large number of granular roles for their use-case as the number of roles can cause the cookie size to exceed.
Stitch multiple cookies together
IMO This could viable option in a short term.
The entire storage layer of cookie is currently abstracted using the session_storage interface in Dashboards. A child implementation of the existing cookie storage can be created which handles the breaking and re-creation of one cookie into multiple one's and re-uses the existing logic of cookie management.

The storage layer to be used can be made configurable via the security config defaulting to the current one. Thus, it will not be non-invasive.
RFC 2109 mentions the support of 20 cookies per domain. So, this approach can support most of the use-cases by increasing the number of cookies used for storage.

IMHO, we should identify that Cookie shall not carry the entire token. Better options are to store in the session storage instead or backend.
Demerits of storing the token in Cookie is that Client has to send this Cookie on each request, and if there are multi-cookie - all the cookies will be sent and impact the throughput of the systems.
For the  per the best practices we should decouple this into :
Front-end :
Is just a mechanism to challenge the user to fetch. In case of SAML - facilitates the redirection to IDP/Cognito.
Front-end should just validate the cookie which was directed by the backend. The cookie is the front-end session management, and technically can have different lifetimes from the backend.
Backend:
1)Security Token Services (STS)
The STS is just a token granting party - internally does a job of Authorization Server and Token Server in the OIDC terminology. Based on the token produced be it SAML, OIDC, Basic, etc., it grants the permission to create a session.
2)Session Management
Can be Cache based, index based or externalised totally. All have their own pros and cons. Ideally can be externalised to something like Redis Cache based approach.

Regarding "Filter JWT attributes stored in the cookie":
This approach is a bit more complicated as it might sound. The Dashboards plugin would be not able to just filter the JWT attrs, as then the backend would no longer trust these JWTs (due to a wrong signature). To overcome this, some module would need to re-issue JWTs which are trusted by the backend. As this is still just a workaround, I am not sure whether this is worth the effort. Also, as mentioned before, there is the chance that it is actually the roles which blow up the JWT size. So, I don't really think that this approach is suitable.

The entire storage layer of cookie is currently abstracted using the session_storage interface in Dashboards. A child implementation of the existing cookie storage can be created which handles the breaking and re-creation of one cookie into multiple one's and re-uses the existing logic of cookie management.

The storage layer to be used can be made configurable via the security config defaulting to the current one. Thus, it will be non-invasive.
RFC 2109 mentions the support of 20 cookies per domain. So, this approach can support most of the use-cases by increasing the number of cookies used for storage.
It will also provide enough time to eventually move towards server side session management.

I like this approach, as the abstraction layer exists already and we can eventually point to what @prabhat-chaturvedi suggests as an externalized storage for sessions (and at that point stop the cutting/stitching).
This is the shortest path to solving the immediate problem while allowing some more time for the larger efforts in Dashboards linked above (some of which might even make things easier to build, like the externalized metadata store).

The externalised storage is the best solution, but we will have similar challenged for the Frontend session management, which is where to store the JWT. If its opaque token which is stored as Key:randomString,Value:JWT, then the frontend can just keep a handle of that randomString which maps to the session identifier.
Otherwise, even when the JWT should be stored in the Frontend, Cookie length issue is must to be solved.
What I pointed above as an immediate alternative was to save the token in the Browser "session storage".
Dashboard frontend, can extract the JWT from the browser session storage was the session is governed by JSESSIONID(or such application managed session cookie) and then populate the JWT in a POST param, header or Cookie which ever best suits the security plugin to validate.
Again emphasising the demerits for using Cookie to  store the JWT are:

Cookie is sent on each request making the request bloated
MultiCookie may make the request too heavy, even though there was not much in the request, but the Cookies would have to go to backend.
Cookie has limitations of 4MB, but Session Storage can go upto 10MB or more in some browsers

There were some benefits when the Cookie was relayed directly from Dashboard to Security plugin, but it might just be a big change to make in the exit gate of Dashboard to pull from Session Storage and populate in the ongoing request to plugin.

Use compression

Seems really cheap and easy to check.  While we want to target the end state, this might be able to mitigate the user experience for a couple of impacted clusters while we get there.  Has anyone looked into this?

It seems like a reasonable high level estimate of gzip's rate of compression of ASCII text is 1:11 [1]. With the cookie size being limit as per this RFC to 4Kb, that could raise the limit to something like 44Kb of uncompressed ASCII text.
@jimishs, I know you have done some research on use cases around SAML assertions. Do you have a sense for what reasonable upper bound for the cookie size we should consider in this design?

@nibix regarding "Filter JWT attributes stored in the cookie" the backend trusting the JWT:
At least in my case, the the id_token is the big thing, because it contains all the group memberships.
And AFAIK only the access_token is required for authenticating against the backend. So I think it would help to only store the access_token and not storing the id_token in the cookie.

@domruf Actually, it is the other way round. The id_token is needed by the OpenSearch backend to determine the user's roles from the group membership and thus their authorization. The access_token, on the other hand, is only meaningful to the OIDC IdP service.

Besides discussing the workarounds, IMHO, we should also have a closer look at how an actual solution might look like. As a kind of kickoff for a discussion, I have drafted the following document which outlines how such a solution could look like.
Any comments are very appreciated!
Proposal: Architecture for session based login in Dashboards
Goals and basic requirements

Compact cookie size. Get rid of issues due to excessive cookie size.
"Logout" is capable of destroying the session
Usable with any authentication type available for Dashboards (i.e., password based auth, OIDC and SAML)
Sessions expire after a certain period of inactivity (i.e., an access to a session extends its time to live). However, there shall be also an absolute maximum time, after which a session gets invalidated.
Requires no external dependencies.
Should not facilitate DOS attacks by unauthenticated users.
Usable with OpenSearch Dashboards 2.x

Non-goals

Only meant for authentication at OpenSearch via Dashboards. Clients directly authenticating at OpenSearch won't be affected.
In order to keep the scope manageable, this does not yet take any plans for OpenSearch 3 into account.
The session feature is only meant for authentication. It shall not provide a flexible session storage or similar functionality.

Open: Goals or non-goals
For these items, it needs to be clarified whether it shall be a goal or not:

Shall the old session-less authentication mechanism remain available or shall it be replaced by the new mechanism
Related: Is it acceptable that users get logged out when switching to the new mechanism?
Besides the default session storage, shall we provide also support for external storage components such as Redis?
What about exotic authentication mechanisms in Dashboards such as proxy auth?

Fundamental design

Introduce a new REST API which consumes credentials (user name, password or SAML AuthnResponses or OIDC tokens), validates these, creates a session and returns a session handle. This REST API is provided by the security plugin of the OpenSearch backend. The REST API is used by the Dashboards security plugin to create a session when a user is trying to log in.
The session handle is actually a signed JWT. In contrast to the current JWT usage of Dashboards, the JWT shall only contain a session ID. It shall not contain information about roles or user attributes. By using a signed JWT, we can avoid a certain class of DOS attacks of unauthenticated users on the session storage, as we can validate the JWT before hitting the session storage.
By default, sessions are stored in a private OpenSearch index. Each session is represented by one document. A session document contains: User name, roles, user attributes, session expiry time.
Dashboards saves the session JWT at the client side using a cookie.
The Dashboards security plugin reads that cookie, extracts the JWT and sends it along with every request it issues to the OpenSearch backend. It is sent inside a HTTP Authentication: bearer header.
The OpenSearch backend has a new authentication module that can check the validity of the JWT based on its signature and the private session store. If the JWT is invalid or the session is expired, the OpenSearch backend issues a 401 response.

Open design decisions

Dashboards can generate quite a lot requests during a single page load. Is it okay that every single request triggers an access to the internal session index? Or do we need a kind of cache in front of Open Search? We also need to take into account that using a session is supposed to extend the session lifetime. Thus, we actually would need to write information to the index when a session is used.

I've run some tests with actual SAML payloads with 200 SAML roles in them, and after compressing them they are still under 4Kb:
80 -rw-r--r--   1 davelago  38091 Feb 15 13:32 saml_response_200_roles.txt
 8 -rw-r--r--   1 davelago   3723 Feb 15 13:32 saml_response_200_roles.txt.gz

For reference, the js I used to test compression was:
const { createGzip } = require('node:zlib');
const { pipeline } = require('node:stream');
const {
  createReadStream,
  createWriteStream,
} = require('node:fs');

const gzip = createGzip();
const source = createReadStream('saml_response_200_roles.txt');
const destination = createWriteStream('saml_response_200_roles.txt.gz');

pipeline(source, gzip, destination, (err) => {
  if (err) {
    console.error('An error occurred:', err);
    process.exitCode = 1;
  }
});

It seems like a worthy fix to add compression to solve the immediate painpoint, and we can tackle the larger server-side session management as part of those ongoing efforts (https://github.com/opensearch-project/OpenSearch-Dashboards/issues/1215, https://github.com/opensearch-project/OpenSearch-Dashboards/issues/1441).

+1 to the idea of cookie compression and optimizing cookie to exclude parameters that arent needed / can be removed.
Regarding the question of upper bound - im aware of customers with 800-1000 workgroup attributes (that may or may not map to one or more OpenSearch security roles). So while the size could vary, 20-30 kb was what was estimated for such large assertions. If we can go upto 44kb, uncompressed, that could solve near term concerns for large customers.

Just a small clarification regarding SAML responses: The Dashboards security plugin never stores SAML responses in its cookies. Rather, it converts these into a JWT using an API in the OpenSearch backend.
Still, as these JWTs are not encrypted,  these should also have a good compression ratio.
One other question: Do you only encounter these issues only for SAML or also for OIDC?
For OIDC, the id_token is stored in the cookie. This can be also compressed. However, some IdPs might choose to provide encrypted id_tokens. As encrypted data is not well compressible, this will not provide such a strong benefit (it will bring some benefit by compensating for the base64 encoding size gain).

I have filed a draft pull request with a first shot at the compression solution. See https://github.com/opensearch-project/security-dashboards-plugin/pull/1338
Observations so far:

The core Dashboards/Hapi session cookie storage mechanism uses JSON to serialize the payload. In order to put compressed data - which is binary - into the JSON payload, we need to base64 encode it. This nullifies a part of the compression effect. We are still looking into serializing the cookie differently to avoid the base64 encoding.
In order to verify the actual effectiveness of this change, some real world example (or close to real world example) of a payload triggering the issue would be great.

I have created a sample SAML AuthnResponse to serve as a test case for our research. The AuthnResponse contains 200 roles encoded in a DN format. I understand that even though this is quite a big amount, it is still something that is realistic and must be expected.
The encoded SAML AuthnResponse (28 kb):
[encoded data, 28468 characters]

The JWT created by the security plugin from the AuthnResponse (14kb):
eyJhbGciOiJIUzUxMiJ9.[encoded data, 13988 characters].oM0VAgxeElWHtohYbOt4VnisE3Ne87ZRdXKvngQPytpP7jHAFDQIK6akRzyXOcYy0_3IHbpwdFCieNei64m9wQ

When using the compression solution in #1338 this JWT goes through the following stages:

Deflate to binary data: 2326 byte
Base 64 encoded data (because we need to store in in JSON): 3104 byte
Embedded in JSON and iron encrypted and base 64 encoded (in order to store binary encrypted data as cookie): 4409 byte

Maximum cross-browser safe cookie size is 4093 bytes. Thus, the solution in #1338 won't work with the reference AuthnResponse.
Still, this size is blown up due to the double base64 encoding. We are still investigating ways to avoid the double encoding.

I guess in my previous post there was a confusion on Long term solution on Server based session management Vs Local Browser Session storage(another short term solution).
Not in complete favour of cookie compression apart from other issues stated above, cookie goes in each request thereby increasing the payload overhead.
Can we look into immediate short term solution alternative - using Local Session storage, as Cookie has limitations of 4MB, but Session Storage can go upto 10MB or more in some browsers.
The effort required are still the same, the place of compression and decompression - we need to put logic to put in browser session storage and pull out at the same entry and exit points.
Please don't confuse with Long term solution of server side session management, this proposal is for short term Browser session storage. This is the best practice as per the industry standard for session management and not storing details in the cookie as much as possible.

@prabhat-chaturvedi

Not in complete favour of cookie compression apart from other issues stated above, cookie goes in each request thereby increasing the payload overhead.

As the possibly bloated JWT is the authentication token, we must pass it to Dashboards (and the OpenSearch backend) for every request. Otherwise, these requests are unauthenticated.

True, @nibix - all that remains as is, instead of storing the JWT in the cookie, store in something like session.setAttribute("autheticatedJWT", actualJWT).

Only storage changes as a short term solution
logic to send to backend remains same.
code changes only around the place where we currently read and write cookie.

@prabhat-chaturvedi If we use local storage or any other means of storing the data instead of Cookie on browser side, then we need to read it and attach in every request sent from Dashboards front-end to Dashboards backend.
Currently, browser takes care of attaching the cookie with every request for us.

@anijain-Amazon I will have to dig deep onto it and limited to my knowledge specific to OpenSearch.
Generally assuming, there are two security context(sessions) which are talking about here.
Session 1 : User session at Dashboard frontend - where I propose to use browser session storage and passed on to below
Session 2 : Take info from above and Dashboard (acting as browser) to Security plugin - populate and send as a Authorization "Bearer" Header(as the best practices to send Bearer JWT for OAuth/OIDC) or other custom header.

@nibix Great details with the reference cookie, 4409 byte is on the order of 4093 bytes.
If we employed a cookie splitter, seems like the base case without implementing the server side solution.  Do you think that is a viable approach for the near term?
(Even setting aside the double base64 encoding issue - which means there might be even more headroom!)

@peternied

If we employed a cookie splitter, seems like the base case without implementing the server side solution. Do you think that is a viable approach for the near term?

I believe that cookie splitting/stitching would be viable - but also tricky. The base solution linked in the PR above has the advantage that it is limited to the Dashboards security plugin. For more intrusive solutions, it is likely that we need to make changes in core Dashboards.

Session 1 : User session at Dashboard frontend - where I propose to use browser session storage and passed on to below

@prabhat-chaturvedi If we use Browser Session storage, then pass it to the request to backend will require code changes at all places.

A short status update on this:
We're working on cleaning up the PoC we started in https://github.com/opensearch-project/security-dashboards-plugin/pull/1352.
We've implemented this for OIDC as well, and we've made the number of additional cookies configurable.
One interesting finding that probably should be mentioned in the docs when this goes live is that if the id token from the IdP is very large, we may run into the following error:
server    log   [13:38:20.110] [error][plugins][securityDashboards] OpenId authentication failed: Error: [too_long_http_header_exception] HTTP header is larger than 8192 bytes. server    log   [13:38:23.333] [error][plugins][securityDashboards] OpenId authentication failed: Error: Request Timeout after 30000ms

I ran into this while testing an id token containing 200 roles. After receiving the token from the IdP, the authinfo endpoint is called with the token in the authorization header.
Luckily, this can be mitigated by changing the following value in opensearch.yml:
http.max_header_size: 16kb
I believe this is something that the customers would need to configure together with the cookie compression/splitting, otherwise the request will fail before we squeeze the token into the additional cookies.
Not sure if changing this setting has any negative side effects...?

We've updated the pull request with the cookie stitching: https://github.com/opensearch-project/security-dashboards-plugin/pull/1352
The main difference is that it is now implemented for both OpenId and SAML.
I updated the description in the PR to reflect the changes, and I also added some questions to which I would appreciate your feedback.
The main questions going forward whether you would like the number of cookies to be configurable. There are some drawbacks to that, mainly related to if the customer changes the number of cookies.
We could also give making the number of used cookies dynamic another try. In other words don't register any cookies with the Hapi server, and instead try to write as many cookies as needed by the actual content.
The issue we ran into with that was that we weren't able to delete the cookies completely, e.g. when logging out...
Anyhow, happy for suggestions for improvements!

@anijain-Amazon

Also, we might be introducing some overhead in latency by extra base64 encoding, so turning this off if required can be desirable.

This base64 encoding also occurs now and is kind of essential as the encrypted data is likely binary and cannot be stored as such in a cookie. Also, I don't believe that such a overhead is significant here.

We can consider keeping this implementation as optional via configuration since most of the users of OpenSearch might not have a requirement of cookies more than 4Kb.
Also, we might be introducing some overhead in latency by extra base64 encoding, so turning this off if required can be desirable.

Given that the ones turning this off would be users with small cookies to pass around, the overhead in their case would be quite negligible, and IMHO not worth the additional settings and code paths (that would also need to be tested) to make this an optional feature.

I am one of the many sufferers of getting booted out of dashboards after an hour. I have tried making the two recommended changes of extending the JWT time and cookie session length, but neither appears to make any meaningful change. Is there anything I can do as an admin/user today to increase this timeout value before getting kicked out to my IDP and having to re-auth. Other SAML based applications don't seem to suffer from this  issue.

@nobletrout
For SAML Authentication, the expiry of the cookie is determined using the expiry of the JWT Token ( generated by Security Plugin by parsing the SAML Response ) CodePointer
The expiry of the JWT Token is determined using the SAML Response. CodePointer
Maybe you can try increasing the Session TTL at your IdP.
PS at the first glance, your issue seems to be not related to this RFC. Please feel free to open a separate Github issue for the same, so that the discussion can be continued there.

I believe #828 is the one you might be thinking of @anijain-Amazon... this issue links it at the top as one of the many things we'd like to fix with better session management. The cookie splitting work addresses just a few of these and more as a workaround for now, but yeah, there are still plenty of the ones linked above that won't be fixed this upcoming release.

authors: anijain-Amazon davidlago domruf jimishs jochen-kressin nibix nobletrout peternied prabhat-chaturvedi


text:
Clear the contents of opensearch_dashboards prior to putting settings

Description
Figured out the issue was the removal of newline here: https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5777/files#r1480507232. This caused the first line of server.host to be written into a comment, and thus leaving cypress not able to access OSD.
Category
[Enhancement, New feature, Bug fix, Test fix, Refactoring, Maintenance, Documentation]
Why these changes are required?
What is the old behavior before changes and new behavior after changes?
Issues Resolved
[List any issues this PR will resolve (Is this a backport? If so, please add backport PR # and/or commits #)]
Testing
[Please provide details of testing done: unit testing, integration testing and manual testing]
Check List

[ ] New functionality includes testing
[ ] New functionality has been documented
[ ] Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

FYI this is useful to debug GHA: https://github.com/lhotari/action-upterm

Also FYI: https://stackoverflow.com/questions/20778771/what-is-the-difference-between-0-0-0-0-127-0-0-1-and-localhost

source: gharchive/pull-request


text:
Incremented version to 1.2.4.

Coming from https://github.com/opensearch-project/opensearch-build/issues/1417.

Closing in favor of #1558 to save you the work of updating as that includes the cherrypick of a fix for the annoying timeouts in github actions that make PRs a multi-hour affair to merge.

authors: davidlago dblock

repo: opensearch-project/security


text:
[BUG] Query anonymization doesn't work in fallback log

What is the bug?
Query anonymization is not working in the fallback log
How can one reproduce the bug?
Write a simple query and add ?format=json to trigger falling back to old engine.
POST /_plugins/_sql?format=json
{
  "query" : """
  SELECT min(DistanceMiles), avg(DistanceMiles), max(DistanceMiles)
    FROM opensearch_dashboards_sample_data_flights
      GROUP BY Carrier
  """
}

Or write a query which is unsupported in V2, e.g JOIN
POST /_plugins/_sql
{
  "query" : """
  SELECT s.FlightNum, s.OriginCityName, d.DestCityName
    FROM opensearch_dashboards_sample_data_flights s
      JOIN opensearch_dashboards_sample_data_flights d
        ON s.DestCityName = d.OriginCityName
  """
}

See this line in the logs

Request SQLQueryRequest(jsonContent={"query":"\n  SELECT min(DistanceMiles),avg(DistanceMiles),max(DistanceMiles) FROM opensearch_dashboards_sample_data_flights GROUP BY Carrier\n  "}, query=
SELECT min(DistanceMiles),avg(DistanceMiles),max(DistanceMiles) FROM opensearch_dashboards_sample_data_flights GROUP BY Carrier
, path=/_plugins/_sql, format=json, params={pretty=true, format=json}, sanitize=true, cursor=Optional.empty) is not supported and falling back to old SQL engine
Request SQLQueryRequest(jsonContent={"query":"\n  SELECT s.FlightNum, s.OriginCityName, d.DestCityName FROM opensearch_dashboards_sample_data_flights s JOIN opensearch_dashboards_sample_data_flights d ON s.DestCityName = d.OriginCityName\n  "}, query=
SELECT s.FlightNum, s.OriginCityName, d.DestCityName FROM opensearch_dashboards_sample_data_flights s JOIN opensearch_dashboards_sample_data_flights d ON s.DestCityName = d.OriginCityName
, path=/_plugins/_sql, format=jdbc, params={pretty=true}, sanitize=true, cursor=Optional.empty) is not supported and falling back to old SQL engine

What is the expected behavior?
Logs should show

Request SQLQueryRequest(query=
SELECT MIN ( identifier ), AVG ( identifier ), MAX ( identifier ) FROM table GROUP BY identifier
, path=/_plugins/_sql, format=json, params={pretty=true, format=json}, sanitize=true, cursor=Optional.empty) is not supported and falling back to old SQL engine
Request SQLQueryRequest(query=
SELECT identifier, identifier, identifier FROM table s JOIN table d ON identifier = identifier
, path=/_plugins/_sql, format=jdbc, params={pretty=true}, sanitize=true, cursor=Optional.empty) is not supported and falling back to old SQL engine

What is your host/environment?

Version 2.15
Plugins SQL

Do you have any screenshots?
If applicable, add screenshots to help explain your problem.
Do you have any additional context?
Add any other context about the problem.

[Catch All Triage w/ 1, 2, 3]

authors: LantaoJin dblock

repo: opensearch-project/sql


text:
Givingtuesday landing page

Ticket(s)
2385 Social Media icons

2385 Editing Social Icons

Type of change
Please delete any options that are not relevant

[ ] Bug fix (non-breaking change which fixes an issue)
[x] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)

Description
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
Fixes # (issue)
Screenshots
No visual change
Before
After
Checklist:

[x] I have performed a self-review of my own code
[x] I have commented my code, particularly in hard-to-understand areas
[x] I have made corresponding changes to the documentation (if applicable)
[x] My changes generate no new warnings
[x] I have added tests that prove my fix is effective or that my feature works (if applicable)
[x] New and existing unit tests pass locally with my changes

 Picture won't show up in Facebook or LinkedIn pop-ups until deployed to live.

Great work! Looks like the social pop-ups are working on the Amplify preview. I would like to find a way to make the home banner a bit more responsive though. It ends up looking like this on small screen/mobile -

Also, can I remove the standalone "Donate" button on the landing page if we have the sticky button at the top?

Also, can I remove the standalone "Donate" button on the landing page if we have the sticky button at the top?

Sounds fine with me 🤷

repo: openseattle/clearviction

license: MIT


text:
remove old files

How do people feel about removing these old files? I checked the page traffic for this repo and these two don't appear at all under the popular content tab, so they haven't had many views recently (I can't determine if they've had any views though).
I know the CF docs no longer link to those files.

yes please!
LGTM

LGTM

LGTM

LGTM

authors: duglin fmui mattmcneeney pmorie vaikas-google

repo: openservicebrokerapi/servicebroker

license_source: bigquery


text:
MGMT-13752: Fix description not filled in

Back in
https://github.com/openshift-assisted/assisted-installer-deployment/pull/353 I acted on the assumption that filling-in the description right at ticket creation is not required, as later on we're activating the FailureDescription signature (which is what's filling-in the ticket description). It turns out that the code reads logs URL from the description, and that the signature doesn't apply by default.
This changes the code to be a bit more coherent around the source-of-truth for getting logs base URL, as well as making the signatures less dependent on each other.

/cc @omertuc @carbonin
/hold (testing in jenkins)

/lgtm

/unhold seems to work on jenkins

/unhold

authors: omertuc osherdp


text:
Bug 2057495: Auto increase volume size to 20 GiB in the default storage class

This will re-create the default storage class with a new volumeSizeAutoAvailable parameter that tells the driver to bump the volume sizes to 20 GiB.
Users may opt-out by creating a different default storage class.
cc @storage

/hold
https://github.com/openshift/alibaba-cloud-csi-driver/pull/12 must be merged first

Umm, how the tests could pass if the driver does not support volumeSizeAutoAvailable yet?

There is an open BZ for that: https://bugzilla.redhat.com/show_bug.cgi?id=2047735 :-D

/test e2e-alibabacloud-disk-csi

/retest

/retest
It works for me on cmdline

/hold cancel
https://github.com/openshift/alibaba-cloud-csi-driver/pull/12 was merged

authors: bertinatto jsafrane


text:
Add List types for console API

What this PR does?
This PR adds missing registration for List types for console API.
It looks like a bug for me but please correct me if I'm wrong.

/ok-to-test

/lgtm

/assign @bparees
Just for approval.

/approve

authors: benjaminapetersen bparees sleshchenko

repo: openshift/api


text:
DPTP-3921: Updating docs for supplemental images

/cc @openshift/test-platform @hongkailiu
/hold wait for https://github.com/openshift/release/pull/50503 to merge

/lgtm

/unhold

repo: openshift/ci-docs


text:
Bug 1732945: Report infra provider and feature set metrics

The infrastructure provider metric summarizes the desired type and
region (if any) to prometheus, based on the desired config. If type
is not recognized it is passed along, if it is empty it is set as
empty.
The featureset metric reports what featureset is in use and 1 if it
is "supported" and 0 if it is "not supported" (in the tech preview
sense).

@deads2k @sttts is verify-reps supposed to be broken?

#535 is open to fix the verify-deps issue you are seeing
That aside, I see two different PRs both attempting to backport this fix. This one, and #537 cc @p0lyn0mial
I can't approve both of them. Let me know which one of these you actually want approved.

/bugzilla refresh

/bugzilla refresh

I can't approve both of them. Let me know which one of these you actually want approved.

let's take this one.

/retest
/bugzilla refresh

@deads2k @sttts is verify-reps supposed to be broken?

can you pick commits from https://github.com/openshift/cluster-kube-apiserver-operator/pull/535 (@eparis didn't want it to go separately so we don't spin an image with no changes)

Picked #535 into this PR

Can I get approval and lgtm on this now that we're back to green?

/approve
/lgtm

authors: derekwaynecarr jwforres p0lyn0mial smarterclayton tnozicka


text:
Bump boilerplate to v5.0.0

to enable https://github.com/openshift/configuration-anomaly-detection/pull/271

/close needs to be done with #271


text:
Ensure hive-trusted-cabundle earlier

In HIVE-2198 / #1999 / 253408ed8 we started injecting a ConfigMap containing a CA bundle for better proxy support. Alas, we were only injecting it in the provisioning path. For hives that never had the earlier code, this probably would have been fine, as the ConfigMap would continue to exist until the cluster was deleted, whereupon it could properly be mounted on the deprovisioning pods. But for spokes that existed prior to upgrading to that code, and which were never re-reconciled before they were deleted (or, conceivably, if a user deleted the ConfigMap at some point in there) the ConfigMap would not exist, resulting in mount failures attempting to bring up the deprovisioning pods.
This commit moves the ConfigMap injection earlier in the flow, ensuring it exists for both provisioning and deprovisioning paths.
HIVE-2207

/assign @suhanime
/cc @abutcher

/test e2e

Manual merging around freeze as this is a fix that needs to get into stage before we promote.

repo: openshift/hive


text:
OCPBUGS-17669: Remove cluster name validation from HCC

What this PR does / why we need it:
This PR mostly reverts https://github.com/openshift/hypershift/pull/2914. The note added to the docs in that PR should remain. This PR removes the cluster name validation from the HostedCluster Controller (HCC). This is not needed in the HCC since the KAS will reject a HostedCluster CR with an invalid name automatically.
Which issue(s) this PR fixes:
Fixes OCPBUGS-17669
Checklist

[x] Subject and description added to both, commit and PR.
[x] Relevant issues have been referenced.
[ ] This change includes docs.
[x] This change includes unit tests.

/jira refresh

/lgtm

/cherry-pick release-4.14

/approve

authors: bryan-cox muraee sjenning

repo: openshift/hypershift


text:
add Route ServicePublishingStrategy for Konnectivity

Add a Route ServicePublishingStrategy for Konnectivity and switches to use it by default.
The APIServer is the only service remaining that requires an LB.
@derekwaynecarr @csrwng

/test e2e-aws-pooled

e2e test timed out during cleanup but the tests passed

/lgtm
/hold
Feel free to remove hold when ready, thanks!

try one more time
/test e2e-aws-pooled

/hold cancel

authors: ironcladlou sjenning


text:
Update dependencies, keep up with API changes

bump github.com/containerd/containerd from v1.7.11 to v1.7.12
bump github.com/docker/docker from v24.0.7 to v25.0.1
bump github.com/fsouza/go-dockerclient from v1.9.7 to v1.10.1
bump github.com/containers/storage from v1.51.0 to v1.52.0
bump github.com/moby/buildkit from v0.11.4 to v0.12.4

The github.com/docker/docker/api/types.AuthConfig type was moved to the github.com/docker/docker/api/types/registry package, and the type alias that allowed us to not care has gone, so our declarations for variables of that type need to be updated.

This blocks https://github.com/containers/buildah/pull/5301.  I expect to tag a release once this is merged.

FWIW, Docker is not using Go modules the usual way, meaning go github.com/docker/… does not update transitive dependencies.
The dependencies are managed in a file named vendor.mod; I usually copy&paste the whole contents into the …/docker/docker caller’s go.mod, run go mod tidy, and then manually clean up.
Doing so here causes a few more (fairly minor) updates.

Oh, I wasn't aware.  Worth doing since there's no automation in this repository for it.

@nalind tests aren't hip

Unit tests are waiting on https://github.com/openshift/release/pull/48288.  I don't have the privileges to see what the errors are in the Go 1.21 build in Travis, but it's passing with 1.20.

Can we keep it at 1.20 for now?

I guess.

/test unit

Unit tests are passing now that https://github.com/openshift/release/pull/48288 is merged, and Travis is only testing with 1.20.
We should probably move off Travis to something where the permissions allow us to view failure logs.

authors: TomSweeneyRedHat mtrmac nalind rhatdan

repo: openshift/imagebuilder


text:
Fix build on other Linux architectures

Merge from upstream:
https://github.com/kubernetes-sigs/kubefed/pull/1039
https://github.com/kubernetes-sigs/kubefed/commit/6370d758172836c6501fe33764f0c9f6e37aa3bb
We will need this in 4.2 and later branches.

/retest

@yselkowitz Thanks for filing this PR. It looks like we're planning to fast-forward our master branch to upstream v0.1.0-rc4 release which includes your change as well so I'm closing this PR to avoid any duplicate work.

@sohankunkerkar will 4.2 and 4.3 branches also be fast-forwarded?  We need those fixed as well.

@yselkowitz yes, those will get forwarded from the downstream master.

authors: abhat sohankunkerkar yselkowitz

repo: openshift/kubefed


text:
Add general helper in resourceapply that evaluate multiple resources created.

Follow up for https://github.com/openshift/library-go/pull/70

/assign @sanchezl

repo: openshift/library-go


text:
OLS-1248: Switch to the GPU buildah-remote task provided by Konflux

OLS-1248: Switch to the GPU buildah-remote task provided by Konflux

/hold
The GPU build task PR got backed out https://github.com/konflux-ci/build-definitions/pull/1638

repo: openshift/lightspeed-rag-content


text:
OLS-387: Add user data collection config

Description
Add config for ols user data collection.
Type of change

[x] New feature

Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 96.95%. Comparing base (f5f6370) to head (bf9fb4b).
Report is 22 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #595      +/-   ##
==========================================
- Coverage   96.99%   96.95%   -0.05%
==========================================
  Files          49       49
  Lines        1597     1575      -22
==========================================
- Hits         1549     1527      -22
  Misses         48       48

Files
Coverage Δ

ols/app/models/config.py
98.16% <100.00%> (+0.07%)
:arrow_up:

... and 4 files with indirect coverage changes

/lgtm

The motivation for the new (pydantic/Michal's) approach.

Michal doesn't need to refactor this in his PR.
If Michal is not able to finish his PR, whenever there is a need to touch some part of the config, it should be refactored as part of the work.

@onmete OLS-387 also requires an api the UI can query to check if feedback is disabled, are you intending to do that in a separate PR?  just want to make sure the acceptance criteria isn't missed.

OLS-387 also requires an api the UI can query to check if feedback is disabled

I will do that in the feedback PR.

/approve
/lgtm

authors: bparees codecov-commenter onmete tisnik

repo: openshift/lightspeed-service


text:
Use sorted imports + enable more linters

Type of change

[x] Refactor
[ ] New feature
[ ] Bug fix
[ ] Optimization
[ ] Documentation Update
[x] Configuration Update

Description

Use sorted imports everywhere
More linters can be enabled right now

Checklist before requesting a review

[x] I have performed a self-review of my code.
[ ] If it is a core feature, I have added thorough tests.

Testing

See CI jobs

/assign @bparees

/assign @vishnuchalla

/assign @thoraxe

I don't fully understand this PR. In some cases, it appears that you are not alpha sorting, but rather some other ordering comes first and then, within that ordering, you are alpha sorting.
Is there some PEP that describes the correct way to sort and group imports?

Ideal behavior is:

sort default packages
sort third party packages
sort in-project packages

/approve
/lgtm

authors: thoraxe tisnik


text:
ROSA support topic.

This topic is specifically for ROSA because customers can contact either Red Hat or AWS for support.
QE and peer review needed.
JIRA: https://issues.redhat.com/browse/OSDOCS-1779

@yuwang-RH  @vikram-redhat  @jeremyeder  @okashi18  @davemulford

This looks good to me!

authors: davemulford jonquilwilliams

repo: openshift/managed-openshift-docs


text:
Add commands to list and get must-gathers from hive s3 buckets

Creates two commands:
ocm backplane mg-log-ls # shows list of must-gathers in the hive s3 bucket
ocm backplane mg-log-extractor  -must-gather.tar.gz # extracts a must-gather from the hive s3 bucket

:+1: from me, Taylor planning some additional verification testing to follow.

/hold
Testing via the documented test process doesn't seem to work, bringing it up with the backplane team

/hold cancel

Minor comments, others lgtm.
/approve

/lgtm

authors: fahlmant robotmaxtron wanghaoran1988

repo: openshift/managed-scripts


text:
WIP: dummy PR to test metallb CI

dummy PR to test metallb CI

finished the testing not needed any more

repo: openshift/metallb


text:
odo complains of failing to create ingress but URL gets created

What versions of software are you using?
Operating System: all
Output of odo version: master
How did you run odo exactly?
$ odo url create --host `minikube ip`.nip.io
 ✓  URL node-todo-8080 created for component: node-todo

To apply the URL configuration changes, please use `odo push`

$ odo url list
Found the following URLs for component node-todo
NAME               STATE          URL                                             PORT     SECURE     KIND
node-todo-8080     Not Pushed     http://node-todo-8080.192.168.39.156.nip.io     8080     false      ingress
There are local changes. Please run 'odo push'.

$ odo push

Actual behavior
odo push

Validation
 ✓  Validating the devfile [32094ns]

Creating Kubernetes resources for component node-todo
 ✓  Waiting for component to start [7s]
 ⚠  Unable to create ingress, missing host information for Endpoint http-8080, please check instructions on URL creation (refer `odo url create --help`)

Applying URL changes
 ✓  URL node-todo-8080: http://node-todo-8080.192.168.39.156.nip.io/ created

Syncing to component node-todo
 ✓  Checking files for pushing [568651ns]
 ✓  Syncing files to the component [188ms]

Executing devfile commands for component node-todo
 ✓  Executing install command "npm install" [20s]
 ✓  Executing run command "npm start" [1s]

Pushing devfile component node-todo
 ✓  Changes successfully pushed to component

$ kubectl get ingress
NAME                       CLASS    HOSTS                                  ADDRESS          PORTS   AGE
node-todo-8080-node-todo   <none>   node-todo-8080.192.168.39.156.nip.io   192.168.39.156   80      75s

In above output, odo complains Unable to create ingress, missing host information for Endpoint http-8080, please check instructions on URL creation (refer odo url create --help)
But ingress has been successfully created and I can also access the app on the URL that was created
Expected behavior
odo should not complain when things just work
/kind bug
/area url
/area UX

In above output, odo complains Unable to create ingress, missing host information for Endpoint http-8080, please check instructions on URL creation (refer odo url create --help``

The creation of URL http-8080 failed. It was the default URL in the devfile.yaml.

Applying URL changes
✓  URL node-todo-8080: http://node-todo-8080.192.168.39.156.nip.io/ created

The URL created was node-todo-8080.

In above output, odo complains Unable to create ingress, missing host information for Endpoint http-8080, please check instructions on URL creation (refer odo url create --help``

The creation of URL http-8080 failed. It was the default URL in the devfile.yaml.

But the original devfile for nodejs has port 3000. I changed it from 3000 to 8080 before doing odo push. So http-8080 isn't default in that case, no?

But the original devfile for nodejs has port 3000. I changed it from 3000 to 8080 before doing odo push. So http-8080 isn't default in that case, no?

http-8080 was the URL present when the component was created. It is the default endpoint in the nodejs devfile from the registry (https://github.com/odo-devfiles/registry/) https://github.com/odo-devfiles/registry/blob/0b0e73401e462324a8db32b60c625f56ad639391/devfiles/nodejs/devfile.yaml#L20. odo url create created a new URL in the devfile.

I think we both are talking about http-3000 and not http-8080. The link you shared has http-3000 as the endpoint in devfile. Am I missing something? :thinking:

While doing odo push, odo tries to create url from devfile.yaml, this message is generating during that,
Unable to create ingress, missing host information for Endpoint http-8080, please check instructions on URL creation (refer `odo url create --help`)

however, you already have created the url using odo url create  which is successful.
This behavior of odo url gets confusing for ingress, i mean we are auto creating url during odo push  which works ok for route case but not for ingress one, as we cannot provide host information in devfile.

checked the code, we are checking against endpoint name, before default creating against odo push

But the original devfile for nodejs has port 3000. I changed it from 3000 to 8080 before doing odo push. So http-8080 isn't default in that case, no?

@dharmit you have updated the port but have you changed endpoint name?

@dharmit you have updated the port but have you changed endpoint name?

Yes. I changed the port number as well the endpoint name. I just did sed -i s/3000/8080/' devfile.yaml on the default devfile that odo create pulls for me.

Yes. I changed the port number as well the endpoint name. I just did sed -i s/3000/8080/' devfile.yaml on the default devfile that odo create pulls for me.

hmm, then something is broken here :|

Don't see this any more against main branch.
/close

authors: adisky dharmit mik-dass

repo: openshift/odo


text:
IPI baremetal: Error: missing provider "ironic"

Describe the bug
# openshift-baremetal-install --log-level debug create cluster
...
INFO Creating infrastructure resources...
DEBUG Symlinking plugin terraform-provider-aws src: "/root/bin/openshift-baremetal-install" dst: "/tmp/openshift-install-863457276/plugins/terraform-provider-aws"
DEBUG Symlinking plugin terraform-provider-azureprivatedns src: "/root/bin/openshift-baremetal-install" dst: "/tmp/openshift-install-863457276/plugins/terraform-provider-azureprivatedns"
DEBUG Symlinking plugin terraform-provider-libvirt src: "/root/bin/openshift-baremetal-install" dst: "/tmp/openshift-install-863457276/plugins/terraform-provider-libvirt"
DEBUG Symlinking plugin terraform-provider-vsphereprivate src: "/root/bin/openshift-baremetal-install" dst: "/tmp/openshift-install-863457276/plugins/terraform-provider-vsphereprivate"
DEBUG Symlinking plugin terraform-provider-google src: "/root/bin/openshift-baremetal-install" dst: "/tmp/openshift-install-863457276/plugins/terraform-provider-google"
DEBUG Symlinking plugin terraform-provider-local src: "/root/bin/openshift-baremetal-install" dst: "/tmp/openshift-install-863457276/plugins/terraform-provider-local"
DEBUG Symlinking plugin terraform-provider-vsphere src: "/root/bin/openshift-baremetal-install" dst: "/tmp/openshift-install-863457276/plugins/terraform-provider-vsphere"
DEBUG Symlinking plugin terraform-provider-openstack src: "/root/bin/openshift-baremetal-install" dst: "/tmp/openshift-install-863457276/plugins/terraform-provider-openstack"
DEBUG Symlinking plugin terraform-provider-ovirt src: "/root/bin/openshift-baremetal-install" dst: "/tmp/openshift-install-863457276/plugins/terraform-provider-ovirt"
DEBUG Symlinking plugin terraform-provider-random src: "/root/bin/openshift-baremetal-install" dst: "/tmp/openshift-install-863457276/plugins/terraform-provider-random"
DEBUG Symlinking plugin terraform-provider-azurerm src: "/root/bin/openshift-baremetal-install" dst: "/tmp/openshift-install-863457276/plugins/terraform-provider-azurerm"
DEBUG Symlinking plugin terraform-provider-ignition src: "/root/bin/openshift-baremetal-install" dst: "/tmp/openshift-install-863457276/plugins/terraform-provider-ignition"
DEBUG Symlinking plugin terraform-provider-kubernetes src: "/root/bin/openshift-baremetal-install" dst: "/tmp/openshift-install-863457276/plugins/terraform-provider-kubernetes"
DEBUG Symlinking plugin terraform-provider-kubevirt src: "/root/bin/openshift-baremetal-install" dst: "/tmp/openshift-install-863457276/plugins/terraform-provider-kubevirt"
DEBUG Initializing modules...
DEBUG - bootstrap in ../../tmp/openshift-install-863457276/bootstrap
DEBUG - masters in ../../tmp/openshift-install-863457276/masters
DEBUG
DEBUG Initializing the backend...
DEBUG
DEBUG Initializing provider plugins...
ERROR
ERROR Missing required providers.
ERROR
ERROR The following provider constraints are not met by the currently-installed
ERROR provider plugins:
ERROR
ERROR * ironic (any version)
ERROR
ERROR Terraform can automatically download and install plugins to meet the given
ERROR constraints, but this step was skipped due to the use of -get-plugins=false
ERROR and/or -plugin-dir on the command line.
ERROR
ERROR If automatic installation is not possible or desirable in your environment,
ERROR you may manually install plugins by downloading a suitable distribution package
ERROR and placing the plugin's executable file in one of the directories given in
ERROR by -plugin-dir on the command line, or in the following directory if custom
ERROR plugin directories are not set:
ERROR     terraform.d/plugins/linux_amd64
ERROR
ERROR
ERROR Error: missing provider "ironic"
ERROR
ERROR
FATAL failed to fetch Cluster: failed to generate asset "Cluster": failed to create cluster: failed to initialize Terraform

Version
4.7.0-0.okd-2021-03-28-152009
IPI baremetal
How reproducible
$ openshift-baremetal-install --log-level debug create cluster

This may be related to: https://github.com/openshift/okd/issues/197
I don't think it is resolved yet.

So baremetal IPI never worked in OKD?
No https://docs.okd.io/, no Known Issues about this...

Dupe of #197, no baremetal IPI yet

authors: OleksandrShtepa cgruver vrutkovs

repo: openshift/okd


text:
Setup fix

Fix checking whether setup is required or not, and add V(5) logging to setup.

LGTM

authors: danwinship dcbw

repo: openshift/openshift-sdn


text:
White block visible at bottom of page when loading

cc @rhamilto @cshinn

Given that there aren't really any grey elements anymore, should we just make the .spinner-panel background white?

@cshinn can you take this issue? After talking to @jeff-phillips-18 we were thinking:

set the bg image on the entire landing area not just the main
set the margins so there is some aroudn the catalog browsing area
set the bg of the catalogs area to white

@serenamarie125 is this what you had in mind?

@cshinn 👍

re: the issue, this means that the loading state would just be a big grey rectangle floating in the middle of the background image. Is that acceptable to everyone? I think the padding sort of makes it seem like it's not rooted, insubstantial

this means that the loading state would just be a big grey rectangle floating in the middle of the background image.

For context, all of the other pages in the web console show the message "Loading..." instead of using a spinner. (I mention it since we've recently had other discussions about consistency.)

@cshinn couldn't that background be changed to white?
@spadgett agreed, it should be consistent when loading.
It should also handle the case where there are no services available and show the standard empty page content.

@cshinn I like what you've got so far.
I agree with @spadgett that we need consistency.  Sam can you either embed a screenshot of one of the areas already doing this, or point to the code so we get consistency?
@jeff-phillips-18 brings up a good point, we should have an empty state,

@serenamarie125 Sorry, forgot to follow up on this!
Usually it's just the text "Loading..." in normal font:

Although during API discovery when you first load the console, we use this style:

There are a few places when something might take a long time to load (like logs) where we use an animated ellipsis.

This discussion seems to revolve around the same issue as #198

Its related but slightly different since its related to the loading state before we know what categories to show

repo: openshift/origin-web-catalog


text:
Adds SINGLE_TARGET env var to only build for current GOOS & GOARCH

Adds additional flag to the goreleaser command in the build target, which only compiles the binary for the current GOOS & GOARCH

/lgtm

repo: openshift/osdctl


text:
Added a check for :z :Z SELinux flags in Volumes

These flags are handled when they are passed to Docker. See here
Fixes #587
@bparees ptal?
edit:
My branch was pretty far behind. After the rebase, it looks like we don't check for the ':' character in filepaths. This means we need only change the call to strings.LastIndex() to strings.IndexAny(), so that we split on the first ':'

add a test(positive and negative please) and it should be good to go.

@bparees Added tests, ptal?
[test]

Having some issues with the test that expects a failure

@bparees Added a check for lowercase Windows drive letters, negated the negative test. ptal?

Also, changed all whitespace to tabs in the test case -- there was some inconsistent tabbing :)

Also, changed all whitespace to tabs in the test case -- there was some inconsistent tabbing :)

unfortunately you went the wrong way :(
we should be converting tabs to spaces when found.

@bparees Hopefully I've got everything :) ptal?

lgtm [merge]

[merge]
On Wed, Jan 25, 2017 at 4:07 PM, OpenShift Bot notifications@github.com
wrote:

Source To Image Merge Results: FAILURE (https://ci.openshift.redhat.
com/jenkins/job/test_pr_s2i/415/) (Base Commit: 0fd9583
https://github.com/openshift/source-to-image/commit/0fd9583fab8256281d6122cfd846deb0dc360732
)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/openshift/source-to-image/pull/682#issuecomment-275233026,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AEvl3gJxpZc1kdHaU3kTxVqnMzfPOB5Bks5rV7l5gaJpZM4LrovP
.

--
Ben Parees | OpenShift

@bparees Pretty odd failure, I don't think I mess w/ that test.
re[test]

it's a known flake, jim is working on it.
On Wed, Jan 25, 2017 at 4:34 PM, Ryan Murphy notifications@github.com
wrote:

@bparees https://github.com/bparees Pretty odd failure, I don't think I
mess w/ that test.
re[test]
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/openshift/source-to-image/pull/682#issuecomment-275239742,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AEvl3vw0P5PaWv-CouS1sr6q7qoegwBAks5rV7_KgaJpZM4LrovP
.

--
Ben Parees | OpenShift

Just found it haha
Flake #533, [test]

[merge]

@oatmealraisin
sorry, this is not the flake i thought it was, your test is failing on
windows:
ERROR: An error occurred: Error response from daemon: {"message":"Invalid
bind mount spec "C:\\cygwin\\tmp\\sti\\test-work.xZTj:/home/:z":
Invalid volume specification:
'C:\cygwin\tmp\sti\test-work.xZTj:/home/:z'"}
you're going to need to make the test a little smarter so it expects
failure on windows (or doesn't run on windows)
On Wed, Jan 25, 2017 at 5:23 PM, OpenShift Bot notifications@github.com
wrote:

Source To Image Merge Results: FAILURE (https://ci.openshift.redhat.
com/jenkins/job/test_pr_s2i/418/) (Base Commit: 0fd9583
https://github.com/openshift/source-to-image/commit/0fd9583fab8256281d6122cfd846deb0dc360732
)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/openshift/source-to-image/pull/682#issuecomment-275252032,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AEvl3pT5ctk7IdD71j2F1x6YBG3XtaY5ks5rV8tKgaJpZM4LrovP
.

--
Ben Parees | OpenShift

@bparees That directory looks like it should work? Regardless, I've added a tid-bit to avoid windows.

@bparees I think that the test should pass regardless of Windows, because we use the generated work-dir. I tried looking for a reason why it's failing, but couldn't find anything. I've added a specific case for WIndows.

@bparees Narrowed it down to only my tests to see if I'm timing out the test, not failing my tests. Could this be a problem? Not sure what would have changed to make the unix test fail as well.

@rhcarvalho Thanks for the review! I've updated the commit

[test]

@bparees @rhcarvalho Docker doesn't handle invalid/extra file mode flags anymore, for some reason. This has been causing the tests to hang when it gets to the incorrect part. The Set() method I've been editing now returns an error when the user passes in something like /path/to/dir:/other/path/to/dir:rw:z
Unsure of whether this will fix the windows tests, will see

The Set() method I've been editing now returns an error when the user passes in something like /path/to/dir:/other/path/to/dir:rw:z

The correct syntax is to combine the flags with comma: /path/to/dir:/other/path/to/dir:rw,z.
https://docs.docker.com/engine/reference/run/#volume-shared-filesystems

The comma-delimited options are [rw|ro], [z|Z], [[r]shared|[r]slave|[r]private], and [nocopy].

Unfortunately, I don't think this PR quite makes sense yet as it stands as far as s2i on Windows is concerned.
If you're running s2i on windows, I suppose it's because you're talking to a docker daemon running on Linux.  Any bind using -v is inevitably of a folder locally on the Linux box, into the container running on the Linux box.  Therefore trying to bind anything of the form c:\foo\bar into a Linux container is meaningless.
It's somewhat fiddly - running s2i on Windows:

the path to a repo (if not a URL) will be of the form C:\foo\bar
the argument for an injection will be of the form C:\foo\bar:/baz/bash
the argument for a bound volume will be /foo/bar:/baz/bash[:flags]

@jim-minter Docker for Windows does let you mount windows directories into your running containers with -v like: -v c:\Users\cewong\mydir:/var/mydir

@csrwng urg - how to tell if you're running Docker for Windows, or talking to a Docker socket on a remote Linux box?

Hmm, if that's the case, then does that imply we really shouldn't attempt to parse any -v argument, but just send it straight through to the Docker layer?
The Windows CI test setup we have won't support a test of the form -v c:\foo\bar:/baz/bash because it doesn't run Docker in a VM.  As a result I'd probably make the testing of this non-Windows only.

@jim-minter hmm, that's not easy ... technically Docker for Windows is using a remote Linux vm running on Hyper-V. I guess one way would be to look at the DOCKER_HOST value. In the case of Docker for Windows, it's a windows named pipe (as opposed to a tcp host).

Hmm, if that's the case, then does that imply we really shouldn't attempt to parse any -v argument, but just send it straight through to the Docker layer?

+1

Hmm, if that's the case, then does that imply we really shouldn't attempt to parse any -v argument, but just send it straight through to the Docker layer?

@bparees @jim-minter I'm a bit confused on what you mean. We need to parse -v to create a VolumeSpec. Would we just get rid of that? Or do you mean we should just find the ':' to split on and leave it there?

@oatmealraisin as a starting point I'd at least see what it looks like if you stop using VolumeList (= []VolumeSpec) entirely for -v and use an opaque []string (but keep using VolumeSpecs for injections, because fundamentally we have to be able to parse those).
Don't forget to fix up BuildVolumes VolumeList in type Config in pkg/api/types.go.
Looking at the origin code which calls us in this area, I think it shouldn't be problematic for it to pass []string rather than a []VolumeSpec.  See copyToVolumeList in pkg/build/builder/sti.go.

@jim-minter @bparees  Sorry for the delay, I was making this more complicated than it was. I've reverted VolumeList and changed BuildVolumes to a []string
However, since we let Docker handle more things, it hangs when an extra :rwx,zZ. Because of this, I've removed the negative test in test-stirunimage.sh. Also, I've changed the test to mount ${STI_WORK_DIR}, so that it will test different directories depending on which Jenkins job is running.

[merge]

@bparees Thanks for merging this, I meant to tag you over the weekend.

authors: bparees csrwng jim-minter oatmealraisin rhcarvalho

repo: openshift/source-to-image


text:
Designer once-over on the landing page

What is it?
@ccerv1 has a designer in mind that can help with this on a 1-time short-term contract?

Cici onboarded, he'll start with the OSO page

The once-over is done now

authors: ccerv1 ryscheng ryscheng-mobile

repo: opensource-observer/oso


text:
🛑 Dev is down

In 2a4d51d, Dev (https://dev.opensourcepos.org) was down:

HTTP code: 500
Response time: 606 ms

Resolved: Dev is back up in 6503a80 after 21 hours, 45 minutes.

repo: opensourcepos/upptime


text:
🛑 Weblate is down

In e5a1b95, Weblate (https://translate.opensourcepos.org) was down:

HTTP code: 500
Response time: 1646 ms

Resolved: Weblate is back up in 7e03059 after 24 minutes.


text:
Incomplete libctx/propq support in X509 sigining

The X509 signing still relies on some legacy (non xx_ex) functions, so the digest/signature algorithm selection does not follow the given propq. This can be tracked down to the following:

The openssl req doesn't pass libctx/propq when signing the X509 CSR. It even cannot (see next item).

There is no X509_sign_ex with libctx/propq support.

The ASN1_item_sign_ex is "half-migrated" only. It takes libctx/propq (which is good), but it also takes EVP_MD as input and calls EVP_DigestSignInit (without passing the libctx/propq). In my view, the ASN1_item_sign_ex should take a const char* digest name instead and call EVP_DigestSignInit_ex, which already exists and takes also libctx/propq. This ensures the correct cipher selection, which then manages the digest.

X509_sign_ex() is not necessary as the X509 object can be setup with a libctx, propq.
ie ASN1_item_sign() should use ASN1_item_sign_ex() everywhere with x->libctx, x->propq.
In the apps all EVP_DigestSignInit() should be replaced with EVP_DigestSignInit_ex() so pkey is setup correctly.

Will be fixed by #14752

authors: gotthardp mattcaswell slontis

repo: openssl/openssl


text:
Windows: Enable Hardware-enforced Stack Protection

On Windows, it may be a good idea to enable Hardware-enforced Stack Protection by passing the /CETCOMPAT option to the MSVC linker [1] by default. More details in [2,3].
References

/CETCOMPAT (CET Shadow Stack compatible)
Developer Guidance for Hardware-enforced Stack Protection
Enabling Hardware-enforced Stack Protection (cetcompat) in Chrome

Would it require some special modification of the asm files? Or are the changes necessary exactly the same as those that are there already for Linux?

Marking as inactive, to be closed at the end of 3.4 dev barring further input

authors: matbech nhorman t8m


text:
Some CI builds are too slow

The following five builds exceed the desired 15 minute run time for CIs:

Windows GitHub CI / shared (win32) Successful in 37m
Windows GitHub CI / shared (win64) Successful in 24m
CIFuzz / Fuzzing Successful in 20m
Windows GitHub CI / plain Successful in 17m
GitHub CI / non-caching Successful in 17m

The first two or three are well over.

On the other hand without Windows CI on PRs we are getting constant regressions. I would say we could perhaps drop some tests from the shared builds if we can identify some that are slow and unlikely to get broken only on Windows.

I would say that if you want to drop tests for speed reasons, then you need to review your priorities

But then, we might also want to review what tests should be run for every PR, and what tests only need to be done on a daily basis (or some other regular unit of time)

I would not recommend dropping any of these except of CIFuzz from the set of tests run for every PR. All these find regressions regularly and fixing them post-merge is always harder and with more overhead than when catching the issue before the PR is merged.

I don't have a problem relegating CIFuzz to one of the less often run suites.  It's not the bottleneck here, so I'd be tempted to keep it.  At least until the even slower tests are addressed.
I do agree the others find errors too often to be dropped.
Are alternate Windows tests possible?  They aren't all this slow.  We could also look into reducing the test suite coverage.

Closing this as it doesn't seem very relevant.  So CIs are slow but they aren't impacting negatively.

authors: levitte paulidale t8m


text:
Unsafe use of memcpy() in EVP_enc_null()

The memcpy() routine is explicitly not safe if the source and destination buffers overlap. This can happen when the EVP_enc_null() cipher is used to decrypt longer text than BUF_OFFSET bytes because of this code in bio_enc.c:
EVP_CipherUpdate(&(ctx->cipher),
(unsigned char *)ctx->buf, &ctx->buf_len,
(unsigned char *)&(ctx->buf[BUF_OFFSET]), i);
It's the overwrite of the ctx->buf memory that is the problem.
Saw this on a SUSE 12 SP2 machine. It is not necessarily repeatable on other machines. Valgrind also complained.

Could you please tell us what OpenSSL version you're looking at?

1.0.2d

Looks like this has been fixed in master and 1.1.0 (e.g. see c1a7dcbe16). Fix is too complex for backport so I think we close this.

authors: abigailam2000 levitte mattcaswell


text:
test code in the production path when built with enable-fips

Hi team,
I'm investigating right now an issue with an internal program which started to consume an order of magnitude more CPU when migrated from Centos 7 to AlmaLinux 9.
I was surprised to see this output in perf report when I managed to isolate the offending operation:

Checking the openssl source code, it seems like get_entropy has a special code path when FIPS is enabled (the RHEL9 RPM is, indeed, built with enable-fips):  https://github.com/openssl/openssl/blob/master/providers/implementations/rands/drbg.c#L201
And that code path uses multiple test-related functions in the production code path since https://github.com/openssl/openssl/commit/7f9e74403677a995ded00f070d84297401f6e3fd
Did I correctly evaluate the issue, or am I missing something?
Regards,
Radu

The FIPS 140 standards mandate some testing of the entropy source, that's what you've pointed at.  It's normal and expected.
The FIPS installation process also does some failure testing but this should only happen once.  I wouldn't expect to see the failure testing code executing in normal operation.  Moreover, OSSL_SELF_TEST_oncorrupt_byte() should be extremely lightweight and isn't according to your trace.  Something odd is going on here.  I'd suggest build with debug symbols so you get a better trace...

Thank you for orienting me on the correct path. With debug symbols active, it becomes clear that the time is actually spent in sa_doall.  OSSL_SELF_TEST_oncorrupt_byte is located just before sa_doall in memory in the Alma9 binary, so perf picked and showed the incorrect symbol. Compiling with -O0, revealed that most time is spent in X509_load_cert_crl_file_ex, which gives me a very good lead for investigating the increased CPU consumption.
Sorry for the noise. I'm closing the issue.

authors: paulidale rcarpa


text:
Default and mandatory digest for provider-based signature algorithms

OpenSSL master in https://www.openssl.org/docs/man3.0/man7/provider-signature.html does not contain reference to the two parameters "OSSL_PKEY_PARAM_DEFAULT_DIGEST" and "OSSL_PKEY_PARAM_MANDATORY_DIGEST".
This is somewhat problematic when considering this code:  https://github.com/openssl/openssl/blob/d0a3b9d1eb1fc510ec3447b44803bbf5520a0c47/apps/ca.c#L805-L810
This works correctly if --and only if-- "UNDEF" (SN_undef) is properly reported by a provider not needing a default digest (such as oqsprovider).
So: Would it be sensible to add reference to these two parameters in the documentation -- or rather change the code above to accept a provider not delivering default/mandatory digest information? See https://github.com/open-quantum-safe/oqs-provider/pull/119 for what is required in the provider to enable proper operation of openssl ca (short of a code change in openssl).

... or rather change the code above to accept a provider not delivering default/mandatory digest information?

This would be my preference.  The "UNDEF" is a cludge for when we didn't detect a lack of response very well, we did fix that since...

... or rather change the code above to accept a provider not delivering default/mandatory digest information?

This would be my preference. The "UNDEF" is a cludge for when we didn't detect a lack of response very well, we did fix that since then...

The corresponding older (legacy) function, EVP_PKEY_get_default_digest_nid(), returns -2 when the backend doesn't support that particular query.  The documentation says (in the RETURN VALUES section) that EVP_PKEY_get_default_digest_name() does the same, but the description of the function itself talks about "UNDEF".  This is confusing...

There's a little bit of confusion here, BTW.  Are you saying that with oqs algorithms, the user must not use a digest, i.e. it works the same way as for example ed25519?  In that case, the "UNDEF" answer is exactly correct, a lack of response would rather say that the user should use a digest of their choice.

There's a little bit of confusion here, BTW. Are you saying that with oqs algorithms, the user must not use a digest, i.e. it works the same way as for example ed25519? In that case, the "UNDEF" answer is exactly correct, a lack of response would rather say that the user should use a digest of their choice.

No - OQS algs do work without digest (by default), but a user may specify one to be used.

There's a little bit of confusion here, BTW. Are you saying that with oqs algorithms, the user must not use a digest, i.e. it works the same way as for example ed25519? In that case, the "UNDEF" answer is exactly correct, a lack of response would rather say that the user should use a digest of their choice.

No - OQS algs do work without digest (by default), but a user may specify one to be used.

In that case, it's more sensible to respond with "" or "UNDEF" (either is allowed) on OSSL_PKEY_PARAM_DEFAULT_DIGEST, and not respond at all on OSSL_PKEY_PARAM_MANDATORY_DIGEST.

Documentation amendment coming up!

and not respond at all on OSSL_PKEY_PARAM_MANDATORY_DIGEST

Agreed. That was what the provider code did originally -- but the only way to get a "2" into "def_ret" was by responding to "OSSL_PKEY_PARAM_MANDATORY_DIGEST", too (thus labelling it a hack right now and thus, this issue).
Some self-reflection on my side: It seems I try to avoid doing PR's... still a bit exhausted by #19312 apparently (getting old :). Whatever: Please let me know if you'd also do a code change PR or whether I should propose one. If the latter, a suggestion where best to place a test would be very welcome.

See #20430 for documentation amendment.  I hope this can serve as an example for other documentation pages

still a bit exhausted by https://github.com/openssl/openssl/pull/19312

#19312 was a big PR! I would hope that most PRs are significantly smaller and easier to get through the process!!

and not respond at all on OSSL_PKEY_PARAM_MANDATORY_DIGEST

Agreed. That was what the provider code did originally -- but the only way to get a "2" into "def_ret" was by responding to "OSSL_PKEY_PARAM_MANDATORY_DIGEST", too (thus labelling it a hack right now and thus, this issue).

Why do you want the return value to be 2?  That signifies that "UNDEF" is mandatory, i.e. that the caller must not specify a digest with the signature algorithm.   That doesn't seem to match what you want.
As far as I understand you, the expected return value should 1, with the returned digest name being "UNDEF".  I would interpret this to mean that the caller may, but is not obligated to a digest with the signature algorithm...  although, I just realised that this is a new interpretation, which is not documented, and that apps/ca.c doesn't consider.  However, the inner implementation of EVP_PKEY_get_default_digest_name() does support this:
https://github.com/openssl/openssl/blob/08a11ba20461ce14b0a6b9c9e374fbea91fbd8cf/crypto/evp/keymgmt_lib.c#L554-L566

Some self-reflection on my side: It seems I try to avoid doing PR's... still a bit exhausted by #19312 apparently (getting old :).

I totally get you.  That PR was large, heavy and dealt with a relatively difficult concept.  Some level of exhaustion isn't a surprise.

Whatever: Please let me know if you'd also do a code change PR or whether I should propose one. If the latter, a suggestion where best to place a test would be very welcome.

Let's remember the code bit 'til next week, yeah?

As far as I understand you, the expected return value should 1, with the returned digest name being "UNDEF". I would interpret this to mean that the caller may, but is not obligated to a digest with the signature algorithm... although, I just realised that this is a new interpretation, which is not documented, and that apps/ca.c doesn't consider.

Yes, that would be what I mean/expect.

However, the inner implementation of EVP_PKEY_get_default_digest_name() does support this

Correct, EVP_PKEY_get_default_digest_name supports this, but this code does not (it mandates both params to be there): https://github.com/openssl/openssl/blob/d0a3b9d1eb1fc510ec3447b44803bbf5520a0c47/apps/ca.c#L810
--> Would it be reasonable to just remove the test for def_ret == 2? Just did it as a quick sanity check and all openssl tests pass...

Correct, EVP_PKEY_get_default_digest_name supports this, but this code does not (it mandates both params to be there):
https://github.com/openssl/openssl/blob/d0a3b9d1eb1fc510ec3447b44803bbf5520a0c47/apps/ca.c#L810
--> Would it be reasonable to just remove the test for def_ret == 2? Just did it as a quick sanity check and all openssl tests pass...

That wouldn't be right, 'cause that means that def_ret == 1 is treated as if it was mandatory to not specify a digest.  So basically, not quite that simple.  I'll give that some thought.

Oh, I forgot to mention here that the main documentation change for provider developers happened with #20430

For apps/ca.c modification, see #20460

Closed via #20430 and #20460

authors: baentsch levitte mattcaswell


text:
Problem compiling 1.1.0e on mingw

I am using mingw to build openssl. I use
perl Configure mingw shared
make

The result does not have libcrypto shared library. I checked the output and the only place I see an error is this:
C:\MinGW\bin\ar.exe: creating libcrypto.a
C:\MinGW\bin\ar.exe: creating libssl.a
./libcrypto.a(b_addr.o):b_addr.c:(.text+0xaa): undefined reference to `getnameinfo'
./libcrypto.a(b_addr.o):b_addr.c:(.text+0xb48): undefined reference to `getaddrinfo'
./libcrypto.a(b_addr.o):b_addr.c:(.text+0x819): undefined reference to `freeaddrinfo'
./libcrypto.a(init.o):init.c:(.text+0x1d2): undefined reference to `GetModuleHandleEx'
./libcrypto.a(init.o):init.c:(.text+0x8e4): undefined reference to `GetModuleHandleEx'
./libcrypto.a(o_str.o):o_str.c:(.text+0x5db): undefined reference to `strerror_r'
collect2.exe: error: ld returned 1 exit status
make[2]: *** [link_shlib.mingw] Error 1
make[1]: *** [libcrypto.dll.a] Error 2

I have tried several things with whatever I could find on the internet. Like doing make distclean before Configure or to configure using no-async option. Nothing seems to work.
Any hints or guidance will be really helpful to get this to work. Also after the configure step I had to manually go into the make file and set the PERL variable from C:\MinGW\msys\1.0\bin\perl.exe to C:/MinGW/msys/1.0/bin/perl.exe otherwise make would not be able to find perl in the msys shell.

We probably have to proclaim that minimum requirement for 1.1.x is MSYS2. I mean report sounds like your environment is too old. And there were other problems reported with older MSYS. But the last part about replacing backslashes with slashes sounds utterly strange. I even have hard time imagining how did backslashes got there... To summarize, switch to MSYS2, and run perl and make from MinGW prompt. That's what is actually supported.

Thank you for the response. I switched to msys2 and after the basic installation installed perl, gcc and make.  After Configure ran make and it got an error:
crypto/dso/dso_win32.c:96:5: error: unknown type name 'HINSTANCE'
HINSTANCE h = NULL, *p = NULL;
^~~~~~~~~
Maybe I am missing something in the msys installation. What should I install more to make it work?

You have to install mingw compilers, not just "gcc" and even start appropriate MinGW command window... For reference, "just gcc" is compiler that targets MSYS itself. MinGW compiler generates binaries that can be executed on any Windows system, while MSYS compiler generates binaries that can be executed only in MSYS environment.

I did do this:
pacman -S base-devel msys2-devel mingw-w64-i686-toolchain

Would this install them? I get the same error after this. Which package do I need to install?

It worked after I used the the MinGW command window. Thank you very much.

authors: aryajur dot-asm


text:
openssl smime -crl_check works only when I provide -CAfile

Hi.
Setup

CentOS 7.
OpenSSL 1.0.2k-fips  26 Jan 2017.

Purpose
I want to verify CMS with -crl_check.
Steps to reproduce

Add GOST support to openssl.
Generate certificate here https://www.cryptopro.ru/certsrv/.
Copy root and CRL from https://www.cryptopro.ru/certsrv/certcarc.asp.
Convert root by openssl x509 -inform DER -in /home/certnew.cer -out crypto-cert.pem.
Convet CRL by openssl crl -in certcrl.crl -inform der -out certcrl.pem.
Combine root and CRL by cat crypto-cert.pem certcrl.pem > crypto-cert-combine.pem.
Copy crypto-cert-combine.pem to /etc/pki/ca-trust/source/anchors/.
Execute update-ca-trust.

Expected behaviour
Run openssl smime -verify -crl_check -inform DER -in /home/cms.bin -content /home/message.txt and get response:
Hi.
Verification successful

Actual behaviour
Get response:
Verification failure
140580621272976:error:21075075:PKCS7 routines:PKCS7_verify:certificate verify error:pk7_smime.c:336:Verify error:unable to get certificate CRL

Workaround
Run openssl smime -verify -crl_check -inform DER -in /home/cms.bin -content /home/message.txt -CAfile /etc/pki/ca-trust/source/anchors/crypto-cert-combine.pem.
Response:
Hi.
Verification successful

Why workaround is not applicable
I've got around 400-500 combined roots and CRLs and I'm not sure that it's a good idea to go through them all until I have a success or all of them failed.
Questions

Is this the expected behaviour?
Is there any option to don't use the workaround (-CApath changes nothing)?

Attachments
cms-and-message.zip

This is expected behaviour. If you set -crl_check, openssl add flag for checks end entity certificate validity by attempting to look up a valid CRL. If a valid CRL cannot be found an error occurs.
Certificates verifing in check_cert(X509_STORE_CTX *ctx). There you can see, what if cannot finde CRL return error

@algv could you say where openssl looks for valid CRL by default in CentOS? And what does "valid" mean?

First openssl serach CRL by "score" (it some criterias by issuer name, and, most newer...). Then check CRL validaty. Valid CRL is actual by time and have correct signature.
In CentOS and any other systems need give path to CRL. May be help:  https://github.com/openssl/openssl/issues/1701

CentOS uses /etc/pki/tls/cert.pem as the default CA bundle path.

authors: algv colotiline t8m